diff --git a/pkg/controller/cluster/postgresql/role/reconciler.go b/pkg/controller/cluster/postgresql/role/reconciler.go index 5528967d..edb6bd0a 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler.go +++ b/pkg/controller/cluster/postgresql/role/reconciler.go @@ -436,35 +436,10 @@ func (c *external) Delete(ctx context.Context, mg *v1alpha1.Role) (managed.Exter } func upToDate(observed *v1alpha1.RoleParameters, desired *v1alpha1.RoleParameters) bool { - if observed.ConnectionLimit != desired.ConnectionLimit { - return false - } - if observed.Privileges.SuperUser != desired.Privileges.SuperUser { - return false - } - if observed.Privileges.Inherit != desired.Privileges.Inherit { - return false - } - if observed.Privileges.CreateDb != desired.Privileges.CreateDb { - return false - } - if observed.Privileges.CreateRole != desired.Privileges.CreateRole { - return false - } - if observed.Privileges.Login != desired.Privileges.Login { - return false - } - if observed.Privileges.Replication != desired.Privileges.Replication { - return false - } - if observed.Privileges.BypassRls != desired.Privileges.BypassRls { - return false - } - if !cmp.Equal(observed.ConfigurationParameters, desired.ConfigurationParameters, - cmpopts.SortSlices(func(o, d v1alpha1.RoleConfigurationParameter) bool { return o.Name < d.Name })) { - return false - } - return true + return cmp.Equal(observed, desired, + cmpopts.IgnoreFields(v1alpha1.RoleParameters{}, "PasswordSecretRef", "PasswordRotationTrigger"), + cmpopts.SortSlices(func(o, d v1alpha1.RoleConfigurationParameter) bool { return o.Name < d.Name }), + ) } func lateInit(observed *v1alpha1.RoleParameters, desired *v1alpha1.RoleParameters) bool { diff --git a/pkg/controller/cluster/postgresql/role/reconciler_test.go b/pkg/controller/cluster/postgresql/role/reconciler_test.go index afd1f63e..4d5fb97d 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/role/reconciler_test.go @@ -278,6 +278,64 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SameValuesDifferentPointers": { + reason: "We should return ResourceUpToDate=true when the observed values equal the spec through different pointers", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *(dest[7].(**int32)) = ptr.To(int32(-1)) + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + Privileges: v1alpha1.RolePrivilege{}, + ConnectionLimit: ptr.To(int32(-1)), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + }, + err: nil, + }, + }, + "ConnectionLimitChanged": { + reason: "We should return ResourceUpToDate=false if the connection limit differs from the observed one", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *(dest[7].(**int32)) = ptr.To(int32(-1)) + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + Privileges: v1alpha1.RolePrivilege{}, + ConnectionLimit: ptr.To(int32(5)), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + }, + err: nil, + }, + }, "PasswordChanged": { reason: "We should return ResourceUpToDate=false if the password changed", fields: fields{ diff --git a/pkg/controller/namespaced/postgresql/role/reconciler.go b/pkg/controller/namespaced/postgresql/role/reconciler.go index 7f3b4044..d6cf1503 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler.go @@ -419,35 +419,10 @@ func (c *external) Delete(ctx context.Context, mg *namespacedv1alpha1.Role) (man } func upToDate(observed *namespacedv1alpha1.RoleParameters, desired *namespacedv1alpha1.RoleParameters) bool { - if observed.ConnectionLimit != desired.ConnectionLimit { - return false - } - if observed.Privileges.SuperUser != desired.Privileges.SuperUser { - return false - } - if observed.Privileges.Inherit != desired.Privileges.Inherit { - return false - } - if observed.Privileges.CreateDb != desired.Privileges.CreateDb { - return false - } - if observed.Privileges.CreateRole != desired.Privileges.CreateRole { - return false - } - if observed.Privileges.Login != desired.Privileges.Login { - return false - } - if observed.Privileges.Replication != desired.Privileges.Replication { - return false - } - if observed.Privileges.BypassRls != desired.Privileges.BypassRls { - return false - } - if !cmp.Equal(observed.ConfigurationParameters, desired.ConfigurationParameters, - cmpopts.SortSlices(func(o, d namespacedv1alpha1.RoleConfigurationParameter) bool { return o.Name < d.Name })) { - return false - } - return true + return cmp.Equal(observed, desired, + cmpopts.IgnoreFields(namespacedv1alpha1.RoleParameters{}, "PasswordSecretRef", "PasswordRotationTrigger"), + cmpopts.SortSlices(func(o, d namespacedv1alpha1.RoleConfigurationParameter) bool { return o.Name < d.Name }), + ) } func lateInit(observed *namespacedv1alpha1.RoleParameters, desired *namespacedv1alpha1.RoleParameters) bool { diff --git a/pkg/controller/namespaced/postgresql/role/reconciler_test.go b/pkg/controller/namespaced/postgresql/role/reconciler_test.go index cca25719..d5dd1d55 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler_test.go @@ -335,6 +335,74 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SameValuesDifferentPointers": { + reason: "We should return ResourceUpToDate=true when the observed values equal the spec through different pointers", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *(dest[7].(**int32)) = ptr.To(int32(-1)) + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + Privileges: v1alpha1.RolePrivilege{}, + ConnectionLimit: ptr.To(int32(-1)), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordChange: &metav1.Time{Time: metav1.Now().Time}, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + }, + err: nil, + }, + }, + "ConnectionLimitChanged": { + reason: "We should return ResourceUpToDate=false if the connection limit differs from the observed one", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *(dest[7].(**int32)) = ptr.To(int32(-1)) + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + Privileges: v1alpha1.RolePrivilege{}, + ConnectionLimit: ptr.To(int32(5)), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordChange: &metav1.Time{Time: metav1.Now().Time}, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + }, + err: nil, + }, + }, "PasswordChanged": { reason: "We should return ResourceUpToDate=false if the password changed", fields: fields{