-
Notifications
You must be signed in to change notification settings - Fork 2
CXH-2417: extend DDL grant/revoke idempotency to Oracle behind a per-config opt-in #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
al-conductorone
merged 5 commits into
main
from
cxh-2417-baton-sql-extend-ddl-grantrevoke-idempotency-to-oracle
Sep 11, 2026
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
dc46a6d
gate ddl no-rows idempotency behind per-config opt-in for oracle and db2
al-conductorone 0b6f229
CXH-2417: add Oracle full-path idempotency tests; note admin non-idem…
al-conductorone 24d82c7
CXH-2417: address #152 review round (Db2 default-on, Oracle opt-in ga…
al-conductorone 187057b
CXH-2417: address review feedback on grant/revoke idempotency
al-conductorone bf1d301
CXH-2417: docs: no_transaction is required on Oracle-with-flag too, n…
al-conductorone File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # Oracle | ||
|
|
||
| baton-sql talks to Oracle through the pure-Go `go-ora` driver, so no native client is needed | ||
| (unlike Db2). Connect with an `oracle://` DSN: | ||
|
|
||
| ```yaml | ||
| connect: | ||
| dsn: "oracle://${DB_HOST}:${DB_PORT}/${DB_SERVICE}" | ||
| user: "${DB_USER}" | ||
| password: "${DB_PASSWORD}" | ||
| ``` | ||
|
|
||
| See [examples/oracle-test.yml](../examples/oracle-test.yml) for a full config (users, roles, | ||
| privileges, account provisioning, enable/disable/update actions). | ||
|
|
||
| ## Idempotent grant / revoke: `validation_queries_signal_idempotency` | ||
|
|
||
| Oracle applies `GRANT`/`REVOKE` of roles and privileges as DDL that does not report | ||
| rows-affected, and re-running an already-applied statement raises an error. A repeat revoke of a | ||
| role the user no longer has fails with `ORA-01951: ROLE '...' not granted to '...'`. So a resync | ||
| that re-issues a revoke, or a grant of a role the user already holds, would surface as a failure | ||
| even though nothing needs to change. | ||
|
|
||
| To make grant and revoke idempotent, set `validation_queries_signal_idempotency: true` on the | ||
| grant or revoke and add a `validation_query` that answers **"is there work to do?"**. When the | ||
| query returns no rows, the connector reports an idempotent success (`GrantAlreadyExists` on grant, | ||
| `GrantAlreadyRevoked` on revoke) instead of running the DDL and hitting the error. | ||
|
|
||
| ```yaml | ||
| grant: | ||
| no_transaction: true | ||
| validation_queries_signal_idempotency: true | ||
| # returns a row only while the role is NOT yet granted (no rows => already granted) | ||
| # UPPER-normalize the principal: the GRANT inserts it |unquoted, so Oracle folds it to | ||
| # upper-case (CREATE USER jdoe => JDOE); DBA_ROLES roles are already upper-case | ||
| validation_queries: | ||
| - | | ||
| SELECT 1 FROM dual WHERE NOT EXISTS ( | ||
| SELECT 1 FROM DBA_ROLE_PRIVS | ||
| WHERE GRANTEE = UPPER(?<principal_name>) AND GRANTED_ROLE = ?<role_name> | ||
| ) | ||
| queries: | ||
| - GRANT ?<role_name|identifier> TO ?<principal_name|unquoted> | ||
| revoke: | ||
| no_transaction: true | ||
| validation_queries_signal_idempotency: true | ||
| # returns a row only while the role IS still granted (no rows => already revoked) | ||
| validation_queries: | ||
| - | | ||
| SELECT 1 FROM DBA_ROLE_PRIVS | ||
| WHERE GRANTEE = UPPER(?<principal_name>) AND GRANTED_ROLE = ?<role_name> | ||
| queries: | ||
| - REVOKE ?<role_name|identifier> FROM ?<principal_name|unquoted> | ||
| ``` | ||
|
|
||
| Use `|identifier` for role names so they are engine-quoted; they come from `DBA_ROLES` | ||
| already upper-cased. Insert the principal `|unquoted` so Oracle folds it to upper-case, and | ||
| compare it with `UPPER(?<principal_name>)` in the validation query so a lower-case | ||
| `principal.ID` still matches the stored `GRANTEE`. System-privilege entitlements are different: | ||
| privilege names are multiword keywords like `CREATE SESSION`, so their DDL operand must use | ||
| `|keyword`, not `|identifier` (quoting would break the clause and `|unquoted` would strip the | ||
| space): | ||
|
|
||
| ```yaml | ||
| queries: | ||
| - GRANT ?<privilege_name|keyword> TO ?<principal_name|unquoted> | ||
| ``` | ||
|
|
||
| The flag is off by default, so without it Oracle keeps failing loudly on the repeat operation. | ||
| The same warning as Db2 applies: with the flag on, do **not** use `validation_queries` as | ||
| existence preconditions, since a no-rows result is swallowed as an idempotent success. See | ||
| [provisioning.md](provisioning.md) for the full explanation. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,37 +1,60 @@ | ||
| # Provisioning: `validation_queries` semantics | ||
|
|
||
| `validation_queries` run before the provisioning `queries` in a grant or revoke. What a | ||
| **no-rows** result means depends on the engine. | ||
| **no-rows** result means depends on the engine: on the DDL engines (Db2, Oracle) it can mean an | ||
| idempotent success (on by default for Db2, opt-in for Oracle); on every other engine it fails the | ||
| operation. | ||
|
|
||
| ## Default: no rows fails the operation | ||
|
|
||
| On every engine except Db2, a `validation_query` returning no rows **fails the operation**. | ||
| It is an existence precondition that aborts loudly. This includes the DDL engines that don't | ||
| report rows-affected (Oracle): treating their no-rows as idempotency is a follow-up that needs | ||
| a per-config opt-in first, so today they still fail loudly like everyone else. | ||
|
|
||
| ## Db2 (opt-in behind the `db2` build tag) | ||
|
|
||
| Db2 applies `GRANT`/`REVOKE` as DDL that does not report rows-affected, so the connector | ||
| cannot tell from the statement itself whether it changed anything, and an already-applied | ||
| statement raises an error. To make grant and revoke idempotent, a `validation_query` | ||
| returning no rows is reported as an **idempotent success** (`GrantAlreadyExists` on grant, | ||
| `GrantAlreadyRevoked` on revoke). No rows means "the state is already as desired, there is no | ||
| work to do". Db2 ships opt-in behind the `db2` build tag, so no default-build engine changes | ||
| behavior. | ||
|
|
||
| Because of this, on Db2 your `validation_queries` must answer **"is there work to do?"**, not | ||
| **"does this principal or role exist?"**. | ||
|
|
||
| **Do not use `validation_queries` as existence preconditions on Db2.** A no-rows result is | ||
| swallowed as idempotent success, so a missing, deleted, or mistyped principal or role is | ||
| reported as "already done" instead of erroring. For example, a validation query like | ||
| `SELECT 1 FROM users WHERE name = ?<user_id>` will silently mask a bad `user_id`: it returns | ||
| no rows, and the grant is reported as `GrantAlreadyExists` even though nothing was granted. | ||
| By default a `validation_query` returning no rows **fails the operation**. It is an existence | ||
| precondition that aborts loudly. This is the behavior on every engine except where the DDL | ||
| no-rows-means-idempotent behavior below is active. | ||
|
|
||
| ## DDL engines: no rows means idempotent success | ||
|
|
||
| The DDL engines (Db2, Oracle) apply `GRANT`/`REVOKE` as DDL that does not report rows-affected, so | ||
| the connector cannot tell from the statement whether it changed anything, and re-running an | ||
| already-applied statement raises an error (Oracle `ORA-01951` on a repeat revoke, for example). On | ||
| these engines a `validation_query` returning no rows is reported as an idempotent success. This is | ||
| on by **default** for Db2; on Oracle you opt in per entitlement with | ||
| `validation_queries_signal_idempotency: true` on the grant or revoke: | ||
|
|
||
| ```yaml | ||
| grant: | ||
| validation_queries_signal_idempotency: true | ||
| validation_queries: | ||
| - SELECT 1 FROM ... # returns a row only while the grant is MISSING | ||
| queries: | ||
| - GRANT ... | ||
| revoke: | ||
| validation_queries_signal_idempotency: true | ||
| validation_queries: | ||
| - SELECT 1 FROM ... # returns a row only while the grant is PRESENT | ||
| queries: | ||
| - REVOKE ... | ||
| ``` | ||
|
|
||
| On these engines a `validation_query` returning no rows is reported as an **idempotent success** | ||
| (`GrantAlreadyExists` on grant, `GrantAlreadyRevoked` on revoke): no rows means "the state is | ||
| already as desired, there is no work to do". Wherever this reinterpretation is active (Db2 always, | ||
| Oracle when the flag is on), the grant/revoke must also set `no_transaction: true`, since Db2 and | ||
| Oracle report no rows-affected under a transaction. | ||
|
|
||
| ## Writing the query: "is there work to do?", not "does this exist?" | ||
|
|
||
| On the DDL engines (Db2, Oracle), your `validation_queries` must answer **"is there work to do?"**, | ||
| not **"does this principal or role exist?"**. | ||
|
|
||
| **Do not use them as existence preconditions.** A no-rows result is swallowed as idempotent | ||
| success, so a missing, deleted, or mistyped principal or role is reported as "already done" | ||
| instead of erroring. For example, a query like `SELECT 1 FROM users WHERE name = ?<user_id>` | ||
| silently masks a bad `user_id`: it returns no rows, and the grant is reported as | ||
| `GrantAlreadyExists` even though nothing was granted. | ||
|
|
||
| Write the query so no-rows genuinely means idempotent. For a grant, check whether the target | ||
| membership is **missing** (no rows => already granted); for a revoke, check whether it is | ||
| **present** (no rows => already revoked). | ||
|
|
||
| This mirrors the warning on `EntitlementProvisioningQueries.ValidationQueries` in | ||
| `pkg/bsql/config.go`. | ||
| `pkg/bsql/config.go`. See [oracle.md](oracle.md) and [db2.md](db2.md) for engine-specific notes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion:
|unquotedruns the value throughSanitizeIdentifier(pkg/bsql/query.go:54-58), which drops every char outside[A-Za-z0-9_]— including$and#, both legal in Oracle usernames (e.g. the CDB common-user prefixC##ADMIN). The validation query binds the rawUPPER(?<principal_name>), so for such a principal the check passes againstC##ADMINwhile the DDL emitsGRANT ... TO CADMIN, which either fails with ORA-01917 or, if aCADMINuser exists, grants to the wrong principal. Consider keeping|identifierand upper-casingprincipal_nameinvarsinstead, or documenting the special-character limitation. Same pattern inexamples/oracle-test.yml(grant/revoke around lines 333, 341, 354-362 and the privilege blocks).