-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Conversation
Need to ask if someone has applied changes from the AWS CLI without checking the code changes back into the repo |
There was a problem hiding this 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" |
There was a problem hiding this comment.
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?).
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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" { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Closing this in favor of splitting up the module and its use as in: |
Resolves Issue #17
What changed?
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