Clean up auth for host builder (knative#3298)

Signed-off-by: Matej Vašek <[email protected]>

Author: matejvasek

cmd/build.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"strings"
@@ -214,8 +213,6 @@ For more options, run 'func build --help'`, err)
 	}
 	f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc)
 
-	cmd.SetContext(cfg.WithValues(cmd.Context())) // Some optional settings are passed via context
-
 	// Client
 	clientOptions, err := cfg.clientOptions()
 	if err != nil {
@@ -245,16 +242,6 @@ For more options, run 'func build --help'`, err)
 	return f.Stamp()
 }
 
-// WithValues returns a context populated with values from the build config
-// which are provided to the system via the context.
-func (c buildConfig) WithValues(ctx context.Context) context.Context {
-	// Push
-	ctx = context.WithValue(ctx, fn.PushUsernameKey{}, c.Username)
-	ctx = context.WithValue(ctx, fn.PushPasswordKey{}, c.Password)
-	ctx = context.WithValue(ctx, fn.PushTokenKey{}, c.Token)
-	return ctx
-}
-
 type buildConfig struct {
 	// Globals (builder, confirm, registry, verbose)
 	config.Global

cmd/build_test.go

@@ -89,12 +89,6 @@ func TestBuild_RegistryOrImageRequired(t *testing.T) {
 	testRegistryOrImageRequired(NewBuildCmd, t)
 }
 
-// TestBuild_Authentication ensures that Token and Username/Password auth
-// propagate to pushers which support them.
-func TestBuild_Authentication(t *testing.T) {
-	testAuthentication(NewBuildCmd, t)
-}
-
 // TestBuild_BaseImage ensures that base image is used only with the right
 // builders and propagates into f.Build.BaseImage
 func TestBuild_BaseImage(t *testing.T) {

cmd/client.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"os"
 
+	"github.com/ory/viper"
+
 	"knative.dev/func/cmd/prompt"
 	"knative.dev/func/pkg/buildpacks"
 	"knative.dev/func/pkg/config"
@@ -106,6 +108,23 @@ func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath
 	additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...)
 	additionalLoaders = append(additionalLoaders, k8s.GetECRCredentialLoader()...)
 	additionalLoaders = append(additionalLoaders, k8s.GetACRCredentialLoader()...)
+
+	additionalLoaders = append(additionalLoaders,
+		func(registry string) (oci.Credentials, error) {
+			uname := viper.GetString("username")
+			passw := viper.GetString("password")
+			token := viper.GetString("token")
+			if (uname != "" && passw != "") || token != "" {
+				return oci.Credentials{
+					Username: uname,
+					Password: passw,
+					Token:    token,
+				}, nil
+			}
+			return oci.Credentials{}, creds.ErrCredentialsNotFound
+		},
+	)
+
 	options := []creds.Opt{
 		creds.WithPromptForCredentials(prompt.NewPromptForCredentials(os.Stdin, os.Stdout, os.Stderr)),
 		creds.WithPromptForCredentialStore(prompt.NewPromptForCredentialStore()),

cmd/deploy.go

@@ -404,7 +404,6 @@ For more options, run 'func deploy --help'`, err)
 	if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
 		return
 	}
-	cmd.SetContext(cfg.WithValues(cmd.Context())) // Some optional settings are passed via context
 
 	changingNamespace := func(f fn.Function) bool {
 		// We're changing namespace if:

cmd/deploy_test.go

@@ -1501,74 +1501,6 @@ func testRegistryOrImageRequired(cmdFn commandConstructor, t *testing.T) {
 	}
 }
 
-// TestDeploy_Authentication ensures that Token and Username/Password auth
-// propagate their values to pushers which support them.
-func TestDeploy_Authentication(t *testing.T) {
-	testAuthentication(NewDeployCmd, t)
-}
-
-func testAuthentication(cmdFn commandConstructor, t *testing.T) {
-	// This test is currently focused on ensuring the flags for
-	// explicit credentials (bearer token and username/password) are respected
-	// and propagated to pushers which support this authentication method.
-	// Integration tests must be used to ensure correct integration between
-	// the system and credential helpers (Docker, ecs, acs)
-	t.Helper()
-
-	root := FromTempDirectory(t)
-	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	var (
-		testUser  = "alice"
-		testPass  = "123"
-		testToken = "example.jwt.token"
-	)
-
-	// Basic Auth: username/password
-	// -----------------------------
-	pusher := mock.NewPusher()
-	pusher.PushFn = func(ctx context.Context, _ fn.Function) (string, error) {
-		username, _ := ctx.Value(fn.PushUsernameKey{}).(string)
-		password, _ := ctx.Value(fn.PushPasswordKey{}).(string)
-
-		if username != testUser || password != testPass {
-			t.Fatalf("expected username %q, password %q.  Got %q, %q", testUser, testPass, username, password)
-		}
-
-		return "", nil
-	}
-
-	cmd := cmdFn(NewTestClient(fn.WithPusher(pusher)))
-	cmd.SetArgs([]string{"--builder", "host", "--username", testUser, "--password", testPass})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-
-	// Basic Auth: token
-	// -----------------------------
-	pusher = mock.NewPusher()
-	pusher.PushFn = func(ctx context.Context, _ fn.Function) (string, error) {
-		token, _ := ctx.Value(fn.PushTokenKey{}).(string)
-
-		if token != testToken {
-			t.Fatalf("expected token %q, got %q", testToken, token)
-		}
-
-		return "", nil
-	}
-
-	cmd = cmdFn(NewTestClient(fn.WithPusher(pusher)))
-
-	cmd.SetArgs([]string{"--builder", "host", "--token", testToken})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-
-}
-
 // TestDeploy_RemoteBuildURLPermutations ensures that the remote, build and git-url flags
 // are properly respected for all permutations, including empty.
 func TestDeploy_RemoteBuildURLPermutations(t *testing.T) {

pkg/creds/credentials.go

@@ -231,6 +231,7 @@ func NewCredentialsProvider(configPath string, opts ...Opt) oci.CredentialsProvi
 			return oci.Credentials{
 				Username: creds.Username,
 				Password: creds.Password,
+				Token:    creds.IdentityToken,
 			}, nil
 		})
 	defaultCredentialLoaders = append(defaultCredentialLoaders,
@@ -247,6 +248,7 @@ func NewCredentialsProvider(configPath string, opts ...Opt) oci.CredentialsProvi
 			return oci.Credentials{
 				Username: creds.Username,
 				Password: creds.Password,
+				Token:    creds.IdentityToken,
 			}, nil
 		})
 	defaultCredentialLoaders = append(defaultCredentialLoaders,

pkg/functions/client.go

@@ -96,18 +96,6 @@ type Pusher interface {
 	Push(ctx context.Context, f Function) (string, error)
 }
 
-// PushUsernameKey is a type available for use to communicate a basic
-// authentication username to pushers which support this method.
-type PushUsernameKey struct{}
-
-// PushPasswordKey is a type available for use as a context key for
-// providing a basic auth password to pushers which support this method.
-type PushPasswordKey struct{}
-
-// PushTokenKey is a type available for use as a context key for providing a
-// token (for example a jwt bearer token) to pushers which support this method.
-type PushTokenKey struct{}
-
 // Deployer of function source to running status.
 type Deployer interface {
 	// Deploy a function of given name, using given backing image.

pkg/oci/pusher.go

@@ -14,7 +14,6 @@ import (
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/layout"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
-	"github.com/pkg/errors"
 	progress "github.com/schollz/progressbar/v3"
 
 	fn "knative.dev/func/pkg/functions"
@@ -23,6 +22,15 @@ import (
 type Credentials struct {
 	Username string
 	Password string
+	Token    string
+}
+
+func (c Credentials) Authorization() (*authn.AuthConfig, error) {
+	return &authn.AuthConfig{
+		Username:      c.Username,
+		Password:      c.Password,
+		IdentityToken: c.Token,
+	}, nil
 }
 
 type CredentialsProvider func(ctx context.Context, image string) (Credentials, error)
@@ -35,8 +43,6 @@ type Pusher struct {
 	credentialsProvider CredentialsProvider
 
 	Insecure bool
-	Token    string
-	Username string
 	Verbose  bool
 
 	updates chan v1.Update
@@ -169,45 +175,8 @@ func (p *Pusher) writeIndex(ctx context.Context, ref name.Reference, ii v1.Image
 	}
 
 	if !p.Anonymous {
-		a, err := p.authOption(ctx, creds)
-		if err != nil {
-			return err
-		}
-		oo = append(oo, a)
+		oo = append(oo, remote.WithAuth(creds))
 	}
 
 	return remote.WriteIndex(ref, ii, oo...)
 }
-
-// authOption selects an appropriate authentication option.
-// If user provided = basic auth (secret is password)
-// If only secret provided = bearer token auth
-// If neither are provided = creds from credentials provider
-// which performs the following in order:
-// - Default Keychain (docker and podman config files)
-// - Google Keychain
-// - TODO: ECR Amazon
-// - TODO: ACR Azure
-// - interactive prompt for username and password
-func (p *Pusher) authOption(ctx context.Context, creds Credentials) (remote.Option, error) {
-
-	// Basic Auth if provided
-	username, _ := ctx.Value(fn.PushUsernameKey{}).(string)
-	password, _ := ctx.Value(fn.PushPasswordKey{}).(string)
-	token, _ := ctx.Value(fn.PushTokenKey{}).(string)
-	if username != "" && token != "" {
-		return nil, errors.New("only one of username/password or token authentication allowed.  Received both a token and username")
-	} else if token != "" {
-		return remote.WithAuth(&authn.Bearer{Token: token}), nil
-	} else if username != "" {
-		return remote.WithAuth(&authn.Basic{Username: username, Password: password}), nil
-	}
-
-	// Use provided credentials if available or prompt for them
-	if creds.Username != "" && creds.Password != "" {
-		return remote.WithAuth(&authn.Basic{Username: creds.Username, Password: creds.Password}), nil
-	}
-
-	// Return anonymous auth when no credentials are provided (e.g., for localhost registries)
-	return remote.WithAuth(authn.Anonymous), nil
-}

pkg/oci/pusher_test.go

@@ -123,11 +123,21 @@ func TestPusher_BasicAuth(t *testing.T) {
 	}
 	defer server.Close()
 
+	pusher := NewPusher(false, false, verbose,
+		WithCredentialsProvider(func(ctx context.Context, image string) (Credentials, error) {
+			return Credentials{
+				Username: username,
+				Password: password,
+			}, nil
+		}),
+	)
+
 	// Client
 	// initialized with an OCI builder and pusher.
 	client := fn.New(
 		fn.WithBuilder(NewBuilder("", verbose)),
-		fn.WithPusher(NewPusher(false, false, verbose)))
+		fn.WithPusher(pusher),
+	)
 
 	// Function
 	// Built and tagged to push to the mock registry
@@ -144,15 +154,7 @@ func TestPusher_BasicAuth(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Push
-	// Enables optional basic authentication via the push context to use instead
-	// of the default behavior of using the multi-auth chain of config files
-	// and various known credentials managers.
-	ctx := context.Background()
-	ctx = context.WithValue(ctx, fn.PushUsernameKey{}, username)
-	ctx = context.WithValue(ctx, fn.PushPasswordKey{}, password)
-
-	if _, _, err = client.Push(ctx, f); err != nil {
+	if _, _, err = client.Push(context.Background(), f); err != nil {
 		t.Fatal(err)
 	}