Skip to content

CXH-1396: add Amazon Redshift example#125

Merged
al-conductorone merged 3 commits into
mainfrom
cxh-1396-redshift-example
May 21, 2026
Merged

CXH-1396: add Amazon Redshift example#125
al-conductorone merged 3 commits into
mainfrom
cxh-1396-redshift-example

Conversation

@al-conductorone
Copy link
Copy Markdown
Contributor

Adds an Amazon Redshift configuration on top of #124. Customers can sync the entire cluster (users, groups, roles, schemas, tables) and grant or revoke schema-level and table-level access through ConductorOne.

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 11, 2026

CXH-1396

@al-conductorone al-conductorone requested a review from btipling May 11, 2026 20:29
@al-conductorone al-conductorone requested review from a team May 11, 2026 20:29
@al-conductorone al-conductorone force-pushed the cxh-1396-redshift-example branch 3 times, most recently from a170a92 to 9d20148 Compare May 11, 2026 20:44
Comment thread examples/redshift-test.yml Outdated
display_name: "resource.DisplayName + ' member'"
description: "'Membership in Redshift role ' + resource.DisplayName"
purpose: assignment
grantable_to: [user, role]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C1 only grants to users, and the query is wrong for granting to role
TO { username [ WITH GRANT OPTION ] | ROLE role_name | GROUP group_name | PUBLIC } [, ...]

TO ROLE
TO GROUP

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Comment thread examples/redshift-test.yml Outdated
display_name: "resource.DisplayName + ' CONNECT'"
description: "'CONNECT on database ' + resource.DisplayName"
purpose: permission
grantable_to: [user, role, group]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the same problem, and elsewhere we should only grant to user or fix the queries.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Base automatically changed from cxh-1396-multi-database-iteration to main May 19, 2026 16:14
Comment on lines +266 to +269
grantee: "principal.ID"
grant:
no_transaction: true
queries:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: The database grants query runs per-database (no scope: cluster), but svv_database_privileges likely returns cluster-wide results regardless of which database you're connected to. This would produce duplicate grants — the same CONNECT row emitted once per database iteration. Consider adding scope: cluster here to match the database list query.

# Read-only: https://docs.aws.amazon.com/redshift/latest/dg/r_GRANT.html lists CREATE/TEMP/USAGE/ALTER as the
# database-level grantable privileges, not CONNECT. svv_database_privileges
# still surfaces CONNECT rows, so visibility works, but a GRANT CONNECT path
# is not documented and is left out until validated.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: grantable_to: [user] means role-to-role grants (synced via the svv_role_grants block below) are visible but not provisionable. If granting a role to another role should be supported, add role to the list: grantable_to: [user, role].

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

Connector PR Review: CXH-1396: add Amazon Redshift example

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Review mode: full
View review run

Review Summary

Full review of new commits since 16f2613. The new changes add a doc comment to DatabasesConfig in config.go, expand the iterateDBs comment in multidb.go, and apply minor README formatting tweaks (em-dashes to colons). No security or correctness issues. The two suggestions from the prior review are still applicable and unaddressed.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • examples/redshift-test.yml:266-269 — Database grants query lacks scope: cluster; may produce duplicate CONNECT grants in multi-database mode if svv_database_privileges returns cluster-wide results.
  • examples/redshift-test.yml:200 — Role entitlement grantable_to: [user] excludes role-to-role grant provisioning despite syncing those grants.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `examples/redshift-test.yml`:
- Around line 266-269: The `database` grants block runs per-database (no `scope` field) but `svv_database_privileges` likely returns the same cluster-wide rows from every database. Add `scope: cluster` to the grants block to avoid emitting duplicate CONNECT grants.
- Around line 200: The role `member` entitlement has `grantable_to: [user]` but the second grants block syncs role-to-role assignments from `svv_role_grants`. If provisioning role-to-role grants is desired, change to `grantable_to: [user, role]`.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

C1 only provisions grants to user grantees, and the existing GRANT/REVOKE
queries assume a username (no TO ROLE / TO GROUP keyword). Listing role or
group in grantable_to advertised provisioning paths the SQL could not
actually execute. Restrict every static_entitlements grantable_to to [user]
across role, database, schema, and table entitlements. Grant-discovery
blocks still emit role/group principals where the privilege views report
them; that is state reporting, not provisioning.
@al-conductorone al-conductorone force-pushed the cxh-1396-redshift-example branch from 16f2613 to dc14fe3 Compare May 19, 2026 19:07
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@btipling btipling removed their assignment May 20, 2026
@al-conductorone al-conductorone merged commit fdcd268 into main May 21, 2026
9 checks passed
@al-conductorone al-conductorone deleted the cxh-1396-redshift-example branch May 21, 2026 13:31
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.

2 participants