-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: Grant access to schema for role (#36) #37
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,13 @@ interface DatabaseUpdateProps extends DatabaseProps { | |
MasterOwner: string | ||
} | ||
|
||
interface SchemaProps { | ||
/** | ||
* Optional role is granted permissions. | ||
*/ | ||
RoleName?: string | ||
} | ||
|
||
const maxAttempts = 20 | ||
|
||
const jumpTable: JumpTable = { | ||
|
@@ -73,14 +80,58 @@ const jumpTable: JumpTable = { | |
}, | ||
}, | ||
schema: { | ||
Create: async (resourceId: string) => { | ||
return format("create schema if not exists %I", resourceId) | ||
Create: async (resourceId: string, props: SchemaProps) => { | ||
const sql: string[] = [format("create schema if not exists %I", resourceId)] | ||
if (props.RoleName) { | ||
sql.push(format("GRANT USAGE ON SCHEMA %I TO %I", resourceId, props.RoleName)) | ||
sql.push(format("GRANT CREATE ON SCHEMA %I TO %I", resourceId, props.RoleName)) | ||
sql.push( | ||
format("GRANT ALL ON ALL TABLES IN SCHEMA %I TO %I", resourceId, props.RoleName) | ||
) | ||
} | ||
return sql | ||
}, | ||
Update: async (resourceId: string, oldResourceId: string) => { | ||
return format("alter schema %I rename to %I", oldResourceId, resourceId) | ||
Update: async (resourceId: string, oldResourceId: string, props: SchemaProps) => { | ||
const sql: string[] = [] | ||
// TODO: revoke old role-name if props.RoleName was removed or changed | ||
if (props.RoleName) { | ||
sql.push( | ||
format( | ||
"REVOKE ALL ON ALL TABLES IN SCHEMA %I FROM %I", | ||
oldResourceId, | ||
props.RoleName | ||
) | ||
) | ||
sql.push( | ||
format("REVOKE CREATE ON SCHEMA %I FROM %FROM", oldResourceId, props.RoleName) | ||
) | ||
sql.push(format("REVOKE ALL ON SCHEMA %I FROM %I", oldResourceId, props.RoleName)) | ||
} | ||
sql.push(format("alter schema %I rename to %I", oldResourceId, resourceId)) | ||
if (props.RoleName) { | ||
sql.push(format("GRANT USAGE ON SCHEMA %I TO %I", resourceId, props.RoleName)) | ||
sql.push(format("GRANT CREATE ON SCHEMA %I TO %I", resourceId, props.RoleName)) | ||
sql.push( | ||
format("GRANT ALL ON ALL TABLES IN SCHEMA %I TO %I", resourceId, props.RoleName) | ||
) | ||
} | ||
return sql | ||
}, | ||
Delete: (resourceId: string) => { | ||
return format("drop schema if exists %I cascade", resourceId) | ||
Delete: (resourceId: string, props: SchemaProps) => { | ||
const sql: string[] = [] | ||
if (props.RoleName) { | ||
sql.push( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're repeating yourself a lot, so please put the grant + revoke schema into their own function, so you can call that instead of copying the same code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're doing the same in may other places: https://github.com/berenddeboer/cdk-rds-sql/blob/main/src/handler.ts#L100 and https://github.com/berenddeboer/cdk-rds-sql/blob/main/src/handler.ts#L187 and https://github.com/berenddeboer/cdk-rds-sql/blob/main/src/handler.ts#L211 However, I moved the statements into functions. |
||
format( | ||
"REVOKE ALL ON ALL TABLES IN SCHEMA %I FROM %I", | ||
resourceId, | ||
props.RoleName | ||
) | ||
) | ||
sql.push(format("REVOKE CREATE ON SCHEMA %I FROM %I", resourceId, props.RoleName)) | ||
sql.push(format("REVOKE ALL ON SCHEMA %I FROM %I", resourceId, props.RoleName)) | ||
} | ||
sql.push(format("drop schema if exists %I cascade", resourceId)) | ||
return sql | ||
}, | ||
}, | ||
role: { | ||
|
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.
Yeah, that needs doing (and be tested).
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.
You did those notes as well:
https://github.com/berenddeboer/cdk-rds-sql/blob/main/src/handler.ts#L118
https://github.com/berenddeboer/cdk-rds-sql/blob/main/src/handler.ts#L160
If the handler would be prepared to get old props as well I would have done this check. It is out of the scope of this little change to refactor the handler.