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 CodeGuru Reviewer RepositoryAssociation support #18

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Conversation

gsoria
Copy link

@gsoria gsoria commented Oct 27, 2023

Attempting to delete CodeGuru RepositoryAssociations using the Cloud Control API (AWS::CodeGuruReviewer::RepositoryAssociation) results in these errors:

 Errors: time="2023-10-24T20:45:26Z" level=error msg="Listing AWS::CodeGuruReviewer::RepositoryAssociation failed:\n    GeneralServiceException: AWS::CodeGuruReviewer::RepositoryAssociation Handler returned status FAILED: The security token included in the request is invalid (Service: CodeGuruReviewer, Status Code: 403, Request ID: f3bbe738-5aec-48a9-be48-facd30973c11) (HandlerErrorCode: GeneralServiceException, RequestToken: 7b035a0f-71c4-4271-97bd-da5984d361a8)"

This PR adds a new module to support CodeGuru RepositoryAssociations.

Testing

Create Codecommit and CodeGuru resources:

#!/bin/bash

echo "create a codecommit repository"
aws codecommit create-repository --repository-name MyDemoRepo

echo "create a codeguru repository association"
aws codeguru-reviewer associate-repository \
    --repository CodeCommit={Name=MyDemoRepo}

Then run aws-nuke specifying CodeGuruReviewerRepositoryAssociation resource.

And finally, verify if the resource was cleanup correctly, by running

aws codeguru-reviewer list-repository-associations

Copy link
Member

@sstoops sstoops left a comment

Choose a reason for hiding this comment

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

This looks pretty good at first glance. I haven't had a chance to build and run it yet and I'm going to take a sick day the rest of today. I did leave one small thought online. I'll try to test this as soon as I'm back online!

}

func init() {
register("CodeGuruReviewerRepositoryAssociation", ListCodeGuruReviewerRepositoryAssociations)
Copy link
Member

Choose a reason for hiding this comment

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

Should we specify the CC API association here to override? I think the following is the syntax, but I wouldn't blindly accept this suggestion if I were you. 😸

Suggested change
register("CodeGuruReviewerRepositoryAssociation", ListCodeGuruReviewerRepositoryAssociations)
register("CodeGuruReviewerRepositoryAssociation", ListCodeGuruReviewerRepositoryAssociations,
mapCloudControl("AWS::CodeGuruReviewer::RepositoryAssociation"))

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for raising this! The cloud control resource doesn't work, do you think we should include the association anyways?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. If I remember correctly, the association essentially tells AWS Nuke to use the module instead of the CC API if both happen to be specified in someone's configuration.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. Addressed in 888f8d2

Copy link
Member

@sstoops sstoops left a comment

Choose a reason for hiding this comment

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

Bingo!

us-east-1 - CodeGuruReviewerRepositoryAssociation - [AssociationArn: "arn:aws:codeguru-reviewer:us-east-1:476209218594:association:37126b8b-a354-485e-9896-ad98cdd074fe", AssociationId: "37126b8b-a354-485e-9896-ad98cdd074fe", Name: "MyDemoRepo", Owner: "476209218594", ProviderType: "CodeCommit"] - removed

@gsoria gsoria merged commit eed354e into oreilly-main Nov 1, 2023
1 check passed
corybekk pushed a commit that referenced this pull request Nov 6, 2024
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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