[blacksoxx]

Description of your changes

upToDate in the PostgreSQL Role controllers compared ConnectionLimit and the seven privilege fields as pointers. lateInit copies the observed pointers into the spec once, but every later reconcile arrives with fresh pointers from the API server, so an unchanged Role was never up to date and Update re-issued ALTER ROLE ... CONNECTION LIMIT on every poll, which is what breaks RDS Blue/Green deployments in #487. The fields are now compared by value with cmp.Equal, in the cluster and the namespaced controller.

Fixes #487

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Two TestObserve cases in each controller: the observed limit and the spec limit equal through different pointers now report ResourceUpToDate: true, and a different limit still reports false. The first case fails on master. make reviewable passes (generate, tidy, golangci-lint with 0 issues, unit tests).

[jmerciercode]

+1, we hit the same thing on v0.15.0 (AWS RDS PostgreSQL), and the fix in #488 / #468 matches what we found.

We noticed it after enabling pgaudit (pgaudit.log = ddl,role). On one instance, each of the 43 managed roles gets exactly 6 ALTER ROLE "<role>" CONNECTION LIMIT <n> per hour, one per 10m poll. That's about 7,000 audit lines per day per instance, and it buries real role/privilege changes in the ROLE audit class. Our other instances show the same pattern.

It also hits roles that set neither connectionLimit nor a password in the spec: late-init fills connectionLimit: -1, and from then on the role gets CONNECTION LIMIT -1 on every poll. Each poll emits an UpdatedExternalResource event, even though the Role is Ready/Synced and spec.forProvider.privileges matches status.atProvider.privilegesAsClauses. pgaudit shows only the CONNECTION LIMIT statement, never PASSWORD or privilege clauses, which fits Update running with an empty privilege diff and an unconditional connection-limit step.

We'd be glad to see this merged and released.

[fernandezcuesta]

I feel it's better to do cmp.Equal in observed vs desired with IgnoreFields (the ones that Observe never fills) plus SortSlices to avoid spurious diffs:

func upToDate(observed *v1alpha1.RoleParameters, desired *v1alpha1.RoleParameters) bool {
	return cmp.Equal(
		observed,
		desired,
		cmpopts.IgnoreFields(v1alpha1.RoleParameters{}, "PasswordSecretRef", "PasswordRotationTrigger"),
		cmpopts.SortSlices(func(o, d v1alpha1.RoleConfigurationParameter) bool { return o.Name < d.Name })
	)
}

[fernandezcuesta]

but IIRC this will be implemented from crossplane/crossplane-runtime (see http://github.com/crossplane/crossplane-runtime/pull/945)

[blacksoxx]

Thanks, that is cleaner. Done in 265791d for both controllers, the two TestObserve cases still cover it.

On #945: as I read it, it adds an UpToDate condition that surfaces whatever Observe returns in ResourceUpToDate, so the comparison itself stays in the provider and this fix is still needed for it to report true.

[fernandezcuesta]

same