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, remove k8s-pkg-credentialprovider #431

Merged
merged 7 commits into from
Sep 2, 2022

Conversation

joaopapereira
Copy link
Member

Move to cloud credential helpers recommended by GGCR:
https://github.com/google/go-containerregistry/tree/main/pkg/authn#emulating-cloud-provider-credential-helpers

We had to fix our test registry to reject mount requests properly with a 202 due to the changes in google/go-containerregistry#1388

Manually validated against GCP and Azure ambient credentials, both of them work and fail gracefully when access to the metadata server is blocked.
Based on the PR #409


Apart from the removal of the dependency on k8s-pkg-credentialprovider this PR also changes the default behavior of imgpkg. The keychains no longer get loaded by default, in order to load a particular keychain the environment variable IMGPKG_ACTIVE_KEYCHAINS needs to be set. The Keychains that are supported are:

  • gke Enabled access to the GKE authentication helpers
  • aks Enabled access to the AKS authentication helpers
  • ecr Enabled access to the ECR authentication helpers
  • github Enabled access to the Github authentication helpers

To enable multiple keychains you can provide them in a comma separated list like
export IMGPKG_ACTIVE_KEYCHAINS=gke,aks,ecr,github

This PR also starts the deprecation process for the environment variable IMGPKG_ENABLE_IAAS_AUTH. For now, if set to true, it is similar to doing export IMGPKG_ACTIVE_KEYCHAINS=gke,aks,ecr,github. In the future, this flag will no longer be used.

@joaopapereira joaopapereira temporarily deployed to TanzuNet Registry Dev e2e August 31, 2022 21:35 Inactive
@joaopapereira joaopapereira temporarily deployed to GCR e2e August 31, 2022 21:35 Inactive
Copy link
Contributor

@neil-hickey neil-hickey left a comment

Choose a reason for hiding this comment

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

LGTM. Some minor nits :)

pkg/imgpkg/cmd/registry_flags.go Outdated Show resolved Hide resolved
pkg/imgpkg/cmd/registry_flags.go Show resolved Hide resolved
pkg/imgpkg/registry/keychain.go Outdated Show resolved Hide resolved
test/helpers/registry/blobs.go Outdated Show resolved Hide resolved
resp.Header().Set("Docker-Content-Digest", h.String())
resp.WriteHeader(http.StatusCreated)
return nil
// check err type??
Copy link
Contributor

Choose a reason for hiding this comment

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

a TODO? Or just thought? Looks like we are ignoring any error that the Mount() comes back with now.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a temporary pkg that we are waiting it to be merged in ggcr so we can remove it from imgpkg

Signed-off-by: Joao Pereira <joaod@vmware.com>
@joaopapereira joaopapereira temporarily deployed to GCR e2e September 1, 2022 16:48 Inactive
@joaopapereira joaopapereira temporarily deployed to TanzuNet Registry Dev e2e September 1, 2022 16:49 Inactive
@joaopapereira joaopapereira merged commit 7387d02 into develop Sep 2, 2022
@joaopapereira joaopapereira deleted the remove-vdemeester branch September 2, 2022 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants