Finish up test suite port for Use

Author: tomasaschan

pkg/envtest/setup/env/env.go

@@ -46,7 +46,14 @@ func WithFS(fs fs.FS) Option {
 // 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{}
+	env := &Env{
+		// this is the minimal configuration that won't panic
+		Client: &remote.Client{
+			Bucket: remote.DefaultBucket,
+			Server: remote.DefaultServer,
+			Log:    logr.Discard(),
+		},
+	}
 
 	for _, option := range options {
 		option(env)
@@ -60,14 +67,5 @@ func New(options ...Option) (*Env, error) {
 		env.Store = store.NewAt(dir)
 	}
 
-	if env.Client == nil {
-		// this is the minimal configuration that won't panic
-		env.Client = &remote.Client{
-			Bucket: remote.DefaultBucket,
-			Server: remote.DefaultServer,
-			Log:    logr.Discard(),
-		}
-	}
-
 	return env, nil
 }

pkg/envtest/setup/remote/client.go

@@ -23,6 +23,10 @@ import (
 const DefaultBucket = "kubebuilder-tools"
 const DefaultServer = "storage.googleapis.com"
 
+// ErrChecksumMismatch is returned when checksum verification is enabled,
+// but the checksum does not match
+var ErrChecksumMismatch = errors.New("checksum mismatch")
+
 // objectList is the parts we need of the GCS "list-objects-in-bucket" endpoint.
 type objectList struct {
 	Items         []bucketObject `json:"items"`
@@ -180,7 +184,7 @@ func (c *Client) GetVersion(ctx context.Context, version versions.Concrete, plat
 		sum := base64.StdEncoding.EncodeToString(checksum.Sum(nil))
 
 		if sum != platform.MD5 {
-			return fmt.Errorf("checksum mismatch for %s: %s (computed) != %s (reported from GCS)", itemName, sum, platform.MD5)
+			return fmt.Errorf("%w for %s: %s (computed) != %s (reported from GCS)", ErrChecksumMismatch, itemName, sum, platform.MD5)
 		}
 	} else if _, err := io.Copy(out, resp.Body); err != nil {
 		return fmt.Errorf("unable to download %s: %w", itemName, err)

pkg/envtest/setup/testhelpers/package.go

@@ -41,12 +41,12 @@ func ContentsFor(filename string) ([]byte, error) { //nolint:revive
 	return out.Bytes(), nil
 }
 
-func verWith(name string, contents []byte) item {
-	res := item{
-		meta:     bucketObject{Name: name},
-		contents: contents,
+func verWith(name string, contents []byte) Item {
+	res := Item{
+		Meta:     BucketObject{Name: name},
+		Contents: contents,
 	}
-	hash := md5.Sum(res.contents) //nolint:gosec
-	res.meta.Hash = base64.StdEncoding.EncodeToString(hash[:])
+	hash := md5.Sum(res.Contents) //nolint:gosec
+	res.Meta.Hash = base64.StdEncoding.EncodeToString(hash[:])
 	return res
 }

pkg/envtest/setup/testhelpers/remote.go

@@ -10,18 +10,18 @@ import (
 
 // objectList is the parts we need of the GCS "list-objects-in-bucket" endpoint.
 type objectList struct {
-	Items []bucketObject `json:"items"`
+	Items []BucketObject `json:"items"`
 }
 
-// bucketObject is the parts we need of the GCS object metadata.
-type bucketObject struct {
+// BucketObject is the parts we need of the GCS object metadata.
+type BucketObject struct {
 	Name string `json:"name"`
 	Hash string `json:"md5Hash"`
 }
 
-type item struct {
-	meta     bucketObject
-	contents []byte
+type Item struct {
+	Meta     BucketObject
+	Contents []byte
 }
 
 var (
@@ -65,13 +65,13 @@ var (
 		"kubebuilder-tools-v1.19.2-linux-ppc64le.tar.gz",
 	}
 
-	contents map[string]item
+	contents map[string]Item
 )
 
-func makeContents(names []string) ([]item, error) {
-	res := make([]item, len(names))
+func makeContents(names []string) ([]Item, error) {
+	res := make([]Item, len(names))
 	if contents == nil {
-		contents = make(map[string]item, len(RemoteNames))
+		contents = make(map[string]Item, len(RemoteNames))
 	}
 
 	var errs error
@@ -103,26 +103,29 @@ func makeContents(names []string) ([]item, error) {
 // The package names should be a subset of RemoteNames.
 //
 // The returned shutdown function should be called at the end of the test
-func NewServer() (addr string, shutdown func(), err error) {
-	versions, err := makeContents(RemoteNames)
-	if err != nil {
-		return
+func NewServer(items ...Item) (addr string, shutdown func(), err error) {
+	if items == nil {
+		versions, err := makeContents(RemoteNames)
+		if err != nil {
+			return "", nil, err
+		}
+		items = versions
 	}
 
 	server := ghttp.NewServer()
 
-	list := objectList{Items: make([]bucketObject, len(versions))}
-	for i, ver := range versions {
+	list := objectList{Items: make([]BucketObject, len(items))}
+	for i, ver := range items {
 		ver := ver // copy to avoid capturing the iteration variable
-		list.Items[i] = ver.meta
-		server.RouteToHandler("GET", "/storage/v1/b/kubebuilder-tools-test/o/"+ver.meta.Name, func(resp http.ResponseWriter, req *http.Request) {
+		list.Items[i] = ver.Meta
+		server.RouteToHandler("GET", "/storage/v1/b/kubebuilder-tools-test/o/"+ver.Meta.Name, func(resp http.ResponseWriter, req *http.Request) {
 			if req.URL.Query().Get("alt") == "media" {
 				resp.WriteHeader(http.StatusOK)
-				gomega.Expect(resp.Write(ver.contents)).To(gomega.Equal(len(ver.contents)))
+				gomega.Expect(resp.Write(ver.Contents)).To(gomega.Equal(len(ver.Contents)))
 			} else {
 				ghttp.RespondWithJSONEncoded(
 					http.StatusOK,
-					ver.meta,
+					ver.Meta,
 				)(resp, req)
 			}
 		})

pkg/envtest/setup/use/config.go

@@ -53,7 +53,7 @@ func WithEnvOptions(opts ...env.Option) Option {
 }
 
 // VerifySum turns on md5 verification of the downloaded package
-func VerifySum(verify bool) Option { return func(c *config) { c.verifySum = !verify } }
+func VerifySum(verify bool) Option { return func(c *config) { c.verifySum = verify } }
 
 func configure(options ...Option) *config {
 	cfg := &config{

pkg/envtest/setup/use/use.go

@@ -48,21 +48,21 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
 		return Result{}, err
 	}
 
-	if !cfg.forceDownload && !version.CheckLatest || cfg.noDownload {
+	if cfg.noDownload {
 		if selectedLocal != (store.Item{}) {
-			return Result{
-				Path:     env.PathTo(&selectedLocal.Version, selectedLocal.Platform),
-				Version:  versions.Spec{Selector: selectedLocal.Version},
-				Platform: selectedLocal.Platform,
-			}, nil
+			return toResult(env, selectedLocal, ""), nil
 		}
 
 		return Result{}, fmt.Errorf("%w: no local version matching %s found, but you specified NoDownload()", ErrNoMatchingVersion, version)
 	}
 
+	if !cfg.forceDownload && !version.CheckLatest && selectedLocal != (store.Item{}) {
+		return toResult(env, selectedLocal, ""), nil
+	}
+
 	selectedVersion, selectedPlatform, err := env.SelectRemoteVersion(ctx, version, cfg.platform)
 	if err != nil {
-		return Result{}, err
+		return Result{}, fmt.Errorf("%w: %w", ErrNoMatchingVersion, err)
 	}
 
 	if selectedLocal != (store.Item{}) && !selectedVersion.NewerThan(selectedLocal.Version) {
@@ -84,3 +84,12 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
 		MD5:      selectedPlatform.MD5,
 	}, nil
 }
+
+func toResult(env *env.Env, item store.Item, md5 string) Result {
+	return Result{
+		Version:  versions.Spec{Selector: item.Version},
+		Platform: item.Platform,
+		Path:     env.PathTo(&item.Version, item.Platform),
+		MD5:      md5,
+	}
+}

pkg/envtest/setup/use/use_test.go

@@ -200,11 +200,145 @@ var _ = Describe("Use", func() {
 						Patch: versions.AnyPoint,
 					},
 				},
-				use.WithPlatform("*", "*"),
+				use.WithPlatform("linux", "amd64"),
 				use.WithEnvOptions(defaultEnvOpts...),
 			)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 14, Patch: 26}}))
 		})
 	})
+
+	It("should check for a local match first", func() {
+		result, err := use.Use(
+			ctx,
+			versions.Spec{
+				Selector: versions.TildeSelector{
+					Concrete: versions.Concrete{Major: 1, Minor: 16, Patch: 0},
+				},
+			},
+			use.WithPlatform("linux", "amd64"),
+			use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(nil))...),
+		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 1}}))
+	})
+
+	It("should fall back to the network if no local matches are found", func() {
+		result, err := use.Use(
+			ctx,
+			versions.Spec{
+				Selector: versions.TildeSelector{
+					Concrete: versions.Concrete{Major: 1, Minor: 19, Patch: 0},
+				},
+			},
+			use.WithPlatform("linux", "amd64"),
+			use.WithEnvOptions(defaultEnvOpts...),
+		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 19, Patch: 2}}))
+	})
+
+	It("should error out if no matches can be found anywhere", func() {
+		_, err := use.Use(
+			ctx,
+			versions.Spec{
+				Selector: versions.Concrete{Major: 1, Minor: 13, Patch: 0},
+			},
+			use.WithPlatform("*", "*"),
+			use.WithEnvOptions(defaultEnvOpts...),
+		)
+
+		Expect(err).To(MatchError(use.ErrNoMatchingVersion))
+	})
+
+	It("should skip local version matches with non-matching platform", func() {
+		_, err := use.Use(
+			ctx,
+			versions.Spec{
+				Selector: versions.Concrete{Minor: 1, Major: 16, Patch: 2},
+			},
+			use.WithPlatform("linux", "amd64"),
+			use.NoDownload(true),
+			use.WithEnvOptions(defaultEnvOpts...),
+		)
+
+		Expect(err).To(MatchError(use.ErrNoMatchingVersion))
+	})
+
+	It("should skip remote version matches with non-matching platform", func() {
+		_, err := use.Use(
+			ctx,
+			versions.Spec{
+				Selector: versions.Concrete{Minor: 1, Major: 11, Patch: 1},
+			},
+			use.WithPlatform("linux", "amd64"),
+			use.NoDownload(true),
+			use.WithEnvOptions(defaultEnvOpts...),
+		)
+
+		Expect(err).To(MatchError(use.ErrNoMatchingVersion))
+	})
+
+	Context("with an invalid checksum", func() {
+		var client *remote.Client
+		BeforeEach(func() {
+			name := "kubebuilder-tools-86.75.309-linux-amd64.tar.gz"
+			contents, err := testhelpers.ContentsFor(name)
+			Expect(err).NotTo(HaveOccurred())
+
+			server, stop, err := testhelpers.NewServer(testhelpers.Item{
+				Meta: testhelpers.BucketObject{
+					Name: name,
+					Hash: "not the right one!",
+				},
+				Contents: contents,
+			})
+			Expect(err).NotTo(HaveOccurred())
+			DeferCleanup(stop)
+
+			client = &remote.Client{
+				Bucket:   "kubebuilder-tools-test",
+				Server:   server,
+				Insecure: true,
+				Log:      testLog.WithName("test-remote-client"),
+			}
+		})
+
+		When("validating the checksum", func() {
+			It("should fail with an appropriate error", func() {
+				_, err := use.Use(
+					ctx,
+					versions.Spec{
+						Selector: versions.Concrete{
+							Major: 86,
+							Minor: 75,
+							Patch: 309,
+						},
+					},
+					use.VerifySum(true),
+					use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(client))...),
+				)
+
+				Expect(err).To(MatchError(remote.ErrChecksumMismatch))
+			})
+		})
+
+		When("not validating checksum", func() {
+			It("should return the version without error", func() {
+				result, err := use.Use(
+					ctx,
+					versions.Spec{
+						Selector: versions.Concrete{
+							Major: 86,
+							Minor: 75,
+							Patch: 309,
+						},
+					},
+					use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(client))...),
+				)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 86, Minor: 75, Patch: 309}}))
+			})
+		})
+	})
 })