Module 12 - #12
Conversation
BenRamchandani
left a comment
BenRamchandani
left a comment
There was a problem hiding this comment.
Hey, the Terraform config looks good and I can see the app is working.
I'll accept this on Aptem, but please make the suggested changes around marking variables/outputs as sensitive before merging.
| } | ||
|
|
||
| resource "azurerm_app_service" "main" { | ||
| name = "charlie-devops-to-do-terraform" |
There was a problem hiding this comment.
It's good practice to prefix everything, that way you can tell what's terraformed and can have multiple instances of the infrastructure.
| # Deploying the application | ||
|
|
||
| The application is deployed with azure and is accessible at https://charlie-devops-to-do.azurewebsites.net/ | ||
| The application is deployed with azure and is accessible at https://prod-charlie-devops-to-do.azurewebsites.net/ |
There was a problem hiding this comment.
| output "cd_webhook" { | ||
| value = "https://${azurerm_app_service.main.site_credential[0].username}:${azurerm_app_service.main.site_credential[0].password}@${azurerm_app_service.main.name}.scm.azurewebsites.net/docker/hook" | ||
| } |
There was a problem hiding this comment.
Should be marked as sensitive = true so Terraform doesn't print it out, it has your App Service's deployment password in.
You can change the password in the Azure portal, go to the App Service -> Container settings (Classic) -> FTPS credentials -> Application scope and reset the password.
| } | ||
|
|
||
| resource "azurerm_storage_account" "tfstate" { | ||
| name = "tfstate${random_string.resource_code.result}" |
There was a problem hiding this comment.
You don't normally manage Terraform's backend state with Terraform. It means you can't recreate everything with one terraform apply, and can't use workspaces for different environments.
If you delete this file you'll need to remove these objects from the state so Terraform doesn't destroy them, e.g. terraform state rm 'azurerm_storage_account.tfstate'.
| variable "AUTH_CLIENT_SECRET" { | ||
| description = "GitHub client secret for authentication." | ||
| } | ||
|
|
||
| variable "SECRET_KEY" { | ||
| description = "The Azure secret key" | ||
| } |
There was a problem hiding this comment.
These should both be marked as sensitive = true so they don't get printed in terraform plan output.
(Also the SECRET_KEY is for signing Flask's session cookies, not really Azure related)
There was a problem hiding this comment.
Nice work, I can see Travis is now managing your infrastructure as part of its pipeline now, and that your app is up and running in Azure. I've left some thoughts below, and particularly we should definitely add sensitive tags to some of your input/output variables, but I'm happy for you to merge once you've done so
Edit: Looks like this is basically a duplicate of Ben's - feel free to ignore 🙂
| # Deploying the application | ||
|
|
||
| The application is deployed with azure and is accessible at https://charlie-devops-to-do.azurewebsites.net/ | ||
| The application is deployed with azure and is accessible at https://prod-charlie-devops-to-do.azurewebsites.net/ |
There was a problem hiding this comment.
Not sure if you changed your plan, but I was actually able to access it at:
http://charlie-devops-to-do-terraform.azurewebsites.net/
| @@ -0,0 +1,7 @@ | |||
| output "cd_webhook" { | |||
| value = "https://${azurerm_app_service.main.site_credential[0].username}:${azurerm_app_service.main.site_credential[0].password}@${azurerm_app_service.main.name}.scm.azurewebsites.net/docker/hook" | |||
There was a problem hiding this comment.
We should mark this output as sensitive since it contains a password - that way Terraform will avoid logging it unless explicitly told to
| @@ -0,0 +1,20 @@ | |||
| resource "random_string" "resource_code" { | |||
There was a problem hiding this comment.
We wouldn't usually track the storage account for the terraform state in terraform - you can't safely create the account until there's somewhere to store state, and obviously you can't create the state until the storage account exists. If you now tried to rename one of these resources, causing a destroy/recreate, then we'd lose the existing state. Curious to know if you disagree and there is some clever workaround/solution for this
| } | ||
|
|
||
| variable "AUTH_CLIENT_SECRET" { | ||
| description = "GitHub client secret for authentication." |
There was a problem hiding this comment.
As with the outputs, we should mark any sensitive input variables as such to avoid them potentially being printed out in logging anywhere
3 participants
No description provided.