From f712cc1c867e61fcd971debb4c641ae8ae43f493 Mon Sep 17 00:00:00 2001 From: sujeito-operator Date: Wed, 19 Aug 2026 08:04:14 +0000 Subject: [PATCH] fix(test): take the registry keychain from the caller, not the host Three unit tests resolve registry credentials through authn.DefaultKeychain, which reads the host's ~/.docker/config.json. A developer whose config sets a credsStore sends that lookup to docker-credential-, and nix/apps.nix runs the test app with inheritPath = false, so the helper is not on PATH: error getting credentials - err: exec: "docker-credential-desktop": executable file not found in $PATH CI never hits it because the runner has no docker config, so the tests fail only on developer machines: TestFindImageTagForVersionConstraint, TestKCLBuild and TestGoTemplatingBuild. The keychain is now supplied by the caller wherever a test drives the pull, defaulting to authn.DefaultKeychain at each construction site so production behaviour is unchanged: * kclBuilder, goTemplatingBuilder, pythonBuilder and goBuilder gain a keychain field alongside the transport field they already had, and baseImageForArch takes it as an argument. * findImageTagForVersionConstraint accepts crane options and passes them to crane.ListTags. The tests inject an anonymous keychain, following the anonymousKeychain helper internal/project/push_test.go already uses on the push path. It records whether it was consulted and the tests assert that it was, so a future fall back to authn.DefaultKeychain fails on any host rather than only on one with a docker config. Setting DOCKER_CONFIG in the tests was the smaller change but is not available: t.Setenv panics in TestKCLBuild and TestGoTemplatingBuild because they call t.Parallel, and the variable is process-global while those tests run alongside others. Also report the error in TestFindImageTagForVersionConstraint. The assertion printed only the empty result, so a credential lookup that never reached the registry read as an unreachable registry. Fixes #282 Signed-off-by: sujeito-operator --- cmd/crossplane/validate/image.go | 6 ++- cmd/crossplane/validate/image_test.go | 46 +++++++++++++++++++-- internal/project/functions/build_test.go | 36 ++++++++++++++++ internal/project/functions/go.go | 4 +- internal/project/functions/go_templating.go | 5 ++- internal/project/functions/kcl.go | 12 ++++-- internal/project/functions/python.go | 5 ++- 7 files changed, 102 insertions(+), 12 deletions(-) diff --git a/cmd/crossplane/validate/image.go b/cmd/crossplane/validate/image.go index 4177e6a8..4e642585 100644 --- a/cmd/crossplane/validate/image.go +++ b/cmd/crossplane/validate/image.go @@ -168,10 +168,12 @@ func convertToSemver(tags []string) []*semver.Version { } // findImageTagForVersionConstraint fetches the latest tag for an image with a version constraint. +// Callers may pass crane options; tests use this to supply an anonymous +// keychain so the lookup does not consult the host's docker config. // image should be a validated image name with the format: /:. // where can be a version constraint. // if is already an exact version, the same image string is returned back. -func findImageTagForVersionConstraint(image string) (string, error) { +func findImageTagForVersionConstraint(image string, opts ...crane.Option) (string, error) { imageBase, imageTag := separateImageTag(image) // Check if the tag is a constraint or already a valid semantic version @@ -194,7 +196,7 @@ func findImageTagForVersionConstraint(image string) (string, error) { } // Fetch all image tags - tags, err := crane.ListTags(imageBase) + tags, err := crane.ListTags(imageBase, opts...) if err != nil { return "", errors.Wrapf(err, "cannot fetch tags for the image %s", imageBase) } diff --git a/cmd/crossplane/validate/image_test.go b/cmd/crossplane/validate/image_test.go index c91f2c43..378dd63f 100644 --- a/cmd/crossplane/validate/image_test.go +++ b/cmd/crossplane/validate/image_test.go @@ -21,9 +21,34 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync/atomic" "testing" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/crane" ) +// anonymousKeychain returns authn.Anonymous for every request, and records that +// it was asked. The default keychain would consult the host's docker config: if +// that sets a credsStore, resolving credentials shells out to +// docker-credential-, which is not on PATH under `nix run .#test` +// (nix/apps.nix sets inheritPath = false), so the tag lookup fails on a +// developer machine while passing in CI. internal/project/push_test.go carries +// the same helper for the push path. +// +// The used flag is what keeps this honest: if the option stops being threaded +// through to crane, this keychain is never consulted and the test fails on any +// host, with or without a docker config. +type anonymousKeychain struct { + used atomic.Bool +} + +func (k *anonymousKeychain) Resolve(authn.Resource) (authn.Authenticator, error) { + k.used.Store(true) + + return authn.Anonymous, nil +} + func TestFindImageTagForVersionConstraint(t *testing.T) { repoName := "ubuntu" responseTags := []byte(`{"tags":["1.2.3","4.5.6"]}`) @@ -77,6 +102,8 @@ func TestFindImageTagForVersionConstraint(t *testing.T) { }, } + keychain := &anonymousKeychain{} + for name, tc := range cases { t.Run(name, func(t *testing.T) { tagsPath := fmt.Sprintf("/v2/%s/tags/list", repoName) @@ -107,20 +134,33 @@ func TestFindImageTagForVersionConstraint(t *testing.T) { host = tc.host } - image, err := findImageTagForVersionConstraint(fmt.Sprintf("%s/%s:%s", host, repoName, tc.constraint)) + image, err := findImageTagForVersionConstraint( + fmt.Sprintf("%s/%s:%s", host, repoName, tc.constraint), + crane.WithAuthFromKeychain(keychain), + ) expectedImage := "" if !tc.expectError { expectedImage = fmt.Sprintf("%s/%s", host, tc.expectedImage) } - if tc.expectError && err == nil { + switch { + case tc.expectError && err == nil: t.Errorf("[%s] expected: error\n", name) - } else if expectedImage != image { + case !tc.expectError && err != nil: + // Report the error rather than only the empty result: a + // credential lookup that never reached the registry used to + // read here as an unreachable registry. + t.Errorf("[%s] unexpected error: %v\n", name, err) + case expectedImage != image: t.Errorf("[%s] expected: %s, got: %s\n", name, expectedImage, image) } }) } + + if !keychain.used.Load() { + t.Error("tags were listed without the injected keychain; the lookup fell back to the host's docker config") + } } func TestIsErrBaseLayerNotFound(t *testing.T) { diff --git a/internal/project/functions/build_test.go b/internal/project/functions/build_test.go index 4c90a9fc..81d8898d 100644 --- a/internal/project/functions/build_test.go +++ b/internal/project/functions/build_test.go @@ -25,9 +25,11 @@ import ( "reflect" "slices" "strings" + "sync/atomic" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -179,9 +181,11 @@ func TestKCLBuild(t *testing.T) { t.Fatal(err) } + keychain := &anonymousKeychain{} b := &kclBuilder{ baseImage: baseImageRef.String(), transport: regSrv.Client().Transport, + keychain: keychain, configStore: clixpkg.NewStaticImageConfigStore(nil), } projFS := afero.FromIOFS{FS: kclFunction} @@ -206,6 +210,10 @@ func TestKCLBuild(t *testing.T) { t.Errorf("env missing FUNCTION_KCL_DEFAULT_SOURCE=/src; got %v", cfgFile.Config.Env) } + if !keychain.used.Load() { + t.Error("base image was pulled without the injected keychain; the builder fell back to the host's docker config") + } + verifyCodeLayer(t, fnImg, afero.NewBasePathFs(projFS, "testdata/kcl-function"), "/src") } @@ -249,9 +257,11 @@ func TestGoTemplatingBuild(t *testing.T) { t.Fatal(err) } + keychain := &anonymousKeychain{} b := &goTemplatingBuilder{ baseImage: baseImageRef.String(), transport: regSrv.Client().Transport, + keychain: keychain, configStore: clixpkg.NewStaticImageConfigStore(nil), } projFS := afero.FromIOFS{FS: goTemplatingFunction} @@ -276,6 +286,10 @@ func TestGoTemplatingBuild(t *testing.T) { t.Errorf("env missing FUNCTION_GO_TEMPLATING_DEFAULT_SOURCE=/src; got %v", cfgFile.Config.Env) } + if !keychain.used.Load() { + t.Error("base image was pulled without the injected keychain; the builder fell back to the host's docker config") + } + verifyCodeLayer(t, fnImg, afero.NewBasePathFs(projFS, "testdata/go-templating-function"), "/src") } @@ -328,3 +342,25 @@ func verifyCodeLayer(t *testing.T, img v1.Image, sourceFS afero.Fs, destPrefix s return nil }) } + +// anonymousKeychain returns authn.Anonymous for every request, and records +// that it was asked. A real keychain would consult the host's docker config: +// if that sets a credsStore, resolving credentials shells out to +// docker-credential-, which is not on PATH under `nix run .#test` +// (nix/apps.nix sets inheritPath = false), and the base-image pull fails on a +// developer machine while passing in CI. internal/project/push_test.go carries +// the same helper for the push path. +// +// The used flag is what keeps this honest: if a builder stops threading its +// keychain through and falls back to authn.DefaultKeychain, this keychain is +// never consulted and the test fails on any host, with or without a docker +// config. +type anonymousKeychain struct { + used atomic.Bool +} + +func (k *anonymousKeychain) Resolve(authn.Resource) (authn.Authenticator, error) { + k.used.Store(true) + + return authn.Anonymous, nil +} diff --git a/internal/project/functions/go.go b/internal/project/functions/go.go index cc2f0f29..eab00511 100644 --- a/internal/project/functions/go.go +++ b/internal/project/functions/go.go @@ -41,6 +41,7 @@ import ( type goBuilder struct { baseImage string transport http.RoundTripper + keychain authn.Keychain configStore xpkg.ConfigStore } @@ -79,7 +80,7 @@ func (b *goBuilder) Build(ctx context.Context, c BuildContext) ([]v1.Image, erro if err != nil { return nil, nil, err } - img, err := remote.Index(ref, remote.WithTransport(b.transport), remote.WithAuthFromKeychain(authn.DefaultKeychain)) + img, err := remote.Index(ref, remote.WithTransport(b.transport), remote.WithAuthFromKeychain(b.keychain)) return ref, img, err }), build.WithPlatforms(platforms...), @@ -133,6 +134,7 @@ func newGoBuilder(imageConfigs []pkgv1beta1.ImageConfig) *goBuilder { return &goBuilder{ baseImage: "gcr.io/distroless/static-debian12@sha256:a9329520abc449e3b14d5bc3a6ffae065bdde0f02667fa10880c49b35c109fd1", transport: http.DefaultTransport, + keychain: authn.DefaultKeychain, configStore: clixpkg.NewStaticImageConfigStore(imageConfigs), } } diff --git a/internal/project/functions/go_templating.go b/internal/project/functions/go_templating.go index ce5a7710..22a59f7f 100644 --- a/internal/project/functions/go_templating.go +++ b/internal/project/functions/go_templating.go @@ -25,6 +25,7 @@ import ( "path/filepath" "slices" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" @@ -46,6 +47,7 @@ import ( type goTemplatingBuilder struct { baseImage string transport http.RoundTripper + keychain authn.Keychain configStore xpkg.ConfigStore } @@ -111,7 +113,7 @@ func (b *goTemplatingBuilder) Build(ctx context.Context, c BuildContext) ([]v1.I eg, _ := errgroup.WithContext(ctx) for i, arch := range c.Architectures { eg.Go(func() error { - baseImg, err := baseImageForArch(baseRef, arch, b.transport) + baseImg, err := baseImageForArch(baseRef, arch, b.transport, b.keychain) if err != nil { return errors.Wrap(err, "failed to fetch go-templating base image") } @@ -153,6 +155,7 @@ func (b *goTemplatingBuilder) Build(ctx context.Context, c BuildContext) ([]v1.I func newGoTemplatingBuilder(imageConfigs []pkgv1beta1.ImageConfig) *goTemplatingBuilder { return &goTemplatingBuilder{ transport: http.DefaultTransport, + keychain: authn.DefaultKeychain, baseImage: "xpkg.crossplane.io/crossplane-contrib/function-go-templating:v0.12.0", configStore: clixpkg.NewStaticImageConfigStore(imageConfigs), } diff --git a/internal/project/functions/kcl.go b/internal/project/functions/kcl.go index d92cbe8f..525d20a4 100644 --- a/internal/project/functions/kcl.go +++ b/internal/project/functions/kcl.go @@ -53,6 +53,7 @@ const ( type kclBuilder struct { baseImage string transport http.RoundTripper + keychain authn.Keychain configStore xpkg.ConfigStore } @@ -85,7 +86,7 @@ func (b *kclBuilder) Build(ctx context.Context, c BuildContext) ([]v1.Image, err eg, _ := errgroup.WithContext(ctx) for i, arch := range c.Architectures { eg.Go(func() error { - baseImg, err := baseImageForArch(baseRef, arch, b.transport) + baseImg, err := baseImageForArch(baseRef, arch, b.transport, b.keychain) if err != nil { return errors.Wrap(err, "failed to fetch KCL base image") } @@ -129,12 +130,14 @@ func (b *kclBuilder) Build(ctx context.Context, c BuildContext) ([]v1.Image, err // baseImageForArch pulls the image with the given ref, and returns a version of // it suitable for use as a function base image. Package and examples layers -// will be removed if present. -func baseImageForArch(ref name.Reference, arch string, transport http.RoundTripper) (v1.Image, error) { +// will be removed if present. The keychain is supplied by the caller so that +// tests can pull from a local registry without consulting the host's docker +// config. +func baseImageForArch(ref name.Reference, arch string, transport http.RoundTripper, keychain authn.Keychain) (v1.Image, error) { img, err := remote.Image(ref, remote.WithPlatform(v1.Platform{ OS: "linux", Architecture: arch, - }), remote.WithTransport(transport), remote.WithAuthFromKeychain(authn.DefaultKeychain)) + }), remote.WithTransport(transport), remote.WithAuthFromKeychain(keychain)) if err != nil { return nil, errors.Wrap(err, "failed to pull image") } @@ -208,6 +211,7 @@ func newKCLBuilder(imageConfigs []v1beta1.ImageConfig) *kclBuilder { return &kclBuilder{ baseImage: "xpkg.crossplane.io/crossplane-contrib/function-kcl:v0.12.1", transport: http.DefaultTransport, + keychain: authn.DefaultKeychain, configStore: clixpkg.NewStaticImageConfigStore(imageConfigs), } } diff --git a/internal/project/functions/python.go b/internal/project/functions/python.go index 55fd0f38..4ce11f58 100644 --- a/internal/project/functions/python.go +++ b/internal/project/functions/python.go @@ -26,6 +26,7 @@ import ( "path/filepath" "strings" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" @@ -90,6 +91,7 @@ type pythonBuilder struct { buildImage string runtimeImage string transport http.RoundTripper + keychain authn.Keychain configStore xpkg.ConfigStore } @@ -137,7 +139,7 @@ func (b *pythonBuilder) Build(ctx context.Context, c BuildContext) ([]v1.Image, eg, _ := errgroup.WithContext(ctx) for i, arch := range c.Architectures { eg.Go(func() error { - baseImg, err := baseImageForArch(runtimeRef, arch, b.transport) + baseImg, err := baseImageForArch(runtimeRef, arch, b.transport, b.keychain) if err != nil { return errors.Wrap(err, "failed to fetch python runtime base image") } @@ -290,6 +292,7 @@ func newPythonBuilder(imageConfigs []pkgv1beta1.ImageConfig) *pythonBuilder { buildImage: pythonBuildImage, runtimeImage: pythonRuntimeImage, transport: http.DefaultTransport, + keychain: authn.DefaultKeychain, configStore: clixpkg.NewStaticImageConfigStore(imageConfigs), } }