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

Allow DBA password to be updated via Terraform #46

Merged
merged 5 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions internal/framework/generic_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ type ResourceState interface {
Reset()

// CheckReady returns an error if the resource is not in the desired
// state according to its spec, based on the local state.
CheckReady() error
// state according to its spec, based on the local state. The supplied
// client can be used to request additional readiness information from
// the server.
CheckReady(ctx context.Context, client openapi.ClientInterface) error

// Create creates the resource in the backend based on the local state
// and waits for it to satisfy IsReady().
Expand Down Expand Up @@ -319,7 +321,7 @@ func (r *GenericResource) AwaitReady(ctx context.Context, state ResourceState, o
defer cancel()
for {
// Check if resource is ready
readyErr := state.CheckReady()
readyErr := state.CheckReady(ctx, r.client.Client)
if readyErr == nil {
return nil
}
Expand Down
43 changes: 30 additions & 13 deletions internal/provider/database/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func (state *DatabaseResourceModel) DbaPasswordMatches(other *DatabaseResourceMo
return true
}

func (state *DatabaseResourceModel) CheckReady() error {
func (state *DatabaseResourceModel) CheckReady(ctx context.Context, client openapi.ClientInterface) error {
// Check that database is either Available or Stopped based on maintenance.isDisabled value
if state.Status == nil || state.Status.State == nil {
return fmt.Errorf("Database %s/%s/%s has no status information", state.Organization, state.Project, state.Name)
}
Expand All @@ -51,6 +52,18 @@ func (state *DatabaseResourceModel) CheckReady() error {
return fmt.Errorf("Database %s/%s/%s has an unexpected state: expected=%s, found=%s",
state.Organization, state.Project, state.Name, expectedState, *state.Status.State)
}
// Check that DBA password is up-to-date if password update is supported
resp, err := state.UpdateDbaPassword(ctx, client, nil)
if IsDbaPasswordUpdateUnsupportedError(resp, err) {
return nil
}
if err != nil {
return err
}
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("DBA password for database %s/%s/%s has not been updated",
state.Organization, state.Project, state.Name)
Comment on lines +64 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the logic, but it would be nice to include in the error details that the most likely cause of the failure is that there was an external change that set the database password to not match the old or new password and to resolve it, manually set the password to the new one.

Copy link
Collaborator Author

@adriansuarez adriansuarez Apr 5, 2024

Choose a reason for hiding this comment

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

But this is the case where an error was not returned by the REST service. If it did, we would have returned the error to the user.

If we're here, then the response would have been "202 Accepted", which indicates that password rotation is still in progress. Password rotation could be in progress indefinitely if the database is in Stopped state, or if there is some issue that is preventing the operator from doing reconciliation (but not an authentication failure, since that would cause the operator to blow away target key).

}
return nil
}

Expand Down Expand Up @@ -93,24 +106,28 @@ func IsDbaPasswordUpdateUnsupportedError(resp *http.Response, err error) bool {
return false
}

func (state *DatabaseResourceModel) UpdateDbaPassword(ctx context.Context, client openapi.ClientInterface, target *string) (*http.Response, error) {
request := openapi.UpdateDbaPasswordModel{
Current: *state.DbaPassword,
Target: target,
}
resp, err := client.UpdateDbaPassword(ctx, state.Organization, state.Project, state.Name, nil, request)
if err != nil {
return resp, err
}
// Decode the response and check that there is no error
return resp, helper.ParseResponse(resp, nil)
}

func (state *DatabaseResourceModel) Update(ctx context.Context, client openapi.ClientInterface, currentState framework.ResourceState) error {
// Try to update DBA password if it was changed in config
currentDatabase, _ := currentState.(*DatabaseResourceModel)
if !state.DbaPasswordMatches(currentDatabase) {
request := openapi.UpdateDbaPasswordModel{
Current: *currentDatabase.DbaPassword,
Target: state.DbaPassword,
}
resp, err := client.UpdateDbaPassword(ctx, state.Organization, state.Project, state.Name, nil, request)
if err != nil {
return err
resp, err := currentDatabase.UpdateDbaPassword(ctx, client, state.DbaPassword)
if IsDbaPasswordUpdateUnsupportedError(resp, err) {
return fmt.Errorf(DBA_PASSWORD_CHANGE_UNSUPPORTED_MSG)
}
// Decode the response and check that there is no error
err = helper.ParseResponse(resp, nil)
if err != nil {
if IsDbaPasswordUpdateUnsupportedError(resp, err) {
return fmt.Errorf(DBA_PASSWORD_CHANGE_UNSUPPORTED_MSG)
}
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/project/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (state *ProjectResourceModel) Reset() {
*state = ProjectResourceModel{}
}

func (state *ProjectResourceModel) CheckReady() error {
func (state *ProjectResourceModel) CheckReady(ctx context.Context, client openapi.ClientInterface) error {
if state.Status == nil || state.Status.State == nil {
return fmt.Errorf("Project %s/%s has no status information", state.Organization, state.Name)
}
Expand Down
20 changes: 19 additions & 1 deletion internal/provider_test/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1011,9 +1011,27 @@ func TestImmutableAttributeChange(t *testing.T) {
t.Run("dbaPasswordChange", func(t *testing.T) {
// Change DBA password and run `terraform apply`
vars.database.DbaPassword = ptr("updated")
// Expect readiness check to fail due to DBA password
// not being updated and specify small timeout
if vars.providerCfg.Timeouts == nil {
vars.providerCfg.Timeouts = map[string]framework.OperationTimeouts{
"database": {
Update: ptr("2s"),
},
}
}
tf.WriteConfigT(t, vars.builder.Build())
out, err := tf.Apply()
require.NoError(t, err)
if IsE2E() {
// E2E tests disable readiness check completely
require.NoError(t, err)
} else {
// Check that readiness check failed due to DBA password
require.Error(t, err)

// TODO: Figure out how to check error messages despite Terraform line wrapping
require.Contains(t, string(out), "DBA password for database org/proj/db")
}
require.Contains(t, string(out), "0 to add, 1 to change, 0 to destroy.")
adriansuarez marked this conversation as resolved.
Show resolved Hide resolved
})
}
Expand Down