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

XDR-3403: Update Terraform's Azuread provider to 2.x #5

Merged
merged 8 commits into from
May 29, 2022

Conversation

GerardSetho
Copy link
Member

2 modules are upgraded to use azuread provider 2.22:

  • azuread-application and
  • azuread-service-principal

There wasn't any example code for azuread-service-principal, so we add that.

Many of the changes are simply

  • changes argument names
  • addition of new blocks in the azuread_application (e.g. the new api and web blocks)

However there are some more significant changes:

azuread-application: available_to_other_tenants

This argument available_to_other_tenants is changed to sign_in_audience. And while it was previously a boolean, it is now a string, with the following possible values:

  • "AzureADMyOrg" (default)
  • "AzureADMultipleOrgs"
  • "AzureADandPersonalMicrosoftAccount"
  • "PersonalMicrosoftAccount"

azuread-service-principal-password: password

We used to take in an optional input password. If password isn't specified, we would generate one (using the random_password resource).

From v 2.0 onwards, this is no longer possible. The password will be generated; it cannot be specified.

Copy link
Contributor

@hensonto hensonto left a comment

Choose a reason for hiding this comment

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

Looks good, I only have some minor changes

modules/azuread-service-principal/outputs.tf Outdated Show resolved Hide resolved
modules/azuread-application/main.tf Outdated Show resolved Hide resolved
modules/azuread-application/vars.tf Show resolved Hide resolved
modules/azuread-application/main.tf Outdated Show resolved Hide resolved
Copy link
Contributor

@arledesma arledesma left a comment

Choose a reason for hiding this comment

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

With the variable names changing this is a breaking change.
Please amend the commit that changes the vars to include either (major) or (breaking) instead of (feat).

https://github.com/quantum-sec/meta/blob/master/standards/versioning/README.md#incrementing-versions

Copy link
Contributor

@arledesma arledesma left a comment

Choose a reason for hiding this comment

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

I'm not sure if the uuid() changes for each terraform apply.

examples/azuread-application/main.tf Show resolved Hide resolved
modules/azuread-application/main.tf Outdated Show resolved Hide resolved
@arledesma arledesma dismissed their stale review May 27, 2022 03:31

We talked about it

Merely using uuid() would generate a new value each time
we apply the resource. We can avoid this by
generating a random_uuid resource per oauth2_permission_scopes.
Copy link
Contributor

@hensonto hensonto left a comment

Choose a reason for hiding this comment

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

Looks good to me, I think you can merge this but do a tf plan first to make sure changes are reflected.

@GerardSetho GerardSetho merged commit f0edfe4 into master May 29, 2022
@GerardSetho GerardSetho deleted the feature/XDR-3403 branch May 29, 2022 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants