From a5c263b2475300b5cca61f5a2ab8b246a926f914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Va=C5=A1ek?= Date: Mon, 15 Dec 2025 20:28:02 +0100 Subject: [PATCH] Clean up auth for host builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Matej VaĊĦek --- cmd/build.go | 13 -------- cmd/build_test.go | 6 ---- cmd/client.go | 19 +++++++++++ cmd/deploy.go | 1 - cmd/deploy_test.go | 68 ---------------------------------------- pkg/creds/credentials.go | 2 ++ pkg/functions/client.go | 12 ------- pkg/oci/pusher.go | 51 ++++++------------------------ pkg/oci/pusher_test.go | 22 +++++++------ 9 files changed, 43 insertions(+), 151 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index d80c212263..b2cc40852c 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -1,7 +1,6 @@ package cmd import ( - "context" "errors" "fmt" "strings" @@ -212,8 +211,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 { @@ -243,16 +240,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 diff --git a/cmd/build_test.go b/cmd/build_test.go index f0f5d11341..d9b1264801 100644 --- a/cmd/build_test.go +++ b/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) { diff --git a/cmd/client.go b/cmd/client.go index 9c4fc81987..88589e9c09 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -5,6 +5,8 @@ import ( "net/http" "os" + "github.com/ory/viper" + "knative.dev/func/cmd/prompt" "knative.dev/func/pkg/builders/buildpacks" "knative.dev/func/pkg/config" @@ -105,6 +107,23 @@ func newCredentialsProvider(configPath string, t http.RoundTripper) oci.Credenti 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()), diff --git a/cmd/deploy.go b/cmd/deploy.go index 42cb032140..474923d2fe 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -313,7 +313,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: diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 38e2109723..0046fe5e75 100644 --- a/cmd/deploy_test.go +++ b/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) { diff --git a/pkg/creds/credentials.go b/pkg/creds/credentials.go index 3a83a371f8..e65a79274c 100644 --- a/pkg/creds/credentials.go +++ b/pkg/creds/credentials.go @@ -220,6 +220,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, @@ -233,6 +234,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, diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 6bb46612e3..64a42b6220 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -94,18 +94,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. diff --git a/pkg/oci/pusher.go b/pkg/oci/pusher.go index 0487ca69f7..07b97d4791 100644 --- a/pkg/oci/pusher.go +++ b/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 -} diff --git a/pkg/oci/pusher_test.go b/pkg/oci/pusher_test.go index b6a71c1881..d6fd3b6701 100644 --- a/pkg/oci/pusher_test.go +++ b/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) }