Skip to content

Commit

Permalink
fix(glue): remove batchDeletePartition from grantRead() permissio…
Browse files Browse the repository at this point in the history
…ns (#17941)

It is convention in the CDK to expose the underlying `grant()` API to make it simple for users to grant custom permissions to their resource. 

In addition, this PR removes 'glue:BatchDeletePartition' from `readPermissions`, which was previously erroneously added.

closes #17935 and #15116.

BREAKING CHANGE: the grantRead API previously included 'glue:BatchDeletePartition', and now it does not.


 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Dec 10, 2021
1 parent 3a9f206 commit 3d64f9b
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 35 deletions.
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-glue/lib/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,10 @@ export class Table extends Resource implements ITable {
return ret;
}

private grant(grantee: iam.IGrantable, actions: string[]) {
/**
* Grant the given identity custom permissions.
*/
public grant(grantee: iam.IGrantable, actions: string[]) {
return iam.Grant.addToPrincipal({
grantee,
resourceArns: [this.tableArn],
Expand Down Expand Up @@ -400,7 +403,6 @@ function createBucket(table: Table, props: TableProps) {
}

const readPermissions = [
'glue:BatchDeletePartition',
'glue:BatchGetPartition',
'glue:GetPartition',
'glue:GetPartitions',
Expand Down
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-glue/test/integ.connection.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@
"VpcPublicSubnet1NATGateway4D7517AA": {
"Type": "AWS::EC2::NatGateway",
"Properties": {
"SubnetId": {
"Ref": "VpcPublicSubnet1Subnet5C2D37C4"
},
"AllocationId": {
"Fn::GetAtt": [
"VpcPublicSubnet1EIPD7E02669",
"AllocationId"
]
},
"SubnetId": {
"Ref": "VpcPublicSubnet1Subnet5C2D37C4"
},
"Tags": [
{
"Key": "Name",
Expand Down Expand Up @@ -192,15 +192,15 @@
"VpcPublicSubnet2NATGateway9182C01D": {
"Type": "AWS::EC2::NatGateway",
"Properties": {
"SubnetId": {
"Ref": "VpcPublicSubnet2Subnet691E08A3"
},
"AllocationId": {
"Fn::GetAtt": [
"VpcPublicSubnet2EIP3C605A87",
"AllocationId"
]
},
"SubnetId": {
"Ref": "VpcPublicSubnet2Subnet691E08A3"
},
"Tags": [
{
"Key": "Name",
Expand Down Expand Up @@ -289,15 +289,15 @@
"VpcPublicSubnet3NATGateway7640CD1D": {
"Type": "AWS::EC2::NatGateway",
"Properties": {
"SubnetId": {
"Ref": "VpcPublicSubnet3SubnetBE12F0B6"
},
"AllocationId": {
"Fn::GetAtt": [
"VpcPublicSubnet3EIP3A666A23",
"AllocationId"
]
},
"SubnetId": {
"Ref": "VpcPublicSubnet3SubnetBE12F0B6"
},
"Tags": [
{
"Key": "Name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,4 @@
}
}
}
}
}
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-glue/test/integ.table.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,6 @@
"Statement": [
{
"Action": [
"glue:BatchDeletePartition",
"glue:BatchGetPartition",
"glue:GetPartition",
"glue:GetPartitions",
Expand All @@ -442,6 +441,7 @@
"glue:GetTableVersion",
"glue:GetTableVersions",
"glue:BatchCreatePartition",
"glue:BatchDeletePartition",
"glue:CreatePartition",
"glue:DeletePartition",
"glue:UpdatePartition"
Expand Down Expand Up @@ -510,7 +510,6 @@
},
{
"Action": [
"glue:BatchDeletePartition",
"glue:BatchGetPartition",
"glue:GetPartition",
"glue:GetPartitions",
Expand All @@ -519,6 +518,7 @@
"glue:GetTableVersion",
"glue:GetTableVersions",
"glue:BatchCreatePartition",
"glue:BatchDeletePartition",
"glue:CreatePartition",
"glue:DeletePartition",
"glue:UpdatePartition"
Expand Down Expand Up @@ -622,7 +622,6 @@
"Statement": [
{
"Action": [
"glue:BatchDeletePartition",
"glue:BatchGetPartition",
"glue:GetPartition",
"glue:GetPartitions",
Expand All @@ -631,6 +630,7 @@
"glue:GetTableVersion",
"glue:GetTableVersions",
"glue:BatchCreatePartition",
"glue:BatchDeletePartition",
"glue:CreatePartition",
"glue:DeletePartition",
"glue:UpdatePartition"
Expand Down Expand Up @@ -699,7 +699,6 @@
},
{
"Action": [
"glue:BatchDeletePartition",
"glue:BatchGetPartition",
"glue:GetPartition",
"glue:GetPartitions",
Expand All @@ -708,6 +707,7 @@
"glue:GetTableVersion",
"glue:GetTableVersions",
"glue:BatchCreatePartition",
"glue:BatchDeletePartition",
"glue:CreatePartition",
"glue:DeletePartition",
"glue:UpdatePartition"
Expand Down Expand Up @@ -743,7 +743,6 @@
},
{
"Action": [
"glue:BatchDeletePartition",
"glue:BatchGetPartition",
"glue:GetPartition",
"glue:GetPartitions",
Expand All @@ -752,6 +751,7 @@
"glue:GetTableVersion",
"glue:GetTableVersions",
"glue:BatchCreatePartition",
"glue:BatchDeletePartition",
"glue:CreatePartition",
"glue:DeletePartition",
"glue:UpdatePartition"
Expand Down Expand Up @@ -797,4 +797,4 @@
}
}
}
}
}
84 changes: 67 additions & 17 deletions packages/@aws-cdk/aws-glue/test/table.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { Template } from '@aws-cdk/assertions';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import { testFutureBehavior } from '@aws-cdk/cdk-build-tools/lib/feature-flag';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from '@aws-cdk/cdk-build-tools/lib/feature-flag';
import * as glue from '../lib';
import { CfnTable } from '../lib/glue.generated';

Expand Down Expand Up @@ -79,7 +79,6 @@ test('unpartitioned JSON table', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('partitioned JSON table', () => {
Expand Down Expand Up @@ -157,7 +156,6 @@ test('partitioned JSON table', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('compressed table', () => {
Expand Down Expand Up @@ -223,7 +221,6 @@ test('compressed table', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('table.node.defaultChild', () => {
Expand Down Expand Up @@ -325,7 +322,6 @@ test('encrypted table: SSE-S3', () => {
],
},
});

});

test('encrypted table: SSE-KMS (implicitly created key)', () => {
Expand Down Expand Up @@ -413,7 +409,6 @@ test('encrypted table: SSE-KMS (implicitly created key)', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('encrypted table: SSE-KMS (explicitly created key)', () => {
Expand Down Expand Up @@ -506,7 +501,6 @@ test('encrypted table: SSE-KMS (explicitly created key)', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('encrypted table: SSE-KMS_MANAGED', () => {
Expand Down Expand Up @@ -585,7 +579,6 @@ test('encrypted table: SSE-KMS_MANAGED', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('encrypted table: CSE-KMS (implicitly created key)', () => {
Expand Down Expand Up @@ -654,7 +647,6 @@ test('encrypted table: CSE-KMS (implicitly created key)', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('encrypted table: CSE-KMS (explicitly created key)', () => {
Expand Down Expand Up @@ -729,7 +721,6 @@ test('encrypted table: CSE-KMS (explicitly created key)', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('encrypted table: CSE-KMS (explicitly passed bucket and key)', () => {
Expand Down Expand Up @@ -806,7 +797,6 @@ test('encrypted table: CSE-KMS (explicitly passed bucket and key)', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('explicit s3 bucket and prefix', () => {
Expand Down Expand Up @@ -874,7 +864,6 @@ test('explicit s3 bucket and prefix', () => {
TableType: 'EXTERNAL_TABLE',
},
});

});

test('explicit s3 bucket and with empty prefix', () => {
Expand Down Expand Up @@ -942,7 +931,72 @@ test('explicit s3 bucket and with empty prefix', () => {
TableType: 'EXTERNAL_TABLE',
},
});
});

test('grants: custom', () => {
const stack = new cdk.Stack();
const user = new iam.User(stack, 'User');
const database = new glue.Database(stack, 'Database', {
databaseName: 'database',
});

const table = new glue.Table(stack, 'Table', {
database,
tableName: 'table',
columns: [{
name: 'col',
type: glue.Schema.STRING,
}],
compressed: true,
dataFormat: glue.DataFormat.JSON,
});

table.grant(user, ['glue:UpdateTable']);

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'glue:UpdateTable',
Effect: 'Allow',
Resource: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':glue:',
{
Ref: 'AWS::Region',
},
':',
{
Ref: 'AWS::AccountId',
},
':table/',
{
Ref: 'DatabaseB269D8BB',
},
'/',
{
Ref: 'Table4C2D914F',
},
],
],
},
},
],
Version: '2012-10-17',
},
PolicyName: 'UserDefaultPolicy1F97781E',
Users: [
{
Ref: 'User00B015A1',
},
],
});
});

test('grants: read only', () => {
Expand Down Expand Up @@ -970,7 +1024,6 @@ test('grants: read only', () => {
Statement: [
{
Action: [
'glue:BatchDeletePartition',
'glue:BatchGetPartition',
'glue:GetPartition',
'glue:GetPartitions',
Expand Down Expand Up @@ -1048,7 +1101,6 @@ test('grants: read only', () => {
},
],
});

});

testFutureBehavior('grants: write only', s3GrantWriteCtx, cdk.App, (app) => {
Expand Down Expand Up @@ -1151,7 +1203,6 @@ testFutureBehavior('grants: write only', s3GrantWriteCtx, cdk.App, (app) => {
},
],
});

});

testFutureBehavior('grants: read and write', s3GrantWriteCtx, cdk.App, (app) => {
Expand Down Expand Up @@ -1179,7 +1230,6 @@ testFutureBehavior('grants: read and write', s3GrantWriteCtx, cdk.App, (app) =>
Statement: [
{
Action: [
'glue:BatchDeletePartition',
'glue:BatchGetPartition',
'glue:GetPartition',
'glue:GetPartitions',
Expand All @@ -1188,6 +1238,7 @@ testFutureBehavior('grants: read and write', s3GrantWriteCtx, cdk.App, (app) =>
'glue:GetTableVersion',
'glue:GetTableVersions',
'glue:BatchCreatePartition',
'glue:BatchDeletePartition',
'glue:CreatePartition',
'glue:DeletePartition',
'glue:UpdatePartition',
Expand Down Expand Up @@ -1264,7 +1315,6 @@ testFutureBehavior('grants: read and write', s3GrantWriteCtx, cdk.App, (app) =>
},
],
});

});

test('validate: at least one column', () => {
Expand Down

0 comments on commit 3d64f9b

Please sign in to comment.