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

Stop requiring subscription_id and update configuration guides #271

Merged
merged 3 commits into from
Jun 16, 2020

Conversation

manicminer
Copy link
Member

@manicminer manicminer commented Jun 11, 2020

Stop requiring a subscription ID to be configured (though still allow it for compatibility)

Configuration guide updates

  • Walkthroughs reflect current portal experience
  • Amend guidance on AAD permissions
    • Document some common AAD roles and API scopes
    • Note on subscription ID
  • Remove references to configuring subscriptions

Also fix up certificate related tests by shortening validity period to 6 months

DEPRECATION WARNING

The subscription_id attribute will be removed from the azuread_client_config data source in v1.0. The subscription ID here is only an echo of the subscription ID supplied by the user and there is no possible way to use it currently elsewhere in the provider. Configurations can switch over to the azurerm_client_config data source.

Closes: #232

@manicminer manicminer force-pushed the docs/update-guides branch 4 times, most recently from 4e9bb5b to ee35a73 Compare June 11, 2020 02:40
@manicminer manicminer added this to the v0.11.0 milestone Jun 11, 2020
@manicminer manicminer marked this pull request as draft June 11, 2020 11:51
@ghost ghost added size/XL and removed size/L labels Jun 12, 2020
@manicminer manicminer changed the title Docs: Update guides Stop requiring subscription_id and update configuration\ guides Jun 12, 2020
@manicminer manicminer changed the title Stop requiring subscription_id and update configuration\ guides Stop requiring subscription_id and update configuration guides Jun 12, 2020
@manicminer manicminer marked this pull request as ready for review June 12, 2020 17:00
@manicminer manicminer requested a review from a team June 12, 2020 17:00
@manicminer manicminer force-pushed the docs/update-guides branch 3 times, most recently from be29138 to f439921 Compare June 12, 2020 17:35
@manicminer
Copy link
Member Author

OK I think that's ready now! 😊

@manicminer
Copy link
Member Author

One more rebase to split a commit 😅

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

thanks @manicminer! overall this is good but i think we should keep the subscription property in the data source until 1.0 - can deprecate it for now and remove it then.

azuread/data_client_config.go Show resolved Hide resolved
Configuring a subscription ID is a vestige from the provider split. We
don't use subscription_id anywhere and have no plans to. Any resource
that operates on a subscription or its dependents should belong in the
azurerm provider.

We'll keep the configuration property around for now so that users have
time to remove it from their configurations, and to avoid having to
remove the corresponding property from the `azuread_client_config` data
source. This will be dropped in v1.0.

Although it's never used because we only use SDK clients that are
configured with a tenant ID, go-azure-helper requires it for sensible
reasons and it doesn't make sense to push this concern upstream for now.
So for configurations where a subscription ID is not specified, we adopt
the tenant ID for the subscription ID. This mimics the Azure CLI which
does something similar when `--allow-no-subscriptions` is specified
(although we don't intend to expose this to users).
- Walkthroughs reflect current portal experience
- Amend guidance on AAD permissions
  * Document some common AAD roles and API scopes
  * Note on subscription ID
- Remove irrelevant subscription references
@manicminer
Copy link
Member Author

manicminer commented Jun 15, 2020

thanks @manicminer! overall this is good but i think we should keep the subscription property in the data source until 1.0 - can deprecate it for now and remove it then.

@katbyte Sounds good. I've reworked it so that for existing configurations, where a subscription ID is configured for the provider, it's returned by data.azuread_client_config. For newer configurations where it's omitted, the data source attribute is a blank string. Do you see any problems with doing it this way?

Copy link
Collaborator

@katbyte katbyte 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! 👍

@manicminer
Copy link
Member Author

Tests pass

Screenshot 2020-06-15 17 14 18

@manicminer manicminer merged commit c76bb53 into master Jun 16, 2020
@manicminer manicminer deleted the docs/update-guides branch June 16, 2020 22:50
manicminer added a commit that referenced this pull request Jun 16, 2020
@ghost
Copy link

ghost commented Jul 9, 2020

This has been released in version 0.11.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azuread" {
    version = "~> 0.11.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Jul 17, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document minimum permissions required for creating Service Principals
2 participants