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

Azure Creds: Honors the documented default behavior (again) #2190

Conversation

TheLonelyGhost
Copy link
Contributor

@TheLonelyGhost TheLonelyGhost commented Mar 13, 2024

If no environment is specified, will show up as an empty string and (currently) throw an error that "" is an unknown cloud provider. After this change it allows the environment attribute on the datasource to be optional again.

Ideally this would fetch the value from Vault, but for most cases we may want the default to be Azure Public Cloud, which is the default on the Vault backend if nothing is specified.

Description

Documented behavior of the azure credentials data source is that environment is optional. In reality, not providing it (only in v4.0.0) results in an error message saying "" is an unknown cloud provider.

Closes #2189

Checklist

  • Added CHANGELOG entry (only for user-facing changes)
  • Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccDataSourceAzureAccessCredentials'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -run=TestAccDataSourceAzureAccessCredentials -timeout 30m ./...
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/consts   [no test files]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.319s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/group   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/mfa     [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/pki      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/internal/sync     [no test files]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
?       github.com/hashicorp/terraform-provider-vault/util/mountutil    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider (cached) [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/testutil  (cached) [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      (cached) [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/vault     0.654s

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 13, 2024

CLA assistant check
All committers have signed the CLA.

@TheLonelyGhost TheLonelyGhost force-pushed the fix/azure-creds-missing-environment-name branch from 1a7b826 to 36df45e Compare March 13, 2024 23:32
Ideally this would be drawn from Vault Server, but the documented
default there is to point at Azure Public Cloud. This seems like a
decent compromise.

Signed-off-by: David Alexander <opensource@thelonelyghost.com>
@TheLonelyGhost TheLonelyGhost force-pushed the fix/azure-creds-missing-environment-name branch from 36df45e to fb100b3 Compare March 14, 2024 01:40
@TheLonelyGhost TheLonelyGhost marked this pull request as ready for review March 14, 2024 01:41
Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

Thanks @TheLonelyGhost !

@fairclothjm fairclothjm added this to the 4.0.1 milestone Mar 14, 2024
@fairclothjm fairclothjm merged commit 37e87b4 into hashicorp:main Mar 14, 2024
2 checks passed
@TheLonelyGhost TheLonelyGhost deleted the fix/azure-creds-missing-environment-name branch March 19, 2024 00:58
@fairclothjm fairclothjm modified the milestones: 4.0.1, 4.1.0 Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Azure access credentials marked environment as optional, but errors out unless specified
3 participants