Re-implement the setup-envtest functionality in a package

There are two main points of re-implementing vs just moving the code:

1. Error handling in the old code base was based on panicking and recovering,
   where the recover basically just read out a field from the panic value and
   determined the correct exit code.

   In a package, we want more nuanced error handling, especially in order to
   allow test suites to catch the errors and surface them through their own
   reporting mechanisms.

2. There was a lot of global state in the old code base, "hidden" in the
   env.Env type that was used as a receiver for all the methods.

   This re-implementation tries to make the state more explicit, keeping only
   dependencies (like the remote client and local store) in the environment,
   while retaining the same behavior as the previous implementation.

Tests have been ported over to their respective workflow sub-packages, and
a few new ones have been added to cover cases the old test suite for one reason
or another did not. Thus, we can be fairly confident that the new implementation
does not break old functionality, even if it is a significant rewrite.

Author: tomasaschan

examples/scratch-env/go.mod

@@ -48,7 +48,7 @@ require (
 	golang.org/x/sys v0.21.0 // indirect
 	golang.org/x/term v0.21.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
-	golang.org/x/time v0.3.0 // indirect
+	golang.org/x/time v0.5.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

examples/scratch-env/go.sum

@@ -141,8 +141,8 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
 golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=
-golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
-golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
+golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
+golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=

go.mod

@@ -30,6 +30,8 @@ require (
 	sigs.k8s.io/yaml v1.4.0
 )
 
+require github.com/spf13/afero v1.11.0
+
 require (
 	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
 	github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
@@ -83,7 +85,7 @@ require (
 	golang.org/x/sync v0.7.0 // indirect
 	golang.org/x/term v0.21.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
-	golang.org/x/time v0.3.0 // indirect
+	golang.org/x/time v0.5.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect

go.sum

@@ -112,6 +112,8 @@ github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoG
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
 github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
+github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8=
+github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY=
 github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
 github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
@@ -187,8 +189,8 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
 golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=
-golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
-golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
+golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
+golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=

go.work

@@ -0,0 +1,7 @@
+go 1.22.4
+
+use (
+	.
+	./examples/scratch-env
+	./tools/setup-envtest
+)

go.work.sum

@@ -0,0 +1,41 @@
+package cleanup
+
+import (
+	"context"
+	"errors"
+
+	"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 is a list of version-platform pairs that were removed from the store.
+type Result []store.Item
+
+// Cleanup removes binary packages from disk for all version-platform pairs that match the parameters
+//
+// Note that both the item collection and the error might be non-nil, if some packages were successfully
+// removed (they will be listed in the first return value) and some failed (the errors will be collected
+// in the second).
+func Cleanup(ctx context.Context, spec versions.Spec, options ...Option) (Result, error) {
+	cfg := configure(options...)
+
+	env, err := env.New(cfg.envOpts...)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := env.Store.Initialize(ctx); err != nil {
+		return nil, err
+	}
+
+	items, err := env.Store.Remove(ctx, store.Filter{Version: spec, Platform: cfg.platform})
+	if errors.Is(err, store.ErrUnableToList) {
+		return nil, err
+	}
+
+	// store.Remove returns an error if _any_ item failed to be removed,
+	// but it also reports any items that were removed without errors.
+	// Therefore, both items and err might be non-nil at the same time.
+	return items, err
+}

pkg/envtest/setup/cleanup/cleanup.go

@@ -0,0 +1,97 @@
+package cleanup_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/cleanup"
+	"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/testhelpers"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions"
+)
+
+var (
+	testLog logr.Logger
+	ctx     context.Context
+)
+
+func TestCleanup(t *testing.T) {
+	testLog = testhelpers.GetLogger()
+	ctx = logr.NewContext(context.Background(), testLog)
+
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Cleanup Suite")
+}
+
+var _ = Describe("Cleanup", func() {
+	var (
+		defaultEnvOpts []env.Option
+		s              *store.Store
+	)
+
+	BeforeEach(func() {
+		s = testhelpers.NewMockStore()
+	})
+
+	JustBeforeEach(func() {
+		defaultEnvOpts = []env.Option{
+			env.WithClient(nil), // ensures we fail if we try to connect
+			env.WithStore(s),
+			env.WithFS(afero.NewIOFS(s.Root)),
+		}
+	})
+
+	Context("when cleanup is run", func() {
+		version := versions.Spec{
+			Selector: versions.Concrete{
+				Major: 1,
+				Minor: 16,
+				Patch: 1,
+			},
+		}
+
+		var (
+			matching, nonMatching []store.Item
+		)
+
+		BeforeEach(func() {
+			// ensure there are some versions matching what we're about to delete
+			var err error
+			matching, err = s.List(ctx, store.Filter{Version: version, Platform: versions.Platform{OS: "linux", Arch: "amd64"}})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(matching).NotTo(BeEmpty(), "found no matching versions before cleanup")
+
+			// ensure there are some versions _not_ matching what we're about to delete
+			nonMatching, err = s.List(ctx, store.Filter{Version: versions.Spec{Selector: versions.PatchSelector{Major: 1, Minor: 17, Patch: versions.AnyPoint}}, Platform: versions.Platform{OS: "linux", Arch: "amd64"}})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(nonMatching).NotTo(BeEmpty(), "found no non-matching versions before cleanup")
+		})
+
+		JustBeforeEach(func() {
+			Expect(cleanup.Cleanup(
+				ctx,
+				version,
+				cleanup.WithPlatform("linux", "amd64"),
+				cleanup.WithEnvOptions(defaultEnvOpts...),
+			)).To(Succeed())
+		})
+
+		It("should remove matching versions", func() {
+			items, err := s.List(ctx, store.Filter{Version: version, Platform: versions.Platform{OS: "linux", Arch: "amd64"}})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(items).To(BeEmpty(), "found matching versions after cleanup")
+		})
+
+		It("should not remove non-matching versions", func() {
+			items, err := s.List(ctx, store.Filter{Version: versions.AnyVersion, Platform: versions.Platform{OS: "*", Arch: "*"}})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(items).To(ContainElements(nonMatching), "non-matching items were affected")
+		})
+	})
+})

pkg/envtest/setup/cleanup/cleanup_test.go

@@ -0,0 +1,45 @@
+package cleanup
+
+import (
+	"runtime"
+
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/env"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions"
+)
+
+type config struct {
+	envOpts  []env.Option
+	platform versions.Platform
+}
+
+// Option is a functional option for configuring the cleanup process.
+type Option func(*config)
+
+// WithEnvOptions adds options to the environment setup.
+func WithEnvOptions(opts ...env.Option) Option {
+	return func(cfg *config) {
+		cfg.envOpts = append(cfg.envOpts, opts...)
+	}
+}
+
+// WithPlatform sets the platform to use for cleanup.
+func WithPlatform(os string, arch string) Option {
+	return func(cfg *config) {
+		cfg.platform = versions.Platform{OS: os, Arch: arch}
+	}
+}
+
+func configure(options ...Option) *config {
+	cfg := &config{
+		platform: versions.Platform{
+			Arch: runtime.GOARCH,
+			OS:   runtime.GOOS,
+		},
+	}
+
+	for _, opt := range options {
+		opt(cfg)
+	}
+
+	return cfg
+}

pkg/envtest/setup/cleanup/config.go

@@ -0,0 +1,69 @@
+package env
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io/fs"
+	"path/filepath"
+
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+var expectedExecutables = []string{
+	"kube-apiserver",
+	"etcd",
+	"kubectl",
+}
+
+// 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.Spec, bool) {
+	v, err := versions.FromPath(path)
+	if err != nil {
+		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 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 versions.Spec{}, false
+	} else if !ok {
+		log.FromContext(ctx).Error(nil, "Assets path is missing some executables, ignoring", "path", path)
+		return versions.Spec{}, false
+	}
+
+	return versions.Spec{Selector: v}, true
+}
+
+func (e *Env) hasAllExecutables(path string) (bool, error) {
+	for _, expected := range expectedExecutables {
+		_, err := e.FS.Open(filepath.Join(path, expected))
+		if err != nil {
+			if errors.Is(err, fs.ErrNotExist) {
+				return false, nil
+			}
+			return false, fmt.Errorf("check for existence of %s binary in %s: %w", expected, path, err)
+		}
+	}
+
+	return true, nil
+}

pkg/envtest/setup/env/assets.go

@@ -0,0 +1,72 @@
+package env
+
+import (
+	"io/fs"
+
+	"github.com/go-logr/logr"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/remote"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/setup/store"
+)
+
+// KubebuilderAssetsEnvVar is the environment variable that can be used to override the default local storage location
+const KubebuilderAssetsEnvVar = "KUBEBUILDER_ASSETS"
+
+// Env encapsulates the environment dependencies.
+type Env struct {
+	*store.Store
+	remote.Client
+	fs.FS
+}
+
+// Option is a functional option for configuring an environment
+type Option func(*Env)
+
+// WithStoreAt sets the path to the store directory.
+func WithStoreAt(dir string) Option {
+	return func(c *Env) { c.Store = store.NewAt(dir) }
+}
+
+// WithStore allows injecting a envured store.
+func WithStore(store *store.Store) Option {
+	return func(c *Env) { c.Store = store }
+}
+
+// WithClient allows injecting a envured remote client.
+func WithClient(client remote.Client) Option { return func(c *Env) { c.Client = client } }
+
+// WithFS allows injecting a configured fs.FS, e.g. for mocking.
+// TODO: fix this so it's actually used!
+func WithFS(fs fs.FS) Option {
+	return func(c *Env) {
+		c.FS = fs
+	}
+}
+
+// New returns a new environment, configured with the provided options.
+//
+// If no options are provided, it will be created with a production store.Store and remote.Client
+// and an OS file system.
+func New(options ...Option) (*Env, error) {
+	env := &Env{
+		// this is the minimal configuration that won't panic
+		Client: &remote.GCSClient{ //nolint:staticcheck
+			Bucket: remote.DefaultBucket, //nolint:staticcheck
+			Server: remote.DefaultServer, //nolint:staticcheck
+			Log:    logr.Discard(),
+		},
+	}
+
+	for _, option := range options {
+		option(env)
+	}
+
+	if env.Store == nil {
+		dir, err := store.DefaultStoreDir()
+		if err != nil {
+			return nil, err
+		}
+		env.Store = store.NewAt(dir)
+	}
+
+	return env, nil
+}