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

Only list images when their use case is not skipped #363

Conversation

georgantasp
Copy link
Contributor

@georgantasp georgantasp commented Oct 30, 2024

Checklist

  • All new jobs, commands, executors, parameters have descriptions
  • Examples have been added for any significant new features
  • README has been updated, if necessary

Motivation, issues

My CI AWS profile is given an aws managed policy which does not have ecr:ListImages permission. EC2InstanceProfileForImageBuilderECRContainerBuilds

Description

The tag_image command has a flag to skip_when_tags_exist (default false). When the flag is not set, there is no use for the ListImages output.

@georgantasp georgantasp force-pushed the avoidPermissionWhenNotNeeded branch from bf1aa7e to 53a0d1b Compare October 30, 2024 13:04
@georgantasp georgantasp marked this pull request as ready for review October 30, 2024 13:04
@georgantasp georgantasp requested a review from a team as a code owner October 30, 2024 13:04
@marboledacci
Copy link
Contributor

If you look at line 17 that value is then used to do the validation and tag or not, so this implementation would break that.

@georgantasp georgantasp force-pushed the avoidPermissionWhenNotNeeded branch from 53a0d1b to 20b88d6 Compare October 31, 2024 13:53
@georgantasp
Copy link
Contributor Author

@marboledacci you're right, I let the double negative confuse me. I pushed a fix to make it make sense. Now, the EXISTING_TAGS is only set when AWS_ECR_BOOL_SKIP_WHEN_TAGS_EXIST is set to 1.

@marboledacci marboledacci merged commit 94bd383 into CircleCI-Public:master Nov 6, 2024
2 checks passed
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