fix(postgresql): make schema optional and conditionally required in DefaultPrivileges

[AGejr]

Description of your changes

Fixes objectType: schema in DefaultPrivileges, which has never produced valid SQL. Previously, the controller unconditionally appended IN SCHEMA <name> to the generated query, but PostgreSQL does not permit IN SCHEMA when the object type is SCHEMAS. Any resource using objectType: schema would immediately and permanently fail with:

cannot create default privileges: pq: cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS

This PR fixes the issue without removing schema from the enum (so the valid use case - granting privileges on future schemas remains supported.

• Makes spec.forProvider.schema optional in both the cluster-scoped and namespaced CRDs (previously it was required, making objectType: schema impossible to use correctly)
• Adds controller-side validation in Observe enforcing the conditional constraint:
  • objectType is table, sequence, function, or type -> schema is required
  • objectType is schema -> schema must not be set
• Fixes inSchema to never emit IN SCHEMA when objectType is schema
• Expands examples to cover all objectType variants, with the schema variant correctly omitting the schema field

Fixes #378

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

Unit tests cover both new validation paths: schema required when objectType is not schema, and schema rejected when objectType is schema. Full suite passes with go test ./...

Original description

Removes schema from the valid objectType enum in the DefaultPrivileges CRD spec (both postgresql.sql.crossplane.io and postgresql.sql.m.crossplane.io variants), as objectType: schema has never worked - the provider unconditionally includes an IN SCHEMA clause in the generated SQL, but PostgreSQL does not allow IN SCHEMA when the object type is SCHEMAS, causing any resource using this value to immediately and permanently fail with cannot create default privileges: pq: cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS. This is technically a breaking change to the CRD, but since objectType: schema has never produced valid SQL, no working resources can exist that rely on it (as far as I can tell).

Fixes #378

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

The change removes an enum value - no new code paths are introduced. Existing unit tests pass with go test ./...

[fernandezcuesta]

Removing schema from the enum would prevent users from setting up automatic schema-level privilege grants, which is a legitimate multi-tenant/team usecase.

Without becoming a breaking change, you can fix it with (reconciler.go, both cluster+namespace):

-  if params.Schema != nil {
+  // PostgreSQL does not allow IN SCHEMA with ON SCHEMAS.
+  if params.Schema != nil && (params.ObjectType == nil || *params.ObjectType != "schema") {

And ideally validate it with a test 🙏

[fernandezcuesta]

i.e. ALTER DEFAULT PRIVILEGES FOR ROLE foo GRANT CREATE ON SCHEMAS TO bar

[AGejr]

My original use case was exactly what you describe. However, since spec.forProvider.schema is a required field in the CRD, createDefaultPrivilegesQuery will always call inSchema, producing a statement like:

ALTER DEFAULT PRIVILEGES FOR ROLE foo IN SCHEMA public GRANT USAGE ON SCHEMAS TO bar;

This is invalid SQL.

Your suggested guard would only help if schema were optional, but since it's required, params.Schema is never nil and the guard becomes a no-op.

I think the correct fix is to make spec.forProvider.schema optional and enforce a conditional constraint, since the field maps to the IN SCHEMA PostgreSQL clause:

• objectType is table, sequence, function, or type -> schema is required (IN SCHEMA is meaningful, as these objects live inside schemas)
• objectType is schema -> schema must not be set (IN SCHEMA is semantically inapplicable at the PostgreSQL level, since schemas don't live inside other schemas)

I'm happy to rework the PR along those lines, with tests, if that approach makes sense.

[fernandezcuesta]

@AGejr yes, it makes absolute sense to me

[AGejr]

Reworked the PR along the lines we discussed.

• Made spec.forProvider.schema optional in both the cluster and namespaced CRDs
  • Added runtime validation in Observe enforcing the conditional constraint:
  • objectType is table, sequence, function, or type -> schema is required
  • objectType is schema -> schema must not be set (returns an error if it is)
• Fixed inSchema to never emit IN SCHEMA when objectType is schema
• Added test cases covering both validation paths and verified the full test suite passes
• Expanded the examples to include all objectType variants, with schema objectType examples omitting the schema field

[Copilot]

Pull request overview

This PR fixes PostgreSQL DefaultPrivileges reconciliation for objectType: schema by ensuring the controller does not generate an IN SCHEMA ... clause for schema default privileges, and it updates the API/docs/tests/examples to treat schema as conditionally applicable.

Changes:

• Adjust SQL generation so objectType: schema produces ... GRANT ... ON SCHEMAS ... without IN SCHEMA.
• Make schema optional in the CRD/API types and add controller-side validation that schema is required for non-schema objectTypes and must be omitted for objectType: schema.
• Expand unit tests and examples to cover objectType: schema and align objectType casing with the CRD enum.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/controller/namespaced/postgresql/default_privileges/reconciler.go	Omits IN SCHEMA for objectType: schema, uppercases SQL object type, and adds schema/objectType validation in Observe.
pkg/controller/namespaced/postgresql/default_privileges/reconciler_test.go	Updates tests for lowercase objectType, adds schema where required, and adds coverage for objectType: schema.
pkg/controller/cluster/postgresql/default_privileges/reconciler.go	Same as namespaced controller changes for cluster-scoped resources.
pkg/controller/cluster/postgresql/default_privileges/reconciler_test.go	Same as namespaced test updates for cluster-scoped resources.
package/crds/postgresql.sql.m.crossplane.io_defaultprivileges.yaml	Makes schema optional in the CRD and clarifies schema applicability in description.
package/crds/postgresql.sql.crossplane.io_defaultprivileges.yaml	Makes schema optional in the CRD and clarifies schema applicability in description.
examples/namespaced/postgresql/defaultprivileges.yaml	Adds example manifests covering additional object types including schema without schema: field.
examples/cluster/postgresql/defaultprivileges.yaml	Adds cluster-scoped example manifests covering additional object types including schema without schema: field.
apis/namespaced/postgresql/v1alpha1/default_privileges_types.go	Updates API type docs and makes schema optional.
apis/cluster/postgresql/v1alpha1/default_privileges_types.go	Updates API type docs and makes schema optional.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.