Refactor credential provider (#707)

* src: cleanup Signed-off-by: Matej Vasek <mvasek@redhat.com> * src: cleanup Signed-off-by: Matej Vasek <mvasek@redhat.com> * src: cleanup Signed-off-by: Matej Vasek <mvasek@redhat.com> * src: cleanup Signed-off-by: Matej Vasek <mvasek@redhat.com> * src: cleanup Signed-off-by: Matej Vasek <mvasek@redhat.com> * src: allow to set custom crednetial loader Signed-off-by: Matej Vasek <mvasek@redhat.com>
2021-12-09 23:19:28 +01:00 · 2021-12-09 23:19:28 +01:00 · 27e1b0153a
parent 07062c144a
commit 27e1b0153a
5 changed files with 166 additions and 81 deletions
--- a/cmd/build.go
+++ b/cmd/build.go
@ -28,9 +28,8 @@ func newBuildClient(cfg buildConfig) (*fn.Client, error) {
 	pusherOption := fn.WithPusher(nil)
 	if cfg.Push {
 		credentialsProvider := docker.NewCredentialsProvider(
-			newCredentialsCallback(),
-			docker.CheckAuth,
-			newChooseHelperCallback(),
+			docker.WithPromptForCredentials(newPromptForCredentials()),
+			docker.WithPromptForCredentialStore(newPromptForCredentialStore()),
 		)
 		pusher, err := docker.NewPusher(
 			docker.WithCredentialsProvider(credentialsProvider),
--- a/cmd/deploy.go
+++ b/cmd/deploy.go
@ -27,9 +27,8 @@ func newDeployClient(cfg deployConfig) (*fn.Client, error) {
 	builder := buildpacks.NewBuilder()

 	credentialsProvider := docker.NewCredentialsProvider(
-		newCredentialsCallback(),
-		docker.CheckAuth,
-		newChooseHelperCallback())
+		docker.WithPromptForCredentials(newPromptForCredentials()),
+		docker.WithPromptForCredentialStore(newPromptForCredentialStore()))
 	pusher, err := docker.NewPusher(
 		docker.WithCredentialsProvider(credentialsProvider),
 		docker.WithProgressListener(listener))
@ -195,7 +194,7 @@ func runDeploy(cmd *cobra.Command, _ []string, clientFn deployClientFn) (err err
 	// (for example kubectl usually uses ~/.kube/config)
 }

-func newCredentialsCallback() func(registry string) (docker.Credentials, error) {
+func newPromptForCredentials() func(registry string) (docker.Credentials, error) {
 	firstTime := true
 	return func(registry string) (docker.Credentials, error) {
 		var result docker.Credentials
@ -229,7 +228,7 @@ func newCredentialsCallback() func(registry string) (docker.Credentials, error)
 	}
 }

-func newChooseHelperCallback() docker.ChooseCredentialHelperCallback {
+func newPromptForCredentialStore() docker.ChooseCredentialHelperCallback {
 	return func(availableHelpers []string) (string, error) {
 		if len(availableHelpers) < 1 {
 			fmt.Fprintf(os.Stderr, `Credentials will not be saved.
--- a/docker/credentials_helper.go
+++ b/docker/credentials_helper.go
@ -12,7 +12,6 @@ import (
 	"runtime"
 	"strings"

-	"github.com/containers/image/v5/types"
 	"github.com/docker/docker-credential-helpers/client"
 	"github.com/docker/docker-credential-helpers/credentials"
 )
@ -65,15 +64,15 @@ func setCredentialHelperToConfig(confFilePath, helper string) error {
 	return nil
 }

-func getCredentialsByCredentialHelper(confFilePath, registry string) (types.DockerAuthConfig, error) {
-	result := types.DockerAuthConfig{}
+func getCredentialsByCredentialHelper(confFilePath, registry string) (Credentials, error) {
+	result := Credentials{}

 	helper, err := getCredentialHelperFromConfig(confFilePath)
 	if err != nil && !os.IsNotExist(err) {
-		return types.DockerAuthConfig{}, fmt.Errorf("failed to get helper from config: %w", err)
+		return result, fmt.Errorf("failed to get helper from config: %w", err)
 	}
 	if helper == "" {
-		return types.DockerAuthConfig{}, errCredentialsNotFound
+		return result, errCredentialsNotFound
 	}

 	helperName := fmt.Sprintf("docker-credential-%s", helper)
--- a/docker/pusher.go
+++ b/docker/pusher.go
@ -14,6 +14,8 @@ import (
 	"regexp"
 	"strings"

+	"github.com/containers/image/v5/pkg/docker/config"
+
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
@ -22,7 +24,6 @@ import (

 	fn "knative.dev/kn-plugin-func"

-	"github.com/containers/image/v5/pkg/docker/config"
 	containersTypes "github.com/containers/image/v5/types"
 	"github.com/docker/docker/api/types"
 )
@ -42,9 +43,9 @@ var ErrUnauthorized = errors.New("bad credentials")

 // VerifyCredentialsCallback checks if credentials are accepted by the registry.
 // If credentials are incorrect this callback shall return ErrUnauthorized.
-type VerifyCredentialsCallback func(ctx context.Context, username, password, registry string) error
+type VerifyCredentialsCallback func(ctx context.Context, registry string, credentials Credentials) error

-func CheckAuth(ctx context.Context, username, password, registry string) error {
+func CheckAuth(ctx context.Context, registry string, credentials Credentials) error {
 	serverAddress := registry
 	if !strings.HasPrefix(serverAddress, "https://") && !strings.HasPrefix(serverAddress, "http://") {
 		serverAddress = "https://" + serverAddress
@ -53,8 +54,8 @@ func CheckAuth(ctx context.Context, username, password, registry string) error {
 	url := fmt.Sprintf("%s/v2", serverAddress)

 	authenticator := &authn.Basic{
-		Username: username,
-		Password: password,
+		Username: credentials.Username,
+		Password: credentials.Password,
 	}

 	reg, err := name.NewRegistry(registry)
@ -92,22 +93,74 @@ func CheckAuth(ctx context.Context, username, password, registry string) error {

 type ChooseCredentialHelperCallback func(available []string) (string, error)

-// NewCredentialsProvider returns new CredentialsProvider that tires to get credentials from `docker` and `func` config files.
+type credentialProviderConfig struct {
+	promptForCredentials        CredentialsCallback
+	verifyCredentials           VerifyCredentialsCallback
+	promptForCredentialStore    ChooseCredentialHelperCallback
+	additionalCredentialLoaders []CredentialsCallback
+}
+
+type CredentialProviderOptions func(opts *credentialProviderConfig)
+
+// WithPromptForCredentials sets custom callback that is supposed to
+// interactively ask for credentials in case the credentials cannot be found in configuration files.
+// The callback may be called multiple times in case incorrect credentials were returned before.
+func WithPromptForCredentials(cbk CredentialsCallback) CredentialProviderOptions {
+	return func(opts *credentialProviderConfig) {
+		opts.promptForCredentials = cbk
+	}
+}
+
+// WithVerifyCredentials sets custom callback for credentials validation.
+func WithVerifyCredentials(cbk VerifyCredentialsCallback) CredentialProviderOptions {
+	return func(opts *credentialProviderConfig) {
+		opts.verifyCredentials = cbk
+	}
+}
+
+// WithPromptForCredentialStore sets custom callback that is supposed to
+// interactively ask user which credentials store/helper is used to store credentials obtained
+// from user.
+func WithPromptForCredentialStore(cbk ChooseCredentialHelperCallback) CredentialProviderOptions {
+	return func(opts *credentialProviderConfig) {
+		opts.promptForCredentialStore = cbk
+	}
+}
+
+// WithAdditionalCredentialLoaders adds custom callbacks for credential retrieval.
+// The callbacks are supposed to be non-interactive as opposed to WithPromptForCredentials.
+//
+// This might be useful when credentials are shared with some other service.
+//
+// Example: OpenShift builtin registry shares credentials with the cluster (k8s) credentials.
+func WithAdditionalCredentialLoaders(loaders ...CredentialsCallback) CredentialProviderOptions {
+	return func(opts *credentialProviderConfig) {
+		opts.additionalCredentialLoaders = append(opts.additionalCredentialLoaders, loaders...)
+	}
+}
+
+// NewCredentialsProvider returns new CredentialsProvider that tires to get credentials from docker/func config files.
 //
 // In case getting credentials from the config files fails
-// the caller provided callback will be invoked to obtain credentials.
-// The callback may be called multiple times in case it returned credentials that are not correct (see verifyCredentials).
+// the caller provided callback (see WithPromptForCredentials) will be invoked to obtain credentials.
+// The callback may be called multiple times in case the returned credentials
+// are not correct (see WithVerifyCredentials).
 //
-// When the callback succeeds the credentials will be saved by using helper defined in the `func` config.
-// If the helper is not defined in the config the chooseCredentialHelper parameter will be used to pick one.
+// When the callback succeeds the credentials will be saved by using helper defined in the func config.
+// If the helper is not defined in the config file
+// it may be picked by provided callback (see WithPromptForCredentialStore).
 // The picked value will be saved in the func config.
 //
-// To verify that credentials are correct the verifyCredentials parameter is used.
-// If verifyCredentials is not set then CheckAuth will be used as a fallback.
-func NewCredentialsProvider(
-	getCredentials CredentialsCallback,
-	verifyCredentials VerifyCredentialsCallback,
-	chooseCredentialHelper ChooseCredentialHelperCallback) CredentialsProvider {
+// To verify that credentials are correct callback will be used (see WithVerifyCredentials).
+// If the callback is not set then CheckAuth will be used as a fallback.
+func NewCredentialsProvider(opts ...CredentialProviderOptions) CredentialsProvider {
+	var conf credentialProviderConfig
+
+	for _, o := range opts {
+		o(&conf)
+	}
+
+	askUser, verifyCredentials, chooseCredentialHelper := conf.promptForCredentials, conf.verifyCredentials, conf.promptForCredentialStore

 	if verifyCredentials == nil {
 		verifyCredentials = CheckAuth
@ -131,31 +184,40 @@ func NewCredentialsProvider(
 	}
 	dockerConfigPath := filepath.Join(home, ".docker", "config.json")

+	var authLoaders = []CredentialsCallback{
+		func(registry string) (Credentials, error) {
+			creds, err := config.GetCredentials(sys, registry)
+			if err != nil {
+				return Credentials{}, err
+			}
+			return Credentials{
+				Username: creds.Username,
+				Password: creds.Password,
+			}, nil
+		},
+		func(registry string) (Credentials, error) {
+			return getCredentialsByCredentialHelper(authFilePath, registry)
+		},
+		func(registry string) (Credentials, error) {
+			return getCredentialsByCredentialHelper(dockerConfigPath, registry)
+		},
+	}
+
+	authLoaders = append(conf.additionalCredentialLoaders, authLoaders...)
+
 	return func(ctx context.Context, registry string) (Credentials, error) {
 		result := Credentials{}

-		for _, load := range []func() (containersTypes.DockerAuthConfig, error){
-			func() (containersTypes.DockerAuthConfig, error) {
-				return config.GetCredentials(sys, registry)
-			},
-			func() (containersTypes.DockerAuthConfig, error) {
-				return getCredentialsByCredentialHelper(authFilePath, registry)
-			},
-			func() (containersTypes.DockerAuthConfig, error) {
-				return getCredentialsByCredentialHelper(dockerConfigPath, registry)
-			},
-		} {
-			var credentials containersTypes.DockerAuthConfig
-			credentials, err = load()
+		for _, load := range authLoaders {
+
+			result, err = load(registry)

 			if err != nil && !errors.Is(err, errCredentialsNotFound) {
 				return Credentials{}, err
 			}

-			if credentials != (containersTypes.DockerAuthConfig{}) {
-				result.Username, result.Password = credentials.Username, credentials.Password
-
-				err = verifyCredentials(ctx, result.Username, result.Password, registry)
+			if result != (Credentials{}) {
+				err = verifyCredentials(ctx, registry, result)
 				if err == nil {
 					return result, nil
 				} else {
@ -167,12 +229,12 @@ func NewCredentialsProvider(
 		}

 		for {
-			result, err = getCredentials(registry)
+			result, err = askUser(registry)
 			if err != nil {
 				return Credentials{}, err
 			}

-			err = verifyCredentials(ctx, result.Username, result.Password, registry)
+			err = verifyCredentials(ctx, registry, result)
 			if err == nil {
 				err = setCredentialsByCredentialHelper(authFilePath, registry, result.Username, result.Password)
 				if err != nil {
--- a/docker/pusher_test.go
+++ b/docker/pusher_test.go
@ -107,10 +107,11 @@ func TestNewCredentialsProvider(t *testing.T) {
 	}

 	type args struct {
-		credentialsCallback CredentialsCallback
-		verifyCredentials   VerifyCredentialsCallback
-		registry            string
-		setUpEnv            setUpEnv
+		promptUser        CredentialsCallback
+		verifyCredentials VerifyCredentialsCallback
+		additionalLoaders []CredentialsCallback
+		registry          string
+		setUpEnv          setUpEnv
 	}
 	tests := []struct {
 		name string
@ -120,47 +121,47 @@ func TestNewCredentialsProvider(t *testing.T) {
 		{
 			name: "test user callback correct password on first try",
 			args: args{
-				credentialsCallback: correctPwdCallback,
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "docker.io",
+				promptUser:        correctPwdCallback,
+				verifyCredentials: correctVerifyCbk,
+				registry:          "docker.io",
 			},
 			want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd},
 		},
 		{
 			name: "test user callback correct password on second try",
 			args: args{
-				credentialsCallback: pwdCbkFirstWrongThenCorrect(t),
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "docker.io",
+				promptUser:        pwdCbkFirstWrongThenCorrect(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "docker.io",
 			},
 			want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd},
 		},
 		{
 			name: "get quay-io credentials with func config populated",
 			args: args{
-				credentialsCallback: pwdCbkThatShallNotBeCalled(t),
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "quay.io",
-				setUpEnv:            withPopulatedFuncAuthConfig,
+				promptUser:        pwdCbkThatShallNotBeCalled(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "quay.io",
+				setUpEnv:          withPopulatedFuncAuthConfig,
 			},
 			want: Credentials{Username: quayIoUser, Password: quayIoUserPwd},
 		},
 		{
 			name: "get docker-io credentials with func config populated",
 			args: args{
-				credentialsCallback: pwdCbkThatShallNotBeCalled(t),
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "docker.io",
-				setUpEnv:            withPopulatedFuncAuthConfig,
+				promptUser:        pwdCbkThatShallNotBeCalled(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "docker.io",
+				setUpEnv:          withPopulatedFuncAuthConfig,
 			},
 			want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd},
 		},
 		{
 			name: "get quay-io credentials with docker config populated",
 			args: args{
-				credentialsCallback: pwdCbkThatShallNotBeCalled(t),
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "quay.io",
+				promptUser:        pwdCbkThatShallNotBeCalled(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "quay.io",
 				setUpEnv: all(
 					withPopulatedDockerAuthConfig,
 					setUpMockHelper("docker-credential-mock", helperWithQuayIO)),
@ -170,10 +171,20 @@ func TestNewCredentialsProvider(t *testing.T) {
 		{
 			name: "get docker-io credentials with docker config populated",
 			args: args{
-				credentialsCallback: pwdCbkThatShallNotBeCalled(t),
-				verifyCredentials:   correctVerifyCbk,
-				registry:            "docker.io",
-				setUpEnv:            withPopulatedDockerAuthConfig,
+				promptUser:        pwdCbkThatShallNotBeCalled(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "docker.io",
+				setUpEnv:          withPopulatedDockerAuthConfig,
+			},
+			want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd},
+		},
+		{
+			name: "get docker-io credentials from custom loader",
+			args: args{
+				promptUser:        pwdCbkThatShallNotBeCalled(t),
+				verifyCredentials: correctVerifyCbk,
+				registry:          "docker.io",
+				additionalLoaders: []CredentialsCallback{correctPwdCallback},
 			},
 			want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd},
 		},
@ -186,7 +197,10 @@ func TestNewCredentialsProvider(t *testing.T) {
 				defer tt.args.setUpEnv(t)()
 			}

-			credentialsProvider := NewCredentialsProvider(tt.args.credentialsCallback, tt.args.verifyCredentials, nil)
+			credentialsProvider := NewCredentialsProvider(
+				WithPromptForCredentials(tt.args.promptUser),
+				WithVerifyCredentials(tt.args.verifyCredentials),
+				WithAdditionalCredentialLoaders(tt.args.additionalLoaders...))
 			got, err := credentialsProvider(context.Background(), tt.args.registry)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
@ -228,7 +242,10 @@ func TestCredentialsProviderSavingFromUserInput(t *testing.T) {
 		return "", errors.New("this callback shall not be invoked")
 	}

-	credentialsProvider := NewCredentialsProvider(pwdCbk, correctVerifyCbk, chooseNoStore)
+	credentialsProvider := NewCredentialsProvider(
+		WithPromptForCredentials(pwdCbk),
+		WithVerifyCredentials(correctVerifyCbk),
+		WithPromptForCredentialStore(chooseNoStore))
 	_, err := credentialsProvider(context.Background(), "docker.io")
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@ -244,7 +261,10 @@ func TestCredentialsProviderSavingFromUserInput(t *testing.T) {
 	if credsInStore != 0 {
 		t.Errorf("expected to have zero credentials in store, but has: %d", credsInStore)
 	}
-	credentialsProvider = NewCredentialsProvider(pwdCbk, correctVerifyCbk, chooseMockStore)
+	credentialsProvider = NewCredentialsProvider(
+		WithPromptForCredentials(pwdCbk),
+		WithVerifyCredentials(correctVerifyCbk),
+		WithPromptForCredentialStore(chooseMockStore))
 	_, err = credentialsProvider(context.Background(), "docker.io")
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@ -263,9 +283,10 @@ func TestCredentialsProviderSavingFromUserInput(t *testing.T) {
 	if len(l) != 1 {
 		t.Errorf("expected to have exactly one credentials in store, but has: %d", credsInStore)
 	}
-	credentialsProvider = NewCredentialsProvider(pwdCbkThatShallNotBeCalled(t),
-		correctVerifyCbk,
-		shallNotBeInvoked)
+	credentialsProvider = NewCredentialsProvider(
+		WithPromptForCredentials(pwdCbkThatShallNotBeCalled(t)),
+		WithVerifyCredentials(correctVerifyCbk),
+		WithPromptForCredentialStore(shallNotBeInvoked))
 	_, err = credentialsProvider(context.Background(), "docker.io")
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@ -381,7 +402,8 @@ func correctPwdCallback(registry string) (Credentials, error) {
 	return Credentials{}, errors.New("this cbk don't know the pwd")
 }

-func correctVerifyCbk(ctx context.Context, username, password, registry string) error {
+func correctVerifyCbk(ctx context.Context, registry string, creds Credentials) error {
+	username, password := creds.Username, creds.Password
 	if username == dockerIoUser && password == dockerIoUserPwd && registry == "docker.io" {
 		return nil
 	}
@ -738,7 +760,11 @@ func TestCheckAuth(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := CheckAuth(tt.args.ctx, tt.args.username, tt.args.password, tt.args.registry); (err != nil) != tt.wantErr {
+			creds := Credentials{
+				Username: tt.args.username,
+				Password: tt.args.password,
+			}
+			if err := CheckAuth(tt.args.ctx, tt.args.registry, creds); (err != nil) != tt.wantErr {
 				t.Errorf("CheckAuth() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})