Skip to content

Report and retry failing default privileges of prepared schemas - #3235

Open
ritojoao wants to merge 1 commit into
zalando:masterfrom
ritojoao:fix/3230
Open

ritojoao wants to merge 1 commit into
zalando:masterfrom
ritojoao:fix/3230

Conversation

@ritojoao

Copy link
Copy Markdown

Hi, and thank you for maintaining the operator.

This is a small fix for #3230.

Problem: execCreateDatabaseSchema() ignored errors from execAlterSchemaDefaultPrivileges(), and syncPreparedSchemas() skipped schemas that already exist. If granting failed once (for example because a reader/writer role was missing), the sync reported no error and the privileges were never retried.

Change:

  • Return the errors of the default privilege statements from execCreateDatabaseSchema().
  • Run the schema statements for existing prepared schemas, too. CREATE SCHEMA IF NOT EXISTS and ALTER DEFAULT PRIVILEGES are idempotent, so missing privileges get fixed on the next sync.
  • Add a unit test with a fake database/sql driver covering the failure, the retry and the recovery.

One trade-off: default privileges are re-applied on every sync. I am happy to change the approach if you prefer another one.

Testing (golang:1.26.4 container):

  • gofmt -l on the changed files: clean
  • go generate ./... and go vet ./pkg/cluster/: OK
  • go test ./pkg/cluster/ -run TestPreparedSchemaPrivilegeErrors -v: PASS
  • go test ./pkg/cluster/: OK
  • with database.go and sync.go reverted to the base commit, TestPreparedSchemaPrivilegeErrors fails

I did not run the e2e suite.

Fixes #3230

execCreateDatabaseSchema ignored errors of execAlterSchemaDefaultPrivileges,
and syncPreparedSchemas skipped schemas that already exist, so missing
privileges were never retried. Return the errors and run the idempotent
create/privilege statements for existing schemas, too.

Fixes zalando#3230

Signed-off-by: João Rito <16895040+ritojoao@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prepared schema creation ignores grant errors and does not retry missing privileges

1 participant