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

Fix image handling bugs in twoliter update #326

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

cbgbt
Copy link
Contributor

@cbgbt cbgbt commented Jul 17, 2024

Issue number:
n/a

Description of changes:

always canonicalize image manifests

Prior to canonicalizing image manifests, crane and docker would result in slightly different image digests due to non-semantic formatting differences.


fix bug in call to 'docker image inspect'

The --format string erroneously surrounded the output of docker image inspect in quotes, causing the JSON parser to read the output as a string instead of an object.

Testing done:

  • The provided integration tests pass (but on github actions? 😬)
  • I'm able to twoliter update the bottlerocket repo to core-kit 2.1.0.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@cbgbt cbgbt force-pushed the img-inspect branch 3 times, most recently from 8ed56a2 to 7cad644 Compare July 17, 2024 00:43
@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

^ A few force pushes to test the github actions changes.

Maybe the tests will pass? 😬

@cbgbt cbgbt force-pushed the img-inspect branch 2 times, most recently from 354dac4 to 8ab4094 Compare July 17, 2024 03:20
@cbgbt cbgbt marked this pull request as ready for review July 17, 2024 07:12
@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

^ I realized that reference counting was no longer required now that the ImageToolImpl's memory is owned by a struct.

@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

Apologies for the churn, but I noticed an error in a comment, and also decided that the ImageTool structures should implement Debug.

Copy link
Contributor

@jmt-lab jmt-lab left a comment

Choose a reason for hiding this comment

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

LGTM!

curl -sL "https://github.com/google/go-containerregistry/releases/download/${VERSION}/go-containerregistry_Linux_${ARCH}.tar.gz" > go-containerregistry.tar.gz
tar -zxvf go-containerregistry.tar.gz -C "${{ inputs.install-dir }}" crane

echo "${{ inputs.install-dir }}" >> "${GITHUB_PATH}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

pub struct ImageTool {
image_tool_impl: Box<dyn ImageToolImpl>,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this so so much more :D

Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

Nice!


VERSION=${{ inputs.crane-version }}
if [[ "${VERSION}" == "latest" ]]; then
VERSION=$(curl -s "https://api.github.com/repos/google/go-containerregistry/releases/latest" | jq -r '.tag_name')
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is an API that might be rate limited, it could be better to use the gh CLI that's installed (example.)

The --format string erroneously surrounded the output of `docker image
inspect` in quotes, causing the JSON parser to read the output as a
string instead of an object.
Prior to canonicalizing image manifests, crane and docker would result
in slightly different image digests due to non-semantic formatting differences.
@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

^ force-push to use ghCLI to install crane.

@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

^ force-push to rebase on develop

@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 17, 2024

^ Bah, I misunderstood where the GITHUB_TOKEN could be accessed. Should be ready for review again once the tests pass @bcressey.

@cbgbt cbgbt merged commit a238374 into bottlerocket-os:develop Jul 17, 2024
1 check passed
@cbgbt cbgbt deleted the img-inspect branch July 17, 2024 20:27
@jmt-lab jmt-lab mentioned this pull request Jul 17, 2024
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.

None yet

4 participants