feat(s3): add `allowedActionPatterns` parameter to grantWrite #24211

lpizzinidev · 2023-02-17T11:36:03Z

I've added an optional parameter allowedActionPatterns that will restrict the permissions to a certain list of action patterns for the grantWrite method.

Closes #24074

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2023-02-17T11:36:15Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

lpizzinidev · 2023-02-17T15:02:59Z

The pull request linter fails with the following errors:
❌ Features must contain a change to a README file.
PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

Exemption Request.

The S3 README doesn't contain references to the grantWrite method so there is nothing to change there.
Let me know if you'd like me to add an explanation of the method.

TheRealAmazonKendra

Adding an exclusion filter doesn't really fit the pattern of any of the rest of these functions. I would prefer to see a function that explicitly adds the patterns a user wants than one that filters out patterns.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

Pull request has been modified.

lpizzinidev · 2023-02-22T08:38:57Z

@TheRealAmazonKendra
Thanks for the feedback!
I've changed the parameter to allowedActions, granting permission only to specified actions.
Please let me know if this is ok or if you need more changes.

TheRealAmazonKendra

Just a few suggested changes to the contract here.

TheRealAmazonKendra · 2023-02-23T00:58:28Z

packages/@aws-cdk/aws-s3/lib/bucket.ts

@@ -771,8 +772,9 @@ export abstract class BucketBase extends Resource implements IBucket {
      this.arnForObjects(objectsKeyPattern));
  }

-  public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*') {
-    return this.grant(identity, this.writeActions, perms.KEY_WRITE_ACTIONS,
+  public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*', allowedActions: string[] = []) {


Suggested change

public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*', allowedActions: string[] = []) {

public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*', allowedActions: string[] = ['*']) {

TheRealAmazonKendra · 2023-02-23T00:59:03Z

packages/@aws-cdk/aws-s3/lib/bucket.ts

   */
-  grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any): iam.Grant;
+  grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any, allowedActions?: string[]): iam.Grant;


Suggested change

grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any, allowedActions?: string[]): iam.Grant;

grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any, ...allowedActionPatterns?: string[]): iam.Grant;

The problem with using a rest parameter is that it cannot be marked as optional, nor given a default value.
So I opted for a required rest parameter allowedActionPatterns that will default to this.writeActions if empty.
However, with the current implementation, I'm getting this error at CodeBuild:

err - METHOD aws-cdk-lib.aws_s3.Bucket.grantWrite: argument allowedActionPatterns, newly required argument. [new-argument:aws-cdk-lib.aws_s3.Bucket.grantWrite] err - METHOD aws-cdk-lib.aws_s3.BucketBase.grantWrite: argument allowedActionPatterns, newly required argument. [new-argument:aws-cdk-lib.aws_s3.BucketBase.grantWrite] err - METHOD aws-cdk-lib.aws_s3.IBucket.grantWrite: argument allowedActionPatterns, newly required argument. [new-argument:aws-cdk-lib.aws_s3.IBucket.grantWrite]

Any suggestion on how I could fix this?

Ah, that's right. Fine as is.

TheRealAmazonKendra · 2023-02-23T01:00:00Z

packages/@aws-cdk/aws-s3/lib/bucket.ts

@@ -204,8 +204,9 @@ export interface IBucket extends IResource {
   *
   * @param identity The principal
   * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
+   * @param allowedActions: Include only certain actions in the permissions


Suggested change

* @param allowedActions: Include only certain actions in the permissions

* @param allowedActionPatterns: Restrict the permissions to certain list of action patterns (default '*')

…rest parameter

Pull request has been modified.

TheRealAmazonKendra · 2023-03-01T00:24:46Z

Go ahead and change that back to an optional array. It becomes a breaking change if the field is required.

mergify · 2023-03-28T20:35:46Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2023-03-28T22:11:25Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 3d79254
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2023-03-28T22:11:39Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

feat(s3): added excludedKeyActions parameter to grantWrite

dbdce48

github-actions bot added repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. labels Feb 17, 2023

aws-cdk-automation requested a review from a team February 17, 2023 11:36

github-actions bot added the p2 label Feb 17, 2023

aws-cdk-automation previously requested changes Feb 17, 2023

View reviewed changes

aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Feb 17, 2023

TheRealAmazonKendra previously requested changes Feb 20, 2023

View reviewed changes

lpizzinidev added 2 commits February 22, 2023 09:31

changed excludedKeyActions parameter to allowedActions for grantWrite

9e23033

Merge branch 'main' into awsgh-24074

04115e0

lpizzinidev changed the title ~~feat(s3): added excludedKeyActions parameter to grantWrite~~ feat(s3): added allowedActions parameter to grantWrite Feb 22, 2023

TheRealAmazonKendra previously requested changes Feb 23, 2023

View reviewed changes

lpizzinidev added 2 commits February 23, 2023 09:28

fix: changed allowedActions array parameter to allowedActionPatterns …

f50a3a2

…rest parameter

Merge branch 'main' into awsgh-24074

f383804

lpizzinidev changed the title ~~feat(s3): added allowedActions parameter to grantWrite~~ feat(s3): added allowedActionPatterns parameter to grantWrite Feb 23, 2023

lpizzinidev added 2 commits February 23, 2023 14:28

fix: changed test description

b81ebb4

Merge branch 'main' into awsgh-24074

be95818

changed allowedActionPatterns parameter type to optional array

77482ad

Merge branch 'main' into awsgh-24074

c77175d

TheRealAmazonKendra changed the title ~~feat(s3): added allowedActionPatterns parameter to grantWrite~~ feat(s3): add allowedActionPatterns parameter to grantWrite Mar 2, 2023

TheRealAmazonKendra approved these changes Mar 28, 2023

View reviewed changes

Merge branch 'main' into awsgh-24074

3d79254

mergify bot merged commit 5b5c36f into aws:main Mar 28, 2023

filletofish mentioned this pull request Jul 2, 2023

feat(scheduler): ScheduleGroup #26196

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(s3): add `allowedActionPatterns` parameter to grantWrite #24211

feat(s3): add `allowedActionPatterns` parameter to grantWrite #24211

lpizzinidev commented Feb 17, 2023 •

edited

Loading

gitpod-io bot commented Feb 17, 2023

aws-cdk-automation left a comment

lpizzinidev commented Feb 17, 2023

TheRealAmazonKendra left a comment

lpizzinidev commented Feb 22, 2023

TheRealAmazonKendra left a comment

TheRealAmazonKendra Feb 23, 2023

TheRealAmazonKendra Feb 23, 2023

lpizzinidev Feb 23, 2023

TheRealAmazonKendra Mar 1, 2023

TheRealAmazonKendra Feb 23, 2023

TheRealAmazonKendra commented Mar 1, 2023

mergify bot commented Mar 28, 2023

aws-cdk-automation commented Mar 28, 2023

mergify bot commented Mar 28, 2023

	public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*', allowedActions: string[] = []) {
	public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '', allowedActions: string[] = ['']) {

	grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any, allowedActions?: string[]): iam.Grant;
	grantWrite(identity: iam.IGrantable, objectsKeyPattern?: any, ...allowedActionPatterns?: string[]): iam.Grant;

	* @param allowedActions: Include only certain actions in the permissions
	* @param allowedActionPatterns: Restrict the permissions to certain list of action patterns (default '*')

feat(s3): add allowedActionPatterns parameter to grantWrite #24211

feat(s3): add allowedActionPatterns parameter to grantWrite #24211

Conversation

lpizzinidev commented Feb 17, 2023 • edited Loading

gitpod-io bot commented Feb 17, 2023

aws-cdk-automation left a comment

Choose a reason for hiding this comment

lpizzinidev commented Feb 17, 2023

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

lpizzinidev commented Feb 22, 2023

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

TheRealAmazonKendra Feb 23, 2023

Choose a reason for hiding this comment

TheRealAmazonKendra Feb 23, 2023

Choose a reason for hiding this comment

lpizzinidev Feb 23, 2023

Choose a reason for hiding this comment

TheRealAmazonKendra Mar 1, 2023

Choose a reason for hiding this comment

TheRealAmazonKendra Feb 23, 2023

Choose a reason for hiding this comment

TheRealAmazonKendra commented Mar 1, 2023

mergify bot commented Mar 28, 2023

aws-cdk-automation commented Mar 28, 2023

AWS CodeBuild CI Report

mergify bot commented Mar 28, 2023

feat(s3): add `allowedActionPatterns` parameter to grantWrite #24211

feat(s3): add `allowedActionPatterns` parameter to grantWrite #24211

lpizzinidev commented Feb 17, 2023 •

edited

Loading