Skip to content

Conversation

Lowercases
Copy link
Contributor

As a test for r101-api.

@Lowercases Lowercases force-pushed the test-lifecycle-rules-ecr branch from 6f654c6 to 63571d1 Compare May 26, 2022 18:26
tag: "latest,$CIRCLE_BRANCH,$CIRCLE_SHA1"
# create the AWS ECR repo if it does not exist already.
create-repo: true
lifecycle-policy-path: >
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation says this should be a path, not the actual content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! But it's a lie.

I started adding a file as in https://github.com/remind101/r101-api/pull/16623/files#diff-2c27a3138e2c83cb750d9a8a3b415799e8f3806ae6bd8a9d06a6abb23e018d36 (only left there for dev purposes) only to get Invalid length for parameter lifecyclePolicyText, value: 28, valid min length: 100 (https://app.circleci.com/pipelines/github/remind101/r101-api/11123/workflows/b3566098-95c6-4f13-9fd7-8a7f3f79a4d1/jobs/257499).

After my initial wtf moment, my suspects were confirmed when I tried the current approach and bingo! got An error occurred (AccessDeniedException) when calling the PutLifecyclePolicy operation: User: arn:aws:iam::************:user/erc-stage-circleci is not authorized to perform: ecr:PutLifecyclePolicy on resource: arn:aws:ecr:*********:************:repository/remind101/r101-api because no identity-based policy allows the ecr:PutLifecyclePolicy action (https://app.circleci.com/pipelines/github/remind101/r101-api/11188/workflows/00a85887-5e28-4912-81d6-94738f99dbe9/jobs/258057).

Trust no one.

Copy link
Contributor

@isobit isobit May 26, 2022

Choose a reason for hiding this comment

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

WTF lol. Well, at least it would be good to have a comment to explain that.

valid min length: 100

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean with "at least"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment

@Lowercases Lowercases requested a review from rleira May 26, 2022 19:06
Avoid removing "master" and "main" tagged images to avoid removing a
running image after aggressive master/main rebuilding.

In order to glob all master and main images, add a composite tag for the
branch and build number.
# tags for this image, comma separated. A composite of branch+sha1 is
# added so the lifecycle rule (below) can filter images beginning
# with "master" or "main" to avoid applying aggressive rules.
tag: "latest,$CIRCLE_BRANCH,$CIRCLE_SHA1,$CIRCLE_BRANCH-$CIRCLE_BUILD_NUM"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd still want to keep the commit SHA in the prefixed tag, because it's nice to have that info embedded and also would make it easier to transition to something other than CircleCI without causing conflicts.

Copy link

@rleira rleira left a comment

Choose a reason for hiding this comment

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

Looks great I think that having a space of 10k and making the master/main branches have a longer longevity (if I understood that part) covers our cases very well. Custom branches for custom deploys will probably be due well before 90 days

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.

3 participants