diff --git a/pkg/clients/database/rds.go b/pkg/clients/database/rds.go index ec7207c280..abe73d4150 100644 --- a/pkg/clients/database/rds.go +++ b/pkg/clients/database/rds.go @@ -45,8 +45,51 @@ import ( const ( errGetPasswordSecretFailed = "cannot get password secret" + + // The condition CndtnPaswordSet is used to track whether the RDS master password has been + // set or not. + CndtnPaswordSet xpv1.ConditionType = "PasswordSet" + + // CndtnPaswordSetReasonPending indicates that CndtnPaswordSet is False because we haven't + // had the chance to set the password yet. e.g. After a restore it is some time, several minutes, + // before AWS will let you set the password. + CndtnPaswordSetReasonPending xpv1.ConditionReason = "Pending" + + // CndtnPaswordSetReasonSet indicates that CndtnPaswordSet is True because the + // password has been set. + CndtnPaswordSetReasonSet xpv1.ConditionReason = "PasswordSet" + + // CndtnPaswordSetReasonFailed indicates that CndtnPaswordSet is False because an error + // occurred after as password set attempt. + CndtnPaswordSetReasonFailed xpv1.ConditionReason = "Failed" ) +// Create a CndtnPaswordSet Condition with the specified values +func newPasswordSetCondition(sts corev1.ConditionStatus, rsn xpv1.ConditionReason, msg string) xpv1.Condition { + return xpv1.Condition{ + Type: CndtnPaswordSet, + Status: sts, + LastTransitionTime: metav1.Now(), + Reason: rsn, + Message: msg, + } +} + +// Create a CndtnPaswordSet Condition with false status, reason CndtnPaswordSetReasonPending and the provided message +func PasswordSetPending(msg string) xpv1.Condition { + return newPasswordSetCondition(corev1.ConditionFalse, CndtnPaswordSetReasonPending, msg) +} + +// Create a CndtnPaswordSet Condition with true status, reason CndtnPaswordSetReasonSet and the provided message +func PasswordSet(msg string) xpv1.Condition { + return newPasswordSetCondition(corev1.ConditionTrue, CndtnPaswordSetReasonSet, msg) +} + +// Create a CndtnPaswordSet Condition with fale status, reason CndtnPaswordSetReasonFailed and the error text in message +func PasswordSetFail(err error) xpv1.Condition { + return newPasswordSetCondition(corev1.ConditionFalse, CndtnPaswordSetReasonFailed, err.Error()) +} + // Client defines RDS RDSClient operations type Client interface { CreateDBInstance(context.Context, *rds.CreateDBInstanceInput, ...func(*rds.Options)) (*rds.CreateDBInstanceOutput, error) @@ -685,6 +728,15 @@ func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance, if err != nil { return false, "", err } + + if !pwdChanged { + // We test for != ConditionFalse because we don't want to disturb RDS steady state RDS created before + // this fix for https://github.com/crossplane-contrib/provider-aws/issues/1121 went in. + // False will only be set on those created after the fix went in or that had a failed password change + // attempt after the fix went in. ( And somehow the password setting didn't work of course ) + pwdChanged = r.Status.GetCondition(CndtnPaswordSet).Status == corev1.ConditionFalse + } + patch, err := CreatePatch(&db, &r.Spec.ForProvider) if err != nil { return false, "", err diff --git a/pkg/controller/database/rdsinstance/rdsinstance.go b/pkg/controller/database/rdsinstance/rdsinstance.go index af27ed5fb2..ce8881bcf5 100644 --- a/pkg/controller/database/rdsinstance/rdsinstance.go +++ b/pkg/controller/database/rdsinstance/rdsinstance.go @@ -31,6 +31,7 @@ import ( xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/connection" "github.com/crossplane/crossplane-runtime/pkg/controller" + xperrors "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/password" @@ -42,6 +43,7 @@ import ( awsclient "github.com/crossplane-contrib/provider-aws/pkg/clients" rds "github.com/crossplane-contrib/provider-aws/pkg/clients/database" "github.com/crossplane-contrib/provider-aws/pkg/features" + corev1 "k8s.io/api/core/v1" ) const ( @@ -170,7 +172,10 @@ func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext if !ok { return managed.ExternalCreation{}, errors.New(errNotRDSInstance) } - cr.SetConditions(xpv1.Creating()) + cr.Status.SetConditions(xpv1.Creating(), rds.PasswordSetPending("Creating")) + if err := e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return managed.ExternalCreation{}, err + } if cr.Status.AtProvider.DBInstanceStatus == v1beta1.RDSInstanceStateCreating { return managed.ExternalCreation{}, nil } @@ -205,6 +210,10 @@ func (e *external) RestoreOrCreate(ctx context.Context, cr *v1beta1.RDSInstance, if err != nil { return awsclient.Wrap(err, errCreateFailed) } + cr.Status.SetConditions(rds.PasswordSet("Set by CreateDBInstance")) + if err = e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return err + } return nil } @@ -230,7 +239,23 @@ func (e *external) RestoreOrCreate(ctx context.Context, cr *v1beta1.RDSInstance, default: return errors.New(errUnknownRestoreSource) } - return nil + + _, ret := e.client.ModifyDBInstance(ctx, &awsrds.ModifyDBInstanceInput{ + DBInstanceIdentifier: aws.String(meta.GetExternalName(cr)), + MasterUserPassword: aws.String(pw), + }) + + if ret == nil { + cr.Status.SetConditions(rds.PasswordSet("Password set via ModifyDBInstance in Create")) + } else { + cr.Status.SetConditions(rds.PasswordSetFail(ret)) + } + + if err := e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return xperrors.Join(ret, err) + } + + return ret } func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { // nolint:gocyclo @@ -262,16 +287,55 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext if err != nil { return managed.ExternalUpdate{}, err } + + // In this additional check we test for != ConditionFalse because we don't want to disturb RDS + // steady state RDS created before this fix for https://github.com/crossplane-contrib/provider-aws/issues/1121 + // was added. + // False will only be set on those created after the fix went in or that had a password change attempt after + // the fix went in. ( And somehow the password setting didn't work of course ) + if cr.Status.GetCondition(rds.CndtnPaswordSet).Status == corev1.ConditionFalse { + changed = true + } + if changed { + cr.Status.SetConditions(rds.PasswordSetPending("Updating")) + if err = e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return managed.ExternalUpdate{}, err + } + // In case of restore from snapshot we still might not have a set password when we get here. + // So like in CreateOrRestore we look for "" and generate a password if we see it. + if pwd == "" { + pwd, err = password.Generate() + if err != nil { + return managed.ExternalUpdate{}, err + } + } + } + if changed { conn = managed.ConnectionDetails{ xpv1.ResourceCredentialsSecretPasswordKey: []byte(pwd), + // If we're here after after restore from Snapshot then user name + // might not be set. + xpv1.ResourceCredentialsSecretUserKey: []byte(aws.ToString(cr.Spec.ForProvider.MasterUsername)), } modify.MasterUserPassword = aws.String(pwd) } if _, err = e.client.ModifyDBInstance(ctx, modify); err != nil { + if changed { + cr.Status.SetConditions(rds.PasswordSetFail(err)) + if errSts := e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return managed.ExternalUpdate{}, xperrors.Join(err, errSts) + } + } return managed.ExternalUpdate{}, awsclient.Wrap(err, errModifyFailed) } + if changed { + cr.Status.SetConditions(rds.PasswordSet("Password set via ModifyDBInstance in Update")) + if err = e.kube.Status().Update(ctx, cr); err != nil { // Why can't we just let controller-runtime take care of this? + return managed.ExternalUpdate{}, err + } + } if len(patch.Tags) > 0 { tags := make([]awsrdstypes.Tag, len(patch.Tags)) for i, t := range patch.Tags {