Follow up 235

[fernandezcuesta]

Description of your changes

This is a follow up of #235 with some minor issues found while merging from master.

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

Added tests

[baburciu]

hey @fernandezcuesta, I think another issue stemming from the rebase and Crossplane v2 adjustment may be the fact that cluster variant of Grant reconciler is able to use the Database.spec.forProvider.database and only default to ProviderConfig.spec.defaultDatabase if not specified, here, but the namespaced variant is not, as it directly uses ProviderConfig's DefaultDatabase here.
For me I could only get

 apiVersion: postgresql.sql.crossplane.io/v1alpha1
  kind: Grant
  metadata:
    name: demo-application-staging-user-1
  spec:
    deletionPolicy: Delete
    forProvider:
      database: demo-application-staging-dedicated
      databaseRef:
        name: demo-application-staging-dedicated
      privileges:
        - SELECT
      role: demo-application-staging-user
      roleRef:
        name: demo-application-staging-user
      schema: public
      tables:
        - deployments
    managementPolicies:
      - '*'
    providerConfigRef:
      name: demo-application-staging-user-db-connection

to apply after adding the same behaviour to namespaced reconciler (this PR with these 2 commits)

demo-application-staging-dedicated=> SELECT table_schema, table_name, privilege_type
  FROM information_schema.table_privileges
  WHERE grantee = 'demo-application-staging-user';
 table_schema | table_name  | privilege_type
--------------+-------------+----------------
 public       | deployments | SELECT
(1 row)

demo-application-staging-dedicated=>

[fernandezcuesta]

hey @fernandezcuesta, I think another issue stemming from the rebase and Crossplane v2 adjustment may be the fact that cluster variant of Grant reconciler is able to use the Database.spec.forProvider.database and only default to ProviderConfig.spec.defaultDatabase if not specified, here, but the namespaced variant is not, as it directly uses ProviderConfig's DefaultDatabase here. For me I could only get

 apiVersion: postgresql.sql.crossplane.io/v1alpha1
  kind: Grant
  metadata:
    name: demo-application-staging-user-1
  spec:
    deletionPolicy: Delete
    forProvider:
      database: demo-application-staging-dedicated
      databaseRef:
        name: demo-application-staging-dedicated
      privileges:
        - SELECT
      role: demo-application-staging-user
      roleRef:
        name: demo-application-staging-user
      schema: public
      tables:
        - deployments
    managementPolicies:
      - '*'
    providerConfigRef:
      name: demo-application-staging-user-db-connection

to apply after adding the same behaviour to namespaced reconciler (this PR with these 2 commits)

demo-application-staging-dedicated=> SELECT table_schema, table_name, privilege_type
  FROM information_schema.table_privileges
  WHERE grantee = 'demo-application-staging-user';
 table_schema | table_name  | privilege_type
--------------+-------------+----------------
 public       | deployments | SELECT
(1 row)

demo-application-staging-dedicated=>

Thanks! added

[chlunde]

Hmm, this might break when new privs are added in the provider and running against an old server, or old provider-sql running against a new server. It would be nice to have e2e test coverage and also have a manual dispatch or env var override for e2e to run on a different major version of PostgreSQL?

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ecb0fd337

Maybe try on PG 16 and it will fail if we have e2e coverage here. Or we should state the minimum version is PG 17

[fernandezcuesta]

Did a best effort but not 100% confident it's the right approach

[dawidmalina]

@fernandezcuesta thanks for the effort to follow up on this. Is it possible to test this new build somewhere?
Eventually when we can expect this change to be ready?

[fernandezcuesta]

yeah let me finish with the PR review and I'll let you know

[fernandezcuesta]

@dawidmalina ghcr.io/crossplane-contrib/provider-sql:v0.15.0-rc.1

[dawidmalina]

@fernandezcuesta - no issues after switching to this version. Working as expected :) thank you