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

Use default keychain to resolve OCI credentials #8273

Closed
wants to merge 1 commit into from

Conversation

bduffany
Copy link
Member

Credential helpers stopped working for one of our users after they switched from podman to OCI. This PR enables the default keychain (behind a flag) so that credential helpers will work.

Future improvements:

  • Either patch in Support ~/.config podman path in default keychain google/go-containerregistry#2052 or get it merged upstream. This will actually allow the ~/.config/containers/auth.json file to work. Without this patch, the ~/.config/containers/auth.json path will not work, but in the meantime, ~/.docker/config.json can be used instead, or REGISTRY_AUTH_FILE=/root/.config/containers/auth.json can be set as an environment variable on the executor container.
  • Enable this new flag by default, but make sure it's disabled for our cloud executors. Or just enable it in the helm charts.

@bduffany
Copy link
Member Author

I think it's actually feasible to test this, will close and re-open once that's added

@bduffany bduffany closed this Jan 31, 2025
@@ -92,11 +93,7 @@ func CredentialsFromProperties(props *platform.Properties) (Credentials, error)

// If no credentials were provided, fallback to any specified by
// --executor.container_registries.
if len(*registries) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you not need to look at the registries anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

We still look at the registries, this just removes the short-circuit if there are no registries configured, so that we can fall back to using the default keychain below.

@bduffany
Copy link
Member Author

Moved to #8274 (github won't let me reopen since I force-pushed)

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.

2 participants