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

Bump go-containerregistry version to use Kubernetes Secrets before cl… #3098

Merged

Conversation

smileisak
Copy link
Contributor

Bump go-containerregistry version to use Kubernetes Secrets before cloud specific auth methods

Signed-off-by: Ismail KABOUBI ikaboubi@gmail.com

Description of your changes

Fixes #3023

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

I've run e2e tests locally using make e2e. No need to add more unit tests / e2e tests since there is no changes in code.

…oud specific auth methods

Signed-off-by: Ismail KABOUBI <ikaboubi@gmail.com>
@hasheddan hasheddan self-requested a review May 23, 2022 13:14
go.mod Outdated
Comment on lines 10 to 14
// TODO(hasheddan): we prefer to consume release versions of
// go-containerregistry. An incremental version is currently being used to
// consume the new k8schain implementation, which fixes an issue with
// reconcile timeouts when pulling packages in some environments.
github.com/google/go-containerregistry v0.8.1-0.20220302183023-329563766ce8
github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20220302183023-329563766ce8
github.com/google/go-containerregistry v0.8.1-0.20220414143355-892d7a808387
Copy link
Member

Choose a reason for hiding this comment

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

@smileisak thanks so much for this update! It looks like the fix was included in google/go-containerregistry#1346, which was in the most recent release (https://github.com/google/go-containerregistry/releases/tag/v0.9.0). Could we pin to v0.9.0 and drop my TODO comment above?

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, I though that we still have the issue of the reconcile timeouts when pulling packages in some environments. I'll update this MR to use the v0.9.0.

Copy link
Member

Choose a reason for hiding this comment

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

@smileisak the update where we moved from a release version (e.g. v0.7.0) to an incremental version (e.g. v0.8.1-0.20220302183023-329563766ce8) was to fix the problem described. Because there wasn't a release version available we had to use the incremental version even though we prefer not to. Now that the previous fix, and the one you are including in this PR are in a release version v0.9.0 we can depend on it and drop my TODO as it is no longer applicable :)

…ion.

Signed-off-by: Ismail KABOUBI <ikaboubi@gmail.com>
Signed-off-by: Ismail KABOUBI <ikaboubi@gmail.com>
Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @smileisak -- I'll see about what backporting this looks like for a patch release given that it involves k8s dependency bumps 👍🏻

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.

Pulling packages from artifact-registry is not working in GKE cluster
2 participants