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

Add support for "sovereign" Azure cloud environments #4997

Merged
merged 2 commits into from
Aug 15, 2018
Merged

Add support for "sovereign" Azure cloud environments #4997

merged 2 commits into from
Aug 15, 2018

Conversation

chludwig-haufe
Copy link
Contributor

The implementation of the Azure physical storage backend in vault 0.10.4 creates the storage client by means of the Azure storage SDK's function NewBasicClient(accountName, accountKey). This function unconditionally assumes the storage account belongs to the Azure "public" cloud. It therefore is not possible to make vault 0.10.4 use a blob storage created in one of the "sovereign" Azure clouds (like Azure Germany) as its backend.

This PR adds the parameter cloudEnvironment to the configuration of the Azure physical storage backend. It also obtains the Azure storage client by calling NewBasicClientOnSovereignCloud(accountName, accountKey, cloudEnvironment). Since the config parameter cloudEnvironment defaults to "AzurePublicCloud", the change is backwards compatible. Users of one of the sovereign cloud environments have now the option to specify a storage account in their respective environment, though.

Copy link
Contributor

@chrishoffman chrishoffman left a comment

Choose a reason for hiding this comment

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

This looks great! The only comment is minor around variable naming. I think you can simplify all the names down to environment and environmentName.

client, err := storage.NewBasicClient(accountName, accountKey)
cloudEnvironmentName := os.Getenv("AZURE_ENVIRONMENT")
if cloudEnvironmentName == "" {
cloudEnvironmentName = conf["cloudEnvironment"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think environment should be suitable for the config element.

@chludwig-haufe
Copy link
Contributor Author

@chrishoffman I pushed an update that drops the "cloud" from the variable names and config parameter.

@chrishoffman chrishoffman added this to the 0.11 milestone Aug 13, 2018
@chrishoffman chrishoffman merged commit d74fae4 into hashicorp:master Aug 15, 2018
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.

2 participants