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

r/storage_accounts: using a SharedKey Authorizer if opted out of AzureAD for configuring the Static Website #6050

Merged
merged 9 commits into from
Mar 11, 2020

Conversation

tombuildsstuff
Copy link
Contributor

So this is a fun one.

Firstly this started out adding support for SharedKey Authorization for Storage Accounts which was an oversight - however after a bunch of digging it turns out the Storage Data Plane API itself uses a different, undocumented variant of the SharedKey authorization method) for API's hosted in the root of the Data Plane API.

As such this PR does several things:

  1. Switches to using SharedKey Authorization for configuring the Static Website if AzureAD Authentication is disabled
  2. Fixes a crash when an empty static_website block was specified
  3. Updates to v0.9.0 of github.com/tombuildsstuff/giovanni which ensures an Enabled/Disabled value is always submitted to the API since this is now required
  4. Switches to using a fork of github.com/Azure/go-autorest which includes this PR
  5. Fixes the "AzureAD" test by removing the duplicate provider block

Fixes #5914
Fixes #5991
Fixes #6028

This allows us to use SharedKey as an authorizer when AzureAD is not enabled
which is the case for the majority of users.

Fixes #6028
Fixes #5914
…ined

Since the presence of the `static_website` block signifies Enabled but all
fields within it are optional, this allows us to set an empty block.

Fixes #5991
* v0.9.0 of github.com/tombuildsstuff/giovanni
* a fork of github.com/Azure/go-autorest including Azure/go-autorest#512
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tombuildsstuff
Copy link
Contributor Author

Ignoring some (known, unrelated/in flight) test failures the tests look good:

Screenshot 2020-03-11 at 08 07 53

@tombuildsstuff tombuildsstuff merged commit 26beaaf into master Mar 11, 2020
@tombuildsstuff tombuildsstuff deleted the b/storage-account-data-plane branch March 11, 2020 07:08
tombuildsstuff added a commit that referenced this pull request Mar 11, 2020
@ghost
Copy link

ghost commented Mar 11, 2020

This has been released in version 2.1.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 "azurerm" {
    version = "~> 2.1.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Apr 10, 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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.