Skip to content
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

Add S3_AccessControl hook #238

Merged

Conversation

mrinaudo-aws
Copy link
Collaborator

Issue #, if available:

Description of changes:
Add S3_AccessControl hook.

Unit tests excerpts

[...]
[INFO] Running com.awscommunity.s3.accesscontrol.PreUpdateHookHandlerTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.354 s -- in com.awscommunity.s3.accesscontrol.PreUpdateHookHandlerTest
[INFO] Running com.awscommunity.s3.accesscontrol.PreCreateHookHandlerTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.025 s -- in com.awscommunity.s3.accesscontrol.PreCreateHookHandlerTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
[...]
[INFO] --- jacoco:0.8.10:check (jacoco-check) @ awscommunity-s3-accesscontrol-handler ---
[...]
[INFO] Analyzed bundle 'awscommunity-s3-accesscontrol-handler' with 5 classes
[INFO] All coverage checks have been met.

Contract tests excerpts

[...]
5 passed, 2 skipped, 17 deselected
[...]

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ericzbeard ericzbeard changed the title Add S3_AccessControl hook. Add S3_AccessControl hook Sep 21, 2023
Copy link
Contributor

@ericzbeard ericzbeard left a comment

Choose a reason for hiding this comment

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

Very thorough as always! Only a few minor comments.

Are you going to add this to the pipeline in a separate PR?

```

## Hook development notes
The RPDK will automatically generate the correct hook input model from the schema whenever the project is built via Maven. You can also do this manually with the following command: `cfn generate`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think "RPDK" is a public term.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see it described here. That initial line is actually created by the cfn-cli tool in the README file when you create a new extension project. I have added lines below for this specific Kotlin project, and left the original line intact.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will update this "Hook development notes" section.

EOF
```

and then submit the hook configuration using the [AWS Command Line Interface (AWS CLI)](https://aws.amazon.com/cli/). First, get the [Amazon Resource Name](https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html) (ARN) for this hook, as follows (examples are for the `us-east-1` region):
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace double spaces after periods with single space. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do - thank you!

@mrinaudo-aws
Copy link
Collaborator Author

Very thorough as always! Only a few minor comments.

Are you going to add this to the pipeline in a separate PR?

I can add an updated pipeline template here as well.

@ericzbeard ericzbeard merged commit eb7c6b1 into aws-cloudformation:main Sep 22, 2023
141 checks passed
@mrinaudo-aws mrinaudo-aws deleted the add-s3-accesscontrol-hook branch September 22, 2023 19:27
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