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

Downgrade oras version due to ECR bug. #408

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

jonahjon
Copy link
Contributor

@jonahjon jonahjon commented Aug 15, 2022

Signed-off-by: jonahjon [email protected]

Issue #, if available:

This is issue actively blocking pipelines.

Description of changes:

Oras 13 is not working correctly, and blocking pipeline.

Link to issue, and comment from maintainers.

As a registry client, ORAS should do the layer pushing in two step:

Try to initiate an upload process via a POST request
Do the blob upload via PUT or PATCHes
During PUT(step 2), ORAS fails to reuse the token obtained by POST(step 1) and ends in rewinding the request body.

Why it only fails for ECR:
An credential cache was introduced in oras-go v2. The cache was keyed by the scopes in the oauth challenge. An OCI-compliant format of the scope should be like repository:some-project/some-repo:pull,push. ECR returns aws and that unexpected key prevents PUT(step 2) from getting the token cached by POST(step 1).

We had a fix in oras-go and will apply the change to CLI. This issue will be fixed in upcoming ORAS release.

cc @nima

Signed-off-by: jonahjon <[email protected]>
@eks-distro-bot eks-distro-bot added approved size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 15, 2022
Comment on lines +49 to +50
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-${CODEBUILD_BUILD_NUMBER}" bundle.yaml
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-latest" bundle.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-${CODEBUILD_BUILD_NUMBER}" bundle.yaml
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-latest" bundle.yaml
awsAuth "ecr-public"
"$ORAS_BIN" push "${REPO}:v${version}-${CODEBUILD_BUILD_NUMBER}" bundle.yaml
"$ORAS_BIN" push "${REPO}:v${version}-latest" bundle.yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

That is how it used to be. I was just wondering why you were piping the output of that to oras push. I think what you have will work fine, it is just a bit odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's because of the changes Eric made to the common.sh it required being piped now.

Copy link
Contributor

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Comment on lines +49 to +50
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-${CODEBUILD_BUILD_NUMBER}" bundle.yaml
awsAuth "ecr-public" | "$ORAS_BIN" push "${REPO}:v${version}-latest" bundle.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

That is how it used to be. I was just wondering why you were piping the output of that to oras push. I think what you have will work fine, it is just a bit odd.

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonahjon, TerryHowe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot merged commit f12c0cd into aws:main Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants