Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cognito to Terraform #61

Closed
wants to merge 4 commits into from

Conversation

chelseybeck
Copy link
Member

@chelseybeck chelseybeck commented Mar 10, 2024

Resolves Issue #17

What changed?

  • Added a Cognito module for projects to use
  • Added an example Cognito configuration to the people depo project

Issues

I wasn't able to run a successful plan on this because there is a mismatch between the codebase & states. Leaving this as a draft for now, until that is resolved

@chelseybeck chelseybeck changed the title 17 add cognito to tf Add cognito to Terraform Mar 10, 2024
@chelseybeck chelseybeck mentioned this pull request Mar 10, 2024
5 tasks
@chelseybeck chelseybeck marked this pull request as ready for review March 10, 2024 07:06
@chelseybeck chelseybeck marked this pull request as draft March 10, 2024 07:07
@robinglov
Copy link
Member

robinglov commented Mar 14, 2024

Need to ask if someone has applied changes from the AWS CLI without checking the code changes back into the repo

Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of terraform is limited so I'm not sure if my comments are relevant to this issue.

The example user pool client config looks good. I'm not sure how the client app can access the generated secret, but that's probably something to figure out later.


region = "us-west-2"
user_pool_name = "people-depot-user-pool"
client_name = "people-depot-client"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question mainly for my understanding: where should the configuration of the project user pool and client be defined? Does the project create more resources in this file? or is it using the resources configuration defined in cognito/main.tf and the 3 var fields are the only customization possible?

I think people depot deployment will eventually need several clients, ones with secret for the backends (people depot and CTJ?), and ones without secret for frontends (VRMS, website, CTJ?).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @fyliu - apologies for the late response here...i just noticed these comments yesterday.

the modules are basically the recipes that any project can use to build what they need. the variables can have default values but those can be overridden if you pass values when calling the module. so, files in /cognito define the resources (main.tf), along w/ their variables and outputs

the main.tf file inside of people-depot/project calls that module and passes the client info (or whatever other configs you want to pass it)

for adding multiple clients, you'd call the module multiple times like so:

module "cognito_1" {
  source = "../../../terraform-modules/cognito"

  region         = "us-west-2"
  user_pool_name = "x-user-pool"
  client_name    = "x-client"
}
module "cognito_2" {
  source = "../../../terraform-modules/cognito"

  region         = "us-west-2"
  user_pool_name = "x-user-pool"
  client_name    = "x-client"
}

if the configs need to change (like generate_secret = true) - then we can designate those as variables when creating the module and then pass the value when calling the module in project/main.tf

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, each project would use the module inside their project directory's main.tf file. if it's a shared resource, we can move it to a shared directory...but it sounded like this would be project specific

@@ -0,0 +1,15 @@
variable "region" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should something be using this var? It doesn't look like this var is being referenced in any resource.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's set to the wrong default thought, i think...which is getting overridden when called in project/main.tf. here we're saying...these are the possible variables for these resources...we may or may not add defaults...and they can be overridden when called in other main.tf files

@chelseybeck chelseybeck marked this pull request as ready for review May 16, 2024 15:31
@chelseybeck
Copy link
Member Author

Closing this in favor of splitting up the module and its use as in:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants