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

feat: add ability to override registry keychain #1666

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

noamichael
Copy link
Contributor

Summary

Updates options.RegistryOptions to allow users to pass registry credentials via a new type alias Keychain which maps to authn.Keychain.

Resolves: #1665

Ticket Link

Fixes #1665

Release Note

NONE

@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2022

Codecov Report

Merging #1666 (eb17eaf) into main (db90d13) will increase coverage by 0.19%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1666      +/-   ##
==========================================
+ Coverage   28.96%   29.16%   +0.19%     
==========================================
  Files         139      140       +1     
  Lines        8268     8342      +74     
==========================================
+ Hits         2395     2433      +38     
- Misses       5610     5643      +33     
- Partials      263      266       +3     
Impacted Files Coverage Δ
cmd/cosign/cli/options/registry.go 0.00% <0.00%> (ø)
pkg/cosign/tuf/client.go 62.34% <0.00%> (-0.95%) ⬇️
cmd/cosign/cli/options/attach.go 0.00% <0.00%> (ø)
pkg/oci/signature/layer.go 75.92% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db90d13...eb17eaf. Read the comment docs.

@noamichael
Copy link
Contributor Author

I noticed the linting failed. gocritic seems to like switch statements over if/elseif/else chains. If preferred, I could do

switch {
case o.Keychain != nil:
	opts = append(opts, remote.WithAuthFromKeychain(o.Keychain))
case o.KubernetesKeychain:
	kc := authn.NewMultiKeychain(
		authn.DefaultKeychain,
		google.Keychain,
		authn.NewKeychainFromHelper(ecr.NewECRHelper(ecr.WithLogger(ioutil.Discard))),
		authn.NewKeychainFromHelper(credhelper.NewACRCredentialsHelper()),
		github.Keychain,
	)
	opts = append(opts, remote.WithAuthFromKeychain(kc))
default:
	opts = append(opts, remote.WithAuthFromKeychain(authn.DefaultKeychain))
}

but I was trying to make as few changes as possible.

@dlorenc
Copy link
Member

dlorenc commented Mar 26, 2022

Seems right, it usually complains after 3 branches.

@dlorenc
Copy link
Member

dlorenc commented Mar 26, 2022

Looks like you missed the DCO this time around :)

When importing cosign as a library directly in code, it is convenient
to have the option of providing the go-containerregistry keychain. This
allows consumers of cosign to call functions such as sign.SignCmd and
copy.CopyCmd and pass the registry creds in memory without having to
write the creds to a docker/oci auth config file first.

This commit adds a new type alias Keychain which maps to authn.Keychain.

Resolves: sigstore#1665

Signed-off-by: Michael Kucinski <[email protected]>
@noamichael
Copy link
Contributor Author

Looks like you missed the DCO this time around :)

Sorry about that! I'm not in the habit of signing off on commits. I ended up squashing the history as recommended in the contribution guide. Hopefully any future PRs of mine will be cleaner.

@dlorenc
Copy link
Member

dlorenc commented Mar 26, 2022

No worries! The first one is always annoying because I have to keep clicking the GitHub button to allow CI to run in between, after this you'll be able to just rerun yourself!

@dlorenc dlorenc merged commit 516cc39 into sigstore:main Mar 28, 2022
@github-actions github-actions bot added this to the v1.7.0 milestone Mar 28, 2022
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
When importing cosign as a library directly in code, it is convenient
to have the option of providing the go-containerregistry keychain. This
allows consumers of cosign to call functions such as sign.SignCmd and
copy.CopyCmd and pass the registry creds in memory without having to
write the creds to a docker/oci auth config file first.

This commit adds a new type alias Keychain which maps to authn.Keychain.

Resolves: sigstore#1665

Signed-off-by: Michael Kucinski <[email protected]>
@ChrisJBurns
Copy link
Contributor

@noamichael Apologies to bring you back in time 😅 , I have found this PR and wondered how you go this to work in code? I'm using the SignCmd in Cosign as a library to sign container images, but am falling into a bit of a hole at the moment with the fact that Cosign isn't signing anything. Do you know how I would go about creating a Keychain from a registry url, username and password? At the moment, I can't find any examples of doing this

@noamichael
Copy link
Contributor Author

@noamichael Apologies to bring you back in time 😅 , I have found this PR and wondered how you go this to work in code? I'm using the SignCmd in Cosign as a library to sign container images, but am falling into a bit of a hole at the moment with the fact that Cosign isn't signing anything. Do you know how I would go about creating a Keychain from a registry url, username and password? At the moment, I can't find any examples of doing this

It's been awhile! I don't have any working examples, but here is something I just wrote (basically pseudocode) to get the point across on how I expected this to be used. The key point is you need to implement Keychain which returns an Authenticator which itself returns the AuthConfig used to authenticate. The AuthConfig has tons of options for logging into a registry. So a Keychain is like a factory for Authenticators, which are factories for AuthConfigs:

package main

import (
	"fmt"

	"github.com/google/go-containerregistry/pkg/authn"
	"github.com/sigstore/cosign/cmd/cosign/cli/options"
	"github.com/sigstore/cosign/cmd/cosign/cli/sign"
)

// Some struct to hold username and password
type userCredentials struct {
	username string
	password string
}

// A keychain that is able to return the creds based on the registry url
type InMemoryKeyChain struct {
	// An in-memory map of username/passwords
	credentials map[string]userCredentials
}

// Returns a function that is able to produce an AuthConfig struct. This is basically a factory factory
func (k *InMemoryKeyChain) Resolve(resource authn.Resource) (authn.Authenticator, error) {

	// Ask for the registry URL
	registryHost := resource.RegistryStr()

	// Find the user credentials by the registry URL
	userCreds, ok := k.credentials[registryHost]

	if !ok {
		return authn.Anonymous, fmt.Errorf("Unable to find credentials for host %s", registryHost)
	}

	// Return an authConfig that contains the username and password we looked up
	return authn.FromConfig(authn.AuthConfig{
		Username: userCreds.username,
		Password: userCreds.password,
	}), nil
}

func main() {

	// Create a keychain which maps the registry URL to the
	// username and password we use for that registry
	keychain := &InMemoryKeyChain{
		credentials: map[string]userCredentials{
			"registry.example.com": {
				username: "myusername",
				password: "mypassword",
			},
		},
	}

	// OR, if you simply want to use your current host's Docker config,
	// pass authn.DefaultKeychain

	// Call the sign command with all the various flags you need
	sign.SignCmd(
		&options.RootOptions{},
		options.KeyOpts{},
		options.RegistryOptions{
			Keychain: keychain,
		},
		make(map[string]interface{}),
		[]string{"images to sign"},
		"",    // cert path
		"",    // cert chain path
		true,  // upload
		"",    // output signature
		"",    // outputCertificate
		"",    // payloadPath
		true,  // force
		false, // recurisve
		"",    // attachment
		false, // noTlogUpload
	)

}

Please let me know if this helps or if you have any more questions!

@ChrisJBurns
Copy link
Contributor

ChrisJBurns commented Jun 6, 2023

@noamichael That worked perfectly! Thank you so much. You've saved me a fair few hours/days trying to figure what I needed to do 😄 I'm trying to take note of all of the things that we currently do that add that extra bit of code so we can make it easier by putting a lot of it in the pkg layer making it easier to use as a library. This is definitely one of those areas to make simpler 👍

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.

feat: allow consumers of cosign to provide registry credentials
4 participants