diff --git a/CHANGELOG.md b/CHANGELOG.md index 2abc1f045..5246499c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ nav_order: 1 ## [MAJOR.MINOR.PATCH] - YYYY-MM-DD +- Fix `pg_user`: throws an error when the password is modified directly in the service - Enable `read_replica` service integration for `aiven_redis` and `aiven_valkey` resources - Add `disaster_recovery` service integration type support - Add `aiven_flink_jar_application`, `aiven_flink_jar_application_version` and `aiven_flink_jar_application_deployment` BETA resources diff --git a/internal/schemautil/service_user.go b/internal/schemautil/service_user.go index 0313bad96..b2f8c06c1 100644 --- a/internal/schemautil/service_user.go +++ b/internal/schemautil/service_user.go @@ -44,14 +44,9 @@ func ResourceServiceUserCreate(ctx context.Context, d *schema.ResourceData, m in } // Retry because the user may not be immediately available - err = RetryNotFound(ctx, func() error { - // ResourceServiceUserRead resets id each time it gets 404, setting/restoring it here. - d.SetId(BuildResourceID(projectName, serviceName, username)) - err := ResourceServiceUserRead(ctx, d, m) - return ErrorFromDiagnostics(err) - }) - - return diag.FromErr(err) + // todo: Retry NotFound user might be not available immediately + d.SetId(BuildResourceID(projectName, serviceName, username)) + return ResourceServiceUserRead(ctx, d, m) } func ResourceServiceUserUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { diff --git a/internal/schemautil/wait.go b/internal/schemautil/wait.go index 9ec4051b2..bb8c362af 100644 --- a/internal/schemautil/wait.go +++ b/internal/schemautil/wait.go @@ -377,15 +377,5 @@ func WaitUntilNotFound(ctx context.Context, retryableFunc retryGo.RetryableFunc) ) } -// retryNotFoundAttempts just a random number -const retryNotFoundAttempts = 10 - -func RetryNotFound(ctx context.Context, retryableFunc retryGo.RetryableFunc) error { - return retryGo.Do( - retryableFunc, - retryGo.Context(ctx), - retryGo.Delay(time.Second), - retryGo.Attempts(retryNotFoundAttempts), - retryGo.RetryIf(IsNotFound), - ) -} +// RetryNotFoundAttempts just a random number +const RetryNotFoundAttempts = 10 diff --git a/internal/sdkprovider/service/pg/pg_user.go b/internal/sdkprovider/service/pg/pg_user.go index 123aedf08..2473cb297 100644 --- a/internal/sdkprovider/service/pg/pg_user.go +++ b/internal/sdkprovider/service/pg/pg_user.go @@ -2,6 +2,7 @@ package pg import ( "context" + "errors" "fmt" "time" @@ -108,12 +109,24 @@ func ResourcePGUserCreate(ctx context.Context, d schemautil.ResourceData, client } } - // Retry because the user may not be immediately available - return schemautil.RetryNotFound(ctx, func() error { - // ResourcePGUserRead resets id each time it gets 404, setting/restoring it here. - d.SetId(schemautil.BuildResourceID(projectName, serviceName, username)) - return ResourcePGUserRead(ctx, d, client) - }) + // Retries 404 and password not received + return retry.Do( + func() error { + // ResourcePGUserRead resets id each time it gets 404, setting/restoring it here. + d.SetId(schemautil.BuildResourceID(projectName, serviceName, username)) + err := ResourcePGUserRead(ctx, d, client) + if err != nil { + return err + } + return validateUserPassword(d) + }, + retry.Context(ctx), + retry.Delay(time.Second), + retry.Attempts(schemautil.RetryNotFoundAttempts), + retry.RetryIf(func(err error) bool { + return avngen.IsNotFound(err) || errors.Is(err, errPasswordNotReceived) + }), + ) } func ResourcePGUserUpdate(ctx context.Context, d schemautil.ResourceData, client avngen.Client) error { @@ -148,35 +161,31 @@ func ResourcePGUserUpdate(ctx context.Context, d schemautil.ResourceData, client } } - return ResourcePGUserRead(ctx, d, client) -} - -func ResourcePGUserRead(ctx context.Context, d schemautil.ResourceData, client avngen.Client) error { - projectName, serviceName, username, err := schemautil.SplitResourceID3(d.Id()) - if err != nil { - return err - } - - // User password might be Null https://api.aiven.io/doc/#tag/Service/operation/ServiceUserGet - // > Account password. A null value indicates a user overridden password. - var user *service.ServiceUserGetOut - err = retry.Do( + // Retries password not received + return retry.Do( func() error { - user, err = client.ServiceUserGet(ctx, projectName, serviceName, username) + err := ResourcePGUserRead(ctx, d, client) if err != nil { - return retry.Unrecoverable(err) - } - // The field is not nullable, so we compare to an empty string - if user.Password == "" { - return fmt.Errorf("password is not received from the API") + return err } - return nil + return validateUserPassword(d) }, retry.Context(ctx), retry.Delay(time.Second), - retry.LastErrorOnly(true), // retry returns a list of errors by default + retry.Attempts(schemautil.RetryNotFoundAttempts), + retry.RetryIf(func(err error) bool { + return errors.Is(err, errPasswordNotReceived) + }), ) +} + +func ResourcePGUserRead(ctx context.Context, d schemautil.ResourceData, client avngen.Client) error { + projectName, serviceName, username, err := schemautil.SplitResourceID3(d.Id()) + if err != nil { + return err + } + user, err := client.ServiceUserGet(ctx, projectName, serviceName, username) if err != nil { return schemautil.ResourceReadHandleNotFound(err, d) } @@ -195,3 +204,15 @@ func ResourcePGUserRead(ctx context.Context, d schemautil.ResourceData, client a return nil } + +// errPasswordNotReceived password is not received from the API: +// 1. it was changed by TF but the API did not return it +// 2. user has changed it in PG directly, so the API does not have it +var errPasswordNotReceived = fmt.Errorf("password is not received from the API") + +func validateUserPassword(d schemautil.ResourceData) error { + if d.Get("password").(string) == "" { + return errPasswordNotReceived + } + return nil +} diff --git a/internal/sdkprovider/service/pg/pg_user_retries_test.go b/internal/sdkprovider/service/pg/pg_user_retries_test.go index dc1b4c96e..533f106ff 100644 --- a/internal/sdkprovider/service/pg/pg_user_retries_test.go +++ b/internal/sdkprovider/service/pg/pg_user_retries_test.go @@ -14,7 +14,7 @@ import ( "github.com/aiven/terraform-provider-aiven/mocks" ) -func TestRetriesErrors(t *testing.T) { +func TestCreateUpdateRetriesErrors(t *testing.T) { ctx := context.Background() client := mocks.NewMockClient(t) projectName := "foo" @@ -25,8 +25,8 @@ func TestRetriesErrors(t *testing.T) { d.EXPECT().Get("project").Return(projectName) d.EXPECT().Get("service_name").Return(serviceName) d.EXPECT().Get("username").Return(username) + d.EXPECT().Get("password").Return(password).Once() d.EXPECT().Get("pg_allow_replication").Return(true) - d.EXPECT().Get("password").Return(password) d.EXPECT().SetId("foo/bar/baz") d.EXPECT().Id().Return("foo/bar/baz") d.EXPECT().IsNewResource().Return(true) @@ -66,6 +66,7 @@ func TestRetriesErrors(t *testing.T) { ServiceUserGet(ctx, projectName, serviceName, username). Return(new(service.ServiceUserGetOut), nil). Repeatability = retryPasswordFails + d.EXPECT().Get("password").Return("").Times(retryPasswordFails) // Succeeds userOut := &service.ServiceUserGetOut{Password: password} @@ -73,6 +74,7 @@ func TestRetriesErrors(t *testing.T) { ServiceUserGet(ctx, projectName, serviceName, username). Return(userOut, nil). Once() + d.EXPECT().Get("password").Return(password).Once() // Sets lots of things, so doesn't matter what d.EXPECT().Set(mock.Anything, mock.Anything).Return(nil) @@ -84,3 +86,31 @@ func TestRetriesErrors(t *testing.T) { client.AssertNumberOfCalls(t, "ServiceUserCredentialsModify", 1) client.AssertNumberOfCalls(t, "ServiceUserGet", retry404Fails+retryPasswordFails+1) } + +// TestReadDoesNotRetryEmptyPassword +// When user resets user password in PG, the API returns an empty password, +// because the password is not stored in the BE. +// ReadContext must not retry empty password and the plan must show the diff. +func TestReadDoesNotRetryEmptyPassword(t *testing.T) { + ctx := context.Background() + client := mocks.NewMockClient(t) + projectName := "foo" + serviceName := "bar" + username := "baz" + d := mocks.NewMockResourceData(t) + d.EXPECT().Id().Return("foo/bar/baz") + + client.EXPECT(). + ServiceUserGet(ctx, projectName, serviceName, username). + Return(&service.ServiceUserGetOut{Username: username, Type: "normal"}, nil) + + d.EXPECT().Set("username", username).Return(nil) + d.EXPECT().Set("type", "normal").Return(nil) + d.EXPECT().Set("password", "").Return(nil) // Empty password! + + err := ResourcePGUserRead(ctx, d, client) + require.NoError(t, err) + + // No retries + client.AssertNumberOfCalls(t, "ServiceUserGet", 1) +}