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

Ignore service principal password resources from Terraform state management #351

Merged
merged 9 commits into from
Oct 3, 2019

Conversation

erikschlegel
Copy link
Contributor

All Submissions:


  • [YES] Have you followed the guidelines in our Contributing document?
  • [YES] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [YES] I have updated the documentation accordingly.
  • [NA] I have added tests to cover my changes.
  • [YES] All new and existing tests passed.
  • [YES/NO/NA] My code follows the code style of this project.
  • [YES] I ran lint checks locally prior to submission.
  • [YES] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

What is the current behavior?


Currently, all azure resource(s) state changes related to the service principal password are managed through Terraform. This results in state changes being triggered for Terraform incremental builds as the password is randomly generated on each run. The impact is Terraform attempts to delete and recreate the password and KV secrets on each incremental deployment, regardless of template changes. Terraform should only be responsible for creating resources like the SP password and keyvault secrets when bootstrapping the initial azure environment.

Issue Number: #330

What is the new behavior?


Terraform resource types such as service prinipal passwords and keyvault secrets capturing sp passwords are ignored from state change lifecycles events via

lifecycle {
    ignore_changes = ["value"]
  }

Does this introduce a breaking change?


  • [NO]

@erikschlegel
Copy link
Contributor Author

@iphilpot all checks are passing and ready for review

@@ -28,4 +28,8 @@ resource "azuread_service_principal_password" "sp" {
service_principal_id = azuread_service_principal.sp[0].object_id
value = local.sp_password
end_date_relative = var.sp_pwd_end_date_relative

lifecycle {
Copy link
Contributor

Choose a reason for hiding this comment

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

These lifecycle hooks bake in the original passwords inserted into the creation of the service principal password and the key vault secrets that store them. How will password updates be managed due to this change? Outside of terraform?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes should be completed outside of TF

@@ -28,4 +28,8 @@ resource "azuread_service_principal_password" "sp" {
service_principal_id = azuread_service_principal.sp[0].object_id
value = local.sp_password
end_date_relative = var.sp_pwd_end_date_relative

lifecycle {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to put this in the SP module and not in the template?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind, it's a config element it looks like.

@erikschlegel erikschlegel merged commit b6a8507 into master Oct 3, 2019
@nmiodice nmiodice deleted the erisch/features/sp-mod-pwd-ignore branch April 14, 2020 19:06
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