diff --git a/pkg/envtest/setup/env/assets.go b/pkg/envtest/setup/env/assets.go index 94f980d2fe..e1754f7322 100644 --- a/pkg/envtest/setup/env/assets.go +++ b/pkg/envtest/setup/env/assets.go @@ -19,27 +19,39 @@ var expectedExecutables = []string{ // TryUseAssetsFromPath attempts to use the assets from the provided path if they match the spec. // If they do not, or if some executable is missing, it returns an empty string. -func (e *Env) TryUseAssetsFromPath(ctx context.Context, spec versions.Spec, path string) (*versions.Concrete, bool) { +func (e *Env) TryUseAssetsFromPath(ctx context.Context, spec versions.Spec, path string) (versions.Spec, bool) { v, err := versions.FromPath(path) if err != nil { - log.FromContext(ctx).Error(err, "Unable to use assets from path, ignoring", "path", path) - return nil, false + ok, checkErr := e.hasAllExecutables(path) + log.FromContext(ctx).Info("has all executables", "ok", ok, "err", checkErr) + if checkErr != nil { + log.FromContext(ctx).Error(errors.Join(err, checkErr), "Failed checking if assets path has all binaries, ignoring", "path", path) + return versions.Spec{}, false + } else if ok { + // If the path has all executables, we can use it even if we can't parse the version. + // The user explicitly asked for this path, so set the version to a wildcard so that + // it passes checks downstream. + return versions.AnyVersion, true + } + + log.FromContext(ctx).Error(errors.Join(err, errors.New("some required binaries missing")), "Unable to use assets from path, ignoring", "path", path) + return versions.Spec{}, false } if !spec.Matches(*v) { log.FromContext(ctx).Error(nil, "Assets path does not match spec, ignoring", "path", path, "spec", spec) - return nil, false + return versions.Spec{}, false } if ok, err := e.hasAllExecutables(path); err != nil { log.FromContext(ctx).Error(err, "Failed checking if assets path has all binaries, ignoring", "path", path) - return nil, false + return versions.Spec{}, false } else if !ok { log.FromContext(ctx).Error(nil, "Assets path is missing some executables, ignoring", "path", path) - return nil, false + return versions.Spec{}, false } - return v, true + return versions.Spec{Selector: v}, true } func (e *Env) hasAllExecutables(path string) (bool, error) { diff --git a/pkg/envtest/setup/env/local.go b/pkg/envtest/setup/env/local.go index e21fbec3ba..fd50e2933f 100644 --- a/pkg/envtest/setup/env/local.go +++ b/pkg/envtest/setup/env/local.go @@ -11,10 +11,10 @@ import ( // or nil if no such version is available. // // Note that a nil error does not guarantee that a version was found! -func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platform versions.Platform) (*versions.Concrete, error) { +func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platform versions.Platform) (store.Item, error) { localVersions, err := e.Store.List(ctx, store.Filter{Version: spec, Platform: platform}) if err != nil { - return nil, err + return store.Item{}, err } // NB(tomasaschan): this assumes the following of the contract for store.List // * only results matching the provided filter are returned @@ -23,11 +23,11 @@ func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platfo // we want, and there's no need to iterate through the items again. if len(localVersions) > 0 { // copy to avoid holding on to the entire slice - result := localVersions[0].Version - return &result, nil + result := localVersions[0] + return result, nil } - return nil, nil + return store.Item{}, nil } // PathTo returns the local path to the assets directory for the provided version and platform diff --git a/pkg/envtest/setup/testhelpers/store.go b/pkg/envtest/setup/testhelpers/store.go index e53355b087..b511915602 100644 --- a/pkg/envtest/setup/testhelpers/store.go +++ b/pkg/envtest/setup/testhelpers/store.go @@ -47,17 +47,17 @@ func initializeFakeStore(fs afero.Afero, dir string) { } ginkgo.By("making some fake non-store paths") - gomega.Expect(fs.Mkdir(filepath.Join(dir, "missing-binaries"), 0755)).To(gomega.Succeed()) + gomega.Expect(fs.Mkdir(filepath.Join(dir, "missing", "binaries"), 0755)).To(gomega.Succeed()) - gomega.Expect(fs.Mkdir(filepath.Join(dir, "wrong-version"), 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "kubectl"), nil, 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "etcd"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.Mkdir(filepath.Join(dir, "wrong", "version"), 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "kubectl"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "etcd"), nil, 0755)).To(gomega.Succeed()) - gomega.Expect(fs.Mkdir(filepath.Join(dir, "good-version"), 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "kubectl"), nil, 0755)).To(gomega.Succeed()) - gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "etcd"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.Mkdir(filepath.Join(dir, "a", "good", "version"), 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "kubectl"), nil, 0755)).To(gomega.Succeed()) + gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "etcd"), nil, 0755)).To(gomega.Succeed()) // TODO: put the right files } diff --git a/pkg/envtest/setup/use/use.go b/pkg/envtest/setup/use/use.go index 95e7db3a9f..dea42d02c7 100644 --- a/pkg/envtest/setup/use/use.go +++ b/pkg/envtest/setup/use/use.go @@ -5,12 +5,13 @@ import ( "fmt" "sigs.k8s.io/controller-runtime/pkg/envtest/setup/env" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/store" "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" ) // Result summarizes the output of the Use workflow type Result struct { - Version versions.Concrete + Version versions.Spec Platform versions.Platform MD5 string Path string @@ -29,7 +30,7 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result, if cfg.assetPath != "" { if v, ok := env.TryUseAssetsFromPath(ctx, version, cfg.assetPath); ok { return Result{ - Version: *v, + Version: v, Platform: cfg.platform, Path: cfg.assetPath, }, nil @@ -39,13 +40,12 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result, if !cfg.forceDownload && !version.CheckLatest { if selected, err := env.SelectLocalVersion(ctx, version, cfg.platform); err != nil { return Result{}, err - } else if selected != nil { + } else if selected != (store.Item{}) { return Result{ - Path: env.PathTo(selected, cfg.platform), - Version: *selected, - Platform: cfg.platform, + Path: env.PathTo(&selected.Version, selected.Platform), + Version: versions.Spec{Selector: selected.Version}, + Platform: selected.Platform, }, nil - } } @@ -63,7 +63,7 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result, } return Result{ - Version: *selectedVersion, + Version: versions.Spec{Selector: *selectedVersion}, Platform: selectedPlatform.Platform, Path: env.PathTo(selectedVersion, selectedPlatform.Platform), MD5: selectedPlatform.MD5, diff --git a/pkg/envtest/setup/use/use_test.go b/pkg/envtest/setup/use/use_test.go new file mode 100644 index 0000000000..d976c42646 --- /dev/null +++ b/pkg/envtest/setup/use/use_test.go @@ -0,0 +1,141 @@ +package use_test + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/spf13/afero" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/env" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/remote" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/testhelpers" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/use" + "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" +) + +var ( + testLog logr.Logger + ctx context.Context +) + +func TestEnv(t *testing.T) { + testLog = testhelpers.GetLogger() + ctx = logr.NewContext(context.Background(), testLog) + + RegisterFailHandler(Fail) + RunSpecs(t, "Use Suite") +} + +var _ = Describe("Use", func() { + var ( + envOpts []env.Option + version = versions.Spec{ + Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 0}, + } + ) + JustBeforeEach(func() { + addr, shutdown, err := testhelpers.NewServer() + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(shutdown) + + s := testhelpers.NewMockStore() + + envOpts = append( + envOpts, + env.WithClient(&remote.Client{ + Log: testLog.WithName("test-remote-client"), + Bucket: "kubebuilder-tools-test", + Server: addr, + Insecure: true, + }), + env.WithStore(s), + env.WithFS(afero.NewIOFS(s.Root)), + ) + }) + + Context("when useEnv is set", func() { + It("should fall back to normal behavior when the env is not set", func() { + result, err := use.Use( + ctx, + version, + use.WithAssetsFromEnv(true), + use.WithPlatform("*", "*"), + use.WithEnvOptions(envOpts...), + ) + + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Version).To(Equal(version)) + Expect(result.Path).To(HaveSuffix("/1.16.0-linux-amd64"), "should fall back to a local version") + }) + + It("should fall back to normal behavior if binaries are missing", func() { + result, err := use.Use( + ctx, + version, + use.WithAssetsFromEnv(true), + use.WithAssetsAt(".test-binaries/missing-binaries"), + use.WithPlatform("*", "*"), + use.WithEnvOptions(envOpts...), + ) + + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Version).To(Equal(version), "should fall back to a local version") + Expect(result.Path).To(HaveSuffix("/1.16.0-linux-amd64")) + }) + + It("should use the value of the env if it contains the right binaries", func() { + result, err := use.Use( + ctx, + version, + use.WithAssetsFromEnv(true), + use.WithAssetsAt("a/good/version"), + use.WithPlatform("*", "*"), + use.WithEnvOptions(envOpts...), + ) + + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Version).To(Equal(versions.AnyVersion)) + Expect(result.Path).To(HaveSuffix("/good/version")) + }) + + It("should not try to check the version of the binaries", func() { + result, err := use.Use( + ctx, + version, + use.WithAssetsFromEnv(true), + use.WithAssetsAt("wrong/version"), + use.WithPlatform("*", "*"), + use.WithEnvOptions(envOpts...), + ) + + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Version).To(Equal(versions.AnyVersion)) + Expect(result.Path).To(Equal("wrong/version")) + }) + + It("should not need to contact the network", func() { + envOpts = append(envOpts, env.WithClient(nil)) // ensure tests fail if we try to contact remote + + result, err := use.Use( + ctx, + version, + use.WithAssetsFromEnv(true), + use.WithAssetsAt("a/good/version"), + use.WithPlatform("*", "*"), + use.WithEnvOptions(envOpts...), + ) + + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Version).To(Equal(versions.AnyVersion)) + Expect(result.Path).To(HaveSuffix("/good/version")) + }) + }) +})