diff --git a/pkg/envtest/setup/env/env.go b/pkg/envtest/setup/env/env.go index 147bedee6f..2fd25a807a 100644 --- a/pkg/envtest/setup/env/env.go +++ b/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 } diff --git a/pkg/envtest/setup/remote/client.go b/pkg/envtest/setup/remote/client.go index 5184587023..e94b538d6a 100644 --- a/pkg/envtest/setup/remote/client.go +++ b/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) diff --git a/pkg/envtest/setup/testhelpers/package.go b/pkg/envtest/setup/testhelpers/package.go index 28aea358c1..cfeeda85e3 100644 --- a/pkg/envtest/setup/testhelpers/package.go +++ b/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 } diff --git a/pkg/envtest/setup/testhelpers/remote.go b/pkg/envtest/setup/testhelpers/remote.go index 1447bce7f0..c08540ee14 100644 --- a/pkg/envtest/setup/testhelpers/remote.go +++ b/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) } }) diff --git a/pkg/envtest/setup/use/config.go b/pkg/envtest/setup/use/config.go index 5eaecfd592..f5d78614f9 100644 --- a/pkg/envtest/setup/use/config.go +++ b/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{ diff --git a/pkg/envtest/setup/use/use.go b/pkg/envtest/setup/use/use.go index fe1489f571..263b4518a5 100644 --- a/pkg/envtest/setup/use/use.go +++ b/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, + } +} diff --git a/pkg/envtest/setup/use/use_test.go b/pkg/envtest/setup/use/use_test.go index eff3d0abd6..deda32c445 100644 --- a/pkg/envtest/setup/use/use_test.go +++ b/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}})) + }) + }) + }) })