-
Notifications
You must be signed in to change notification settings - Fork 535
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
Support creating Azure secret backend role by specifying the role_id #1573
Conversation
+ added validation so only role_id or role_name is provided but not both + added acceptance test * converted from the deprecated schema callbacks to their version with Context & Diagnostics * various small formatting, warnings and standardization fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Had one nit about a style precedent, but otherwise looks good
err := azureSecretBackendRoleUpdateFields(d, data) | ||
if err != nil { | ||
return err | ||
diags := azureSecretBackendRoleUpdateFields(ctx, d, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like you're only returning diags
if there is an error; else it is nil
, in that case we have a style precedent of something like
if diags := azureSecretBackendRoleUpdateFields(ctx, d, data); diags != nil {
return diags
}
@@ -47,6 +48,10 @@ func TestAzureSecretBackendRole(t *testing.T) { | |||
resource.TestCheckResourceAttrSet("vault_azure_secret_backend_role.test_azure_groups", "azure_groups.0.object_id"), | |||
), | |||
}, | |||
{ | |||
Config: testAzureSecretBackendRoleConfigError(subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Read: ReadWrapper(azureSecretBackendRoleRead), | ||
Update: azureSecretBackendRoleCreate, | ||
Delete: azureSecretBackendRoleDelete, | ||
CreateContext: azureSecretBackendRoleCreate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating these!
…ashicorp#1573) * + support creating Azure secret backend role by specifying the role_id + added validation so only role_id or role_name is provided but not both + added acceptance test * converted from the deprecated schema callbacks to their version with Context & Diagnostics * various small formatting, warnings and standardization fixes * * normalize style of diags.err hangling
Community Note
Closes #715
Release note for CHANGELOG:
Output from acceptance testing: