From cab83922705552b9a111ee0a0393aad10691f249 Mon Sep 17 00:00:00 2001 From: Emily Casey Date: Wed, 15 Jul 2020 18:34:33 -0400 Subject: [PATCH 1/3] Respect dependency mappings in /dependencies/mappings.toml This allows users in an airgapped environment or an environment with restrictive firewall rules to map dependencies to new URIs that are accessible from the build environment Signed-off-by: Emily Casey --- dependency_cache.go | 50 +++- dependency_cache_test.go | 300 ++++++++++++------- dependency_mapping.go | 54 ++++ testdata/platform/dependencies/mappings.toml | 17 ++ 4 files changed, 293 insertions(+), 128 deletions(-) create mode 100644 dependency_mapping.go create mode 100644 testdata/platform/dependencies/mappings.toml diff --git a/dependency_cache.go b/dependency_cache.go index 56d0576..3ae45fe 100644 --- a/dependency_cache.go +++ b/dependency_cache.go @@ -48,16 +48,31 @@ type DependencyCache struct { // UserAgent is the User-Agent string to use with requests. UserAgent string + + // DependencyMap optionally provides override URIs for BuildpackDependencies + Mappings []DependencyMapping } // NewDependencyCache creates a new instance setting the default cache path (/dependencies) and user // agent (/). -func NewDependencyCache(buildpack libcnb.Buildpack) DependencyCache { +// Mappings will be read from /dependencies/mappings.toml +func NewDependencyCache(context libcnb.BuildContext) (DependencyCache, error) { + mappingsFile, err := ReadMappingsFile(DefaultMappingsFilePath(context.Platform.Path)) + if err != nil { + return DependencyCache{}, err + } + var mappings []DependencyMapping + for _, bpm := range mappingsFile.BuildpackMappings { + if bpm.BuildpackID == context.Buildpack.Info.ID { + mappings = bpm.Mappings + } + } return DependencyCache{ - CachePath: filepath.Join(buildpack.Path, "dependencies"), + CachePath: filepath.Join(context.Buildpack.Path, "dependencies"), DownloadPath: os.TempDir(), - UserAgent: fmt.Sprintf("%s/%s", buildpack.Info.ID, buildpack.Info.Version), - } + UserAgent: fmt.Sprintf("%s/%s", context.Buildpack.Info.ID, context.Buildpack.Info.Version), + Mappings: mappings, + }, nil } // RequestModifierFunc is a callback that enables modification of a download request before it is sent. It is often @@ -90,16 +105,23 @@ func (d *DependencyCache) ArtifactWithRequestModification(dependency BuildpackDe actual BuildpackDependency artifact string file string + uri = dependency.URI ) + for _, override := range d.Mappings { + if override.ID == dependency.ID && override.Version == dependency.Version { + uri = override.URI + break + } + } if dependency.SHA256 == "" { d.Logger.Headerf("%s Dependency has no SHA256. Skipping cache.", color.New(color.FgYellow, color.Bold).Sprint("Warning:")) - d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), dependency.URI) - artifact = filepath.Join(d.DownloadPath, filepath.Base(dependency.URI)) - if err := d.download(dependency.URI, artifact, f); err != nil { - return nil, fmt.Errorf("unable to download %s\n%w", dependency.URI, err) + d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), uri) + artifact = filepath.Join(d.DownloadPath, filepath.Base(uri)) + if err := d.download(uri, artifact, f); err != nil { + return nil, fmt.Errorf("unable to download %s\n%w", uri, err) } return os.Open(artifact) @@ -112,7 +134,7 @@ func (d *DependencyCache) ArtifactWithRequestModification(dependency BuildpackDe if reflect.DeepEqual(dependency, actual) { d.Logger.Bodyf("%s cached download from buildpack", color.GreenString("Reusing")) - return os.Open(filepath.Join(d.CachePath, dependency.SHA256, filepath.Base(dependency.URI))) + return os.Open(filepath.Join(d.CachePath, dependency.SHA256, filepath.Base(uri))) } file = filepath.Join(d.DownloadPath, fmt.Sprintf("%s.toml", dependency.SHA256)) @@ -122,13 +144,13 @@ func (d *DependencyCache) ArtifactWithRequestModification(dependency BuildpackDe if reflect.DeepEqual(dependency, actual) { d.Logger.Bodyf("%s previously cached download", color.GreenString("Reusing")) - return os.Open(filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(dependency.URI))) + return os.Open(filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(uri))) } - d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), dependency.URI) - artifact = filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(dependency.URI)) - if err := d.download(dependency.URI, artifact, f); err != nil { - return nil, fmt.Errorf("unable to download %s\n%w", dependency.URI, err) + d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), uri) + artifact = filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(uri)) + if err := d.download(uri, artifact, f); err != nil { + return nil, fmt.Errorf("unable to download %s\n%w", uri, err) } d.Logger.Body("Verifying checksum") diff --git a/dependency_cache_test.go b/dependency_cache_test.go index a813e05..29a7222 100644 --- a/dependency_cache_test.go +++ b/dependency_cache_test.go @@ -26,6 +26,7 @@ import ( "testing" "github.com/BurntSushi/toml" + "github.com/buildpacks/libcnb" . "github.com/onsi/gomega" "github.com/onsi/gomega/ghttp" "github.com/sclevine/spec" @@ -37,153 +38,224 @@ func testDependencyCache(t *testing.T, context spec.G, it spec.S) { var ( Expect = NewWithT(t).Expect - cachePath string - downloadPath string - dependency libpak.BuildpackDependency dependencyCache libpak.DependencyCache - server *ghttp.Server ) - it.Before(func() { - var err error - - cachePath, err = ioutil.TempDir("", "dependency-cache-cache-path") - Expect(err).NotTo(HaveOccurred()) - - downloadPath, err = ioutil.TempDir("", "dependency-cache-download-path") - Expect(err).NotTo(HaveOccurred()) - - RegisterTestingT(t) - server = ghttp.NewServer() - - dependency = libpak.BuildpackDependency{ - ID: "test-id", - Name: "test-name", - Version: "1.1.1", - URI: fmt.Sprintf("%s/test-path", server.URL()), - SHA256: "576dd8416de5619ea001d9662291d62444d1292a38e96956bc4651c01f14bca1", - Stacks: []string{"test-stack"}, - Licenses: []libpak.BuildpackDependencyLicense{ - { - Type: "test-type", - URI: "test-uri", + context("NewDependencyCache", func() { + it.Before(func() { + var err error + dependencyCache, err = libpak.NewDependencyCache(libcnb.BuildContext{ + Buildpack: libcnb.Buildpack{ + Info: libcnb.BuildpackInfo{ + ID: "some-buildpack-id", + Version: "some-buildpack-version", + }, + Path: "some/path", }, - }, - } + Platform: libcnb.Platform{ + Path: filepath.Join("testdata", "platform"), + }, + }) + Expect(err).NotTo(HaveOccurred()) + }) - dependencyCache = libpak.DependencyCache{ - CachePath: cachePath, - DownloadPath: downloadPath, - UserAgent: "test-user-agent", - } + it("defaults CachePath to /dependencies", func() { + Expect(dependencyCache.CachePath).To(Equal(filepath.Join("some/path/dependencies"))) + }) + it("sets the user agent to /", func() { + Expect(dependencyCache.UserAgent).To(Equal("some-buildpack-id/some-buildpack-version")) + }) + it("reads mappings for buildpack from the platform dir", func() { + Expect(dependencyCache.Mappings).To(ConsistOf( + libpak.DependencyMapping{ + ID: "some-dependency-id", + Version: "some-dependency-version", + URI: "some-uri", + }, + libpak.DependencyMapping{ + ID: "other-dependency-id", + Version: "other-dependency-version", + URI: "other-uri", + }, + )) + }) }) - it.After(func() { - Expect(os.RemoveAll(cachePath)).To(Succeed()) - Expect(os.RemoveAll(downloadPath)).To(Succeed()) - server.Close() - }) + context("artifacts", func() { + var ( + cachePath string + downloadPath string + dependency libpak.BuildpackDependency + server *ghttp.Server + ) + + it.Before(func() { + var err error + + cachePath, err = ioutil.TempDir("", "dependency-cache-cache-path") + Expect(err).NotTo(HaveOccurred()) + + downloadPath, err = ioutil.TempDir("", "dependency-cache-download-path") + Expect(err).NotTo(HaveOccurred()) + + RegisterTestingT(t) + server = ghttp.NewServer() + + dependency = libpak.BuildpackDependency{ + ID: "test-id", + Name: "test-name", + Version: "1.1.1", + URI: fmt.Sprintf("%s/test-path", server.URL()), + SHA256: "576dd8416de5619ea001d9662291d62444d1292a38e96956bc4651c01f14bca1", + Stacks: []string{"test-stack"}, + Licenses: []libpak.BuildpackDependencyLicense{ + { + Type: "test-type", + URI: "test-uri", + }, + }, + } - copyFile := func(source string, destination string) { - in, err := os.Open(source) - Expect(err).NotTo(HaveOccurred()) - defer in.Close() + dependencyCache = libpak.DependencyCache{ + CachePath: cachePath, + DownloadPath: downloadPath, + UserAgent: "test-user-agent", + } + }) - Expect(os.MkdirAll(filepath.Dir(destination), 0755)).To(Succeed()) - out, err := os.OpenFile(destination, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) - Expect(err).NotTo(HaveOccurred()) - defer out.Close() + it.After(func() { + Expect(os.RemoveAll(cachePath)).To(Succeed()) + Expect(os.RemoveAll(downloadPath)).To(Succeed()) + server.Close() + }) - _, err = io.Copy(out, in) - Expect(err).NotTo(HaveOccurred()) - } + copyFile := func(source string, destination string) { + in, err := os.Open(source) + Expect(err).NotTo(HaveOccurred()) + defer in.Close() - writeTOML := func(destination string, v interface{}) { - Expect(os.MkdirAll(filepath.Dir(destination), 0755)).To(Succeed()) - out, err := os.OpenFile(destination, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) - Expect(err).NotTo(HaveOccurred()) - defer out.Close() + Expect(os.MkdirAll(filepath.Dir(destination), 0755)).To(Succeed()) + out, err := os.OpenFile(destination, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) + Expect(err).NotTo(HaveOccurred()) + defer out.Close() - Expect(toml.NewEncoder(out).Encode(v)).To(Succeed()) - } + _, err = io.Copy(out, in) + Expect(err).NotTo(HaveOccurred()) + } - it("returns from cache path", func() { - copyFile(filepath.Join("testdata", "test-file"), filepath.Join(cachePath, dependency.SHA256, "test-path")) - writeTOML(filepath.Join(cachePath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) + writeTOML := func(destination string, v interface{}) { + Expect(os.MkdirAll(filepath.Dir(destination), 0755)).To(Succeed()) + out, err := os.OpenFile(destination, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) + Expect(err).NotTo(HaveOccurred()) + defer out.Close() - a, err := dependencyCache.Artifact(dependency) - Expect(err).NotTo(HaveOccurred()) + Expect(toml.NewEncoder(out).Encode(v)).To(Succeed()) + } - Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) - }) + it("returns from cache path", func() { + copyFile(filepath.Join("testdata", "test-file"), filepath.Join(cachePath, dependency.SHA256, "test-path")) + writeTOML(filepath.Join(cachePath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) - it("returns from download path", func() { - copyFile(filepath.Join("testdata", "test-file"), filepath.Join(downloadPath, dependency.SHA256, "test-path")) - writeTOML(filepath.Join(downloadPath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) - a, err := dependencyCache.Artifact(dependency) - Expect(err).NotTo(HaveOccurred()) + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) + }) - Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) - }) + it("returns from download path", func() { + copyFile(filepath.Join("testdata", "test-file"), filepath.Join(downloadPath, dependency.SHA256, "test-path")) + writeTOML(filepath.Join(downloadPath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) - it("downloads", func() { - server.AppendHandlers(ghttp.RespondWith(http.StatusOK, "test-fixture")) + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) - a, err := dependencyCache.Artifact(dependency) - Expect(err).NotTo(HaveOccurred()) + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) + }) - Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) - }) + it("downloads", func() { + server.AppendHandlers(ghttp.CombineHandlers( + ghttp.VerifyRequest(http.MethodGet, "/test-path", ""), + ghttp.RespondWith(http.StatusOK, "test-fixture"), + )) - it("fails with invalid SHA256", func() { - server.AppendHandlers(ghttp.RespondWith(http.StatusOK, "invalid-fixture")) + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) - _, err := dependencyCache.Artifact(dependency) - Expect(err).To(HaveOccurred()) - }) + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) + }) + + context("uri is overridden", func() { + it.Before(func() { + dependencyCache.Mappings = []libpak.DependencyMapping{ + { + ID: dependency.ID, + Version: dependency.Version, + URI: fmt.Sprintf("%s/override-path", server.URL()), + }, + } + }) + + it("downloads from override uri", func() { + server.AppendHandlers(ghttp.CombineHandlers( + ghttp.VerifyRequest(http.MethodGet, "/override-path", ""), + ghttp.RespondWith(http.StatusOK, "test-fixture"), + )) + + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) + + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) + }) + }) - it("skips cache with empty SHA256", func() { - copyFile(filepath.Join("testdata", "test-file"), filepath.Join(cachePath, dependency.SHA256, "test-path")) - writeTOML(filepath.Join(cachePath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) - copyFile(filepath.Join("testdata", "test-file"), filepath.Join(downloadPath, dependency.SHA256, "test-path")) - writeTOML(filepath.Join(downloadPath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) + it("fails with invalid SHA256", func() { + server.AppendHandlers(ghttp.RespondWith(http.StatusOK, "invalid-fixture")) - dependency.SHA256 = "" - server.AppendHandlers(ghttp.RespondWith(http.StatusOK, "alternate-fixture")) + _, err := dependencyCache.Artifact(dependency) + Expect(err).To(HaveOccurred()) + }) - a, err := dependencyCache.Artifact(dependency) - Expect(err).NotTo(HaveOccurred()) + it("skips cache with empty SHA256", func() { + copyFile(filepath.Join("testdata", "test-file"), filepath.Join(cachePath, dependency.SHA256, "test-path")) + writeTOML(filepath.Join(cachePath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) + copyFile(filepath.Join("testdata", "test-file"), filepath.Join(downloadPath, dependency.SHA256, "test-path")) + writeTOML(filepath.Join(downloadPath, fmt.Sprintf("%s.toml", dependency.SHA256)), dependency) - Expect(ioutil.ReadAll(a)).To(Equal([]byte("alternate-fixture"))) - }) + dependency.SHA256 = "" + server.AppendHandlers(ghttp.RespondWith(http.StatusOK, "alternate-fixture")) - it("sets User-Agent", func() { - server.AppendHandlers(ghttp.CombineHandlers( - ghttp.VerifyHeaderKV("User-Agent", "test-user-agent"), - ghttp.RespondWith(http.StatusOK, "test-fixture"), - )) + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) - a, err := dependencyCache.Artifact(dependency) - Expect(err).NotTo(HaveOccurred()) + Expect(ioutil.ReadAll(a)).To(Equal([]byte("alternate-fixture"))) + }) - Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) - }) + it("sets User-Agent", func() { + server.AppendHandlers(ghttp.CombineHandlers( + ghttp.VerifyHeaderKV("User-Agent", "test-user-agent"), + ghttp.RespondWith(http.StatusOK, "test-fixture"), + )) - it("modifies request", func() { - server.AppendHandlers(ghttp.CombineHandlers( - ghttp.VerifyHeaderKV("User-Agent", "test-user-agent"), - ghttp.VerifyHeaderKV("Test-Key", "test-value"), - ghttp.RespondWith(http.StatusOK, "test-fixture"), - )) + a, err := dependencyCache.Artifact(dependency) + Expect(err).NotTo(HaveOccurred()) - a, err := dependencyCache.ArtifactWithRequestModification(dependency, func(request *http.Request) (*http.Request, error) { - request.Header.Add("Test-Key", "test-value") - return request, nil + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) }) - Expect(err).NotTo(HaveOccurred()) - Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) - }) + it("modifies request", func() { + server.AppendHandlers(ghttp.CombineHandlers( + ghttp.VerifyHeaderKV("User-Agent", "test-user-agent"), + ghttp.VerifyHeaderKV("Test-Key", "test-value"), + ghttp.RespondWith(http.StatusOK, "test-fixture"), + )) + + a, err := dependencyCache.ArtifactWithRequestModification(dependency, func(request *http.Request) (*http.Request, error) { + request.Header.Add("Test-Key", "test-value") + return request, nil + }) + Expect(err).NotTo(HaveOccurred()) + Expect(ioutil.ReadAll(a)).To(Equal([]byte("test-fixture"))) + }) + }) } diff --git a/dependency_mapping.go b/dependency_mapping.go new file mode 100644 index 0000000..553f442 --- /dev/null +++ b/dependency_mapping.go @@ -0,0 +1,54 @@ +/* + * Copyright 2018-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package libpak + +import ( + "path/filepath" + + "github.com/BurntSushi/toml" +) + +// MappingsFile defines dependency mappings for a set of buildpacks +type MappingsFile struct { + BuildpackMappings []BuildpackMappings `toml:"buildpacks"` +} + +type BuildpackMappings struct { + BuildpackID string `toml:"id"` + Mappings []DependencyMapping `toml:"mappings"` +} + +// DependencyMapping defines a new URI for a dependency with the given ID and Version +type DependencyMapping struct { + ID string `toml:"id"` + Version string `toml:"version"` + URI string `toml:"uri"` +} + +// ReadMappingsFile read MappingsFile from path +func ReadMappingsFile(path string) (MappingsFile, error) { + mappingsFile := MappingsFile{} + _, err := toml.DecodeFile(path, &mappingsFile) + if err != nil { + return MappingsFile{}, err + } + return mappingsFile, nil +} + +func DefaultMappingsFilePath(platformDir string) string { + return filepath.Join(platformDir, "dependencies", "mappings.toml") +} diff --git a/testdata/platform/dependencies/mappings.toml b/testdata/platform/dependencies/mappings.toml new file mode 100644 index 0000000..3946e00 --- /dev/null +++ b/testdata/platform/dependencies/mappings.toml @@ -0,0 +1,17 @@ +[[buildpacks]] + id = "some-buildpack-id" + [[buildpacks.mappings]] + id = "some-dependency-id" + version = "some-dependency-version" + uri = "some-uri" + [[buildpacks.mappings]] + id = "other-dependency-id" + version = "other-dependency-version" + uri = "other-uri" + +[[buildpacks]] + id = "exclude-buildpack-id" + [[buildpacks.mappings]] + id = "exclude-dependency-id" + version = "exclude-dependency-version" + uri = "exclude-uri" \ No newline at end of file From 884bb818e94bfe95106897cae951c2921a1f937a Mon Sep 17 00:00:00 2001 From: Emily Casey Date: Wed, 15 Jul 2020 18:38:52 -0400 Subject: [PATCH 2/3] format Signed-off-by: Emily Casey --- buildpack_plan_test.go | 6 +++--- carton/package_dependency_test.go | 8 ++++---- crush/crush_test.go | 2 +- effect/mocks/executor.go | 3 ++- sherpa/file_listing.go | 4 ++-- sherpa/nodejs.go | 1 - sherpa/testdata/statik/statik.go | 6 ++---- testdata/platform/dependencies/mappings.toml | 2 +- 8 files changed, 15 insertions(+), 17 deletions(-) diff --git a/buildpack_plan_test.go b/buildpack_plan_test.go index 1d6ecf6..d74d5a1 100644 --- a/buildpack_plan_test.go +++ b/buildpack_plan_test.go @@ -126,10 +126,10 @@ func testBuildpackPlan(t *testing.T, context spec.G, it spec.S) { Name: "test-name-1", }, { - Name: "test-name-2", + Name: "test-name-2", }, { - Name: "test-name-2", + Name: "test-name-2", }, }, } @@ -159,7 +159,7 @@ func testBuildpackPlan(t *testing.T, context spec.G, it spec.S) { Expect(err).NotTo(HaveOccurred()) Expect(ok).To(BeTrue()) Expect(e).To(Equal(libcnb.BuildpackPlanEntry{ - Name: "test-name-2", + Name: "test-name-2", })) }) }) diff --git a/carton/package_dependency_test.go b/carton/package_dependency_test.go index f2f8eba..91fe779 100644 --- a/carton/package_dependency_test.go +++ b/carton/package_dependency_test.go @@ -81,8 +81,8 @@ func testPackageDependency(t *testing.T, context spec.G, it spec.S) { p := carton.PackageDependency{ BuilderPath: path, - ID: "gcr.io/paketo-buildpacks/test-1", - Version: "test-version-3", + ID: "gcr.io/paketo-buildpacks/test-1", + Version: "test-version-3", } p.Update(carton.WithExitHandler(exitHandler)) @@ -101,8 +101,8 @@ func testPackageDependency(t *testing.T, context spec.G, it spec.S) { p := carton.PackageDependency{ PackagePath: path, - ID: "gcr.io/paketo-buildpacks/test-1", - Version: "test-version-3", + ID: "gcr.io/paketo-buildpacks/test-1", + Version: "test-version-3", } p.Update(carton.WithExitHandler(exitHandler)) diff --git a/crush/crush_test.go b/crush/crush_test.go index 4582732..8242d07 100644 --- a/crush/crush_test.go +++ b/crush/crush_test.go @@ -32,7 +32,7 @@ func testCrush(t *testing.T, context spec.G, it spec.S) { var ( Expect = NewWithT(t).Expect - path string + path string ) it.Before(func() { diff --git a/effect/mocks/executor.go b/effect/mocks/executor.go index 28743bd..7df9e80 100644 --- a/effect/mocks/executor.go +++ b/effect/mocks/executor.go @@ -3,8 +3,9 @@ package mocks import ( - effect "github.com/paketo-buildpacks/libpak/effect" mock "github.com/stretchr/testify/mock" + + effect "github.com/paketo-buildpacks/libpak/effect" ) // Executor is an autogenerated mock type for the Executor type diff --git a/sherpa/file_listing.go b/sherpa/file_listing.go index 4807274..e1fec08 100644 --- a/sherpa/file_listing.go +++ b/sherpa/file_listing.go @@ -70,8 +70,8 @@ func NewFileListing(roots ...string) ([]FileEntry, error) { } e := FileEntry{ - Path: path, - Mode: info.Mode().String(), + Path: path, + Mode: info.Mode().String(), } if info.IsDir() { diff --git a/sherpa/nodejs.go b/sherpa/nodejs.go index d77b6a1..527c212 100644 --- a/sherpa/nodejs.go +++ b/sherpa/nodejs.go @@ -44,4 +44,3 @@ func NodeJSMainModule(path string) (string, error) { return m, nil } - diff --git a/sherpa/testdata/statik/statik.go b/sherpa/testdata/statik/statik.go index 20bdc65..01105b5 100644 --- a/sherpa/testdata/statik/statik.go +++ b/sherpa/testdata/statik/statik.go @@ -6,9 +6,7 @@ import ( "github.com/rakyll/statik/fs" ) - func init() { data := "PK\x03\x04\x14\x00\x08\x00\x08\x00\x82\x8a{P\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0d\x00 \x00test-file.txtUT\x05\x00\x01\xc55~^J\xcb\xac()-J\xd5\xcdM,\xcaN-\xd2\xad\xae\xd6+K\xcc)M\xad\xad\xe5\x02\x04\x00\x00\xff\xffPK\x07\x08\x08%\xc5\x99 \x00\x00\x00\x1a\x00\x00\x00PK\x01\x02\x14\x03\x14\x00\x08\x00\x08\x00\x82\x8a{P\x08%\xc5\x99 \x00\x00\x00\x1a\x00\x00\x00\x0d\x00 \x00\x00\x00\x00\x00\x00\x00\x00\x00\xa4\x81\x00\x00\x00\x00test-file.txtUT\x05\x00\x01\xc55~^PK\x05\x06\x00\x00\x00\x00\x01\x00\x01\x00D\x00\x00\x00d\x00\x00\x00\x00\x00" - fs.Register(data) - } - \ No newline at end of file + fs.Register(data) +} diff --git a/testdata/platform/dependencies/mappings.toml b/testdata/platform/dependencies/mappings.toml index 3946e00..8dd6a1f 100644 --- a/testdata/platform/dependencies/mappings.toml +++ b/testdata/platform/dependencies/mappings.toml @@ -14,4 +14,4 @@ [[buildpacks.mappings]] id = "exclude-dependency-id" version = "exclude-dependency-version" - uri = "exclude-uri" \ No newline at end of file + uri = "exclude-uri" From 0f8eb47c0eb673a3d387bbc663a089c1fee3d477 Mon Sep 17 00:00:00 2001 From: Emily Casey Date: Thu, 16 Jul 2020 12:00:04 -0400 Subject: [PATCH 3/3] Style and error handling fixes * Better error messages * Handling missing mappings file * Don't export entire file schema Signed-off-by: Emily Casey --- dependency_cache.go | 26 ++++---- dependency_cache_test.go | 64 +++++++++++--------- dependency_mapping.go | 39 ++++++------ effect/mocks/executor.go | 3 +- testdata/platform/dependencies/mappings.toml | 3 + 5 files changed, 73 insertions(+), 62 deletions(-) diff --git a/dependency_cache.go b/dependency_cache.go index 3ae45fe..ec15ec2 100644 --- a/dependency_cache.go +++ b/dependency_cache.go @@ -49,7 +49,7 @@ type DependencyCache struct { // UserAgent is the User-Agent string to use with requests. UserAgent string - // DependencyMap optionally provides override URIs for BuildpackDependencies + // Mappings optionally provides URIs mapping for BuildpackDependencies Mappings []DependencyMapping } @@ -57,22 +57,20 @@ type DependencyCache struct { // agent (/). // Mappings will be read from /dependencies/mappings.toml func NewDependencyCache(context libcnb.BuildContext) (DependencyCache, error) { - mappingsFile, err := ReadMappingsFile(DefaultMappingsFilePath(context.Platform.Path)) - if err != nil { - return DependencyCache{}, err - } - var mappings []DependencyMapping - for _, bpm := range mappingsFile.BuildpackMappings { - if bpm.BuildpackID == context.Buildpack.Info.ID { - mappings = bpm.Mappings - } - } - return DependencyCache{ + cache := DependencyCache{ CachePath: filepath.Join(context.Buildpack.Path, "dependencies"), DownloadPath: os.TempDir(), UserAgent: fmt.Sprintf("%s/%s", context.Buildpack.Info.ID, context.Buildpack.Info.Version), - Mappings: mappings, - }, nil + } + mappings, err := ReadMappingsForBuildpack( + DefaultMappingsFilePath(context.Platform.Path), + context.Buildpack.Info.ID, + ) + if err != nil { + return DependencyCache{}, fmt.Errorf("unable to read dependency mappings file\n%w", err) + } + cache.Mappings = mappings + return cache, nil } // RequestModifierFunc is a callback that enables modification of a download request before it is sent. It is often diff --git a/dependency_cache_test.go b/dependency_cache_test.go index 29a7222..0057d33 100644 --- a/dependency_cache_test.go +++ b/dependency_cache_test.go @@ -37,14 +37,13 @@ import ( func testDependencyCache(t *testing.T, context spec.G, it spec.S) { var ( Expect = NewWithT(t).Expect - - dependencyCache libpak.DependencyCache ) context("NewDependencyCache", func() { + var ctx libcnb.BuildContext + it.Before(func() { - var err error - dependencyCache, err = libpak.NewDependencyCache(libcnb.BuildContext{ + ctx = libcnb.BuildContext{ Buildpack: libcnb.Buildpack{ Info: libcnb.BuildpackInfo{ ID: "some-buildpack-id", @@ -52,41 +51,48 @@ func testDependencyCache(t *testing.T, context spec.G, it spec.S) { }, Path: "some/path", }, - Platform: libcnb.Platform{ - Path: filepath.Join("testdata", "platform"), - }, - }) - Expect(err).NotTo(HaveOccurred()) + } }) - it("defaults CachePath to /dependencies", func() { + it("set default CachePath and UserAgent", func() { + dependencyCache, err := libpak.NewDependencyCache(ctx) + Expect(err).NotTo(HaveOccurred()) Expect(dependencyCache.CachePath).To(Equal(filepath.Join("some/path/dependencies"))) - }) - it("sets the user agent to /", func() { Expect(dependencyCache.UserAgent).To(Equal("some-buildpack-id/some-buildpack-version")) + Expect(dependencyCache.Mappings).To(BeEmpty()) }) - it("reads mappings for buildpack from the platform dir", func() { - Expect(dependencyCache.Mappings).To(ConsistOf( - libpak.DependencyMapping{ - ID: "some-dependency-id", - Version: "some-dependency-version", - URI: "some-uri", - }, - libpak.DependencyMapping{ - ID: "other-dependency-id", - Version: "other-dependency-version", - URI: "other-uri", - }, - )) + + context("mappings file exists in the platform dir", func() { + it.Before(func() { + ctx.Platform.Path = filepath.Join("testdata", "platform") + }) + + it("reads mappings for buildpack", func() { + dependencyCache, err := libpak.NewDependencyCache(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(dependencyCache.Mappings).To(ConsistOf( + libpak.DependencyMapping{ + ID: "some-dependency-id", + Version: "some-dependency-version", + URI: "some-uri", + }, + libpak.DependencyMapping{ + ID: "other-dependency-id", + Version: "other-dependency-version", + URI: "other-uri", + }, + )) + }) }) }) context("artifacts", func() { var ( - cachePath string - downloadPath string - dependency libpak.BuildpackDependency - server *ghttp.Server + cachePath string + downloadPath string + dependency libpak.BuildpackDependency + dependencyCache libpak.DependencyCache + server *ghttp.Server ) it.Before(func() { diff --git a/dependency_mapping.go b/dependency_mapping.go index 553f442..a963b23 100644 --- a/dependency_mapping.go +++ b/dependency_mapping.go @@ -17,21 +17,13 @@ package libpak import ( + "fmt" + "os" "path/filepath" "github.com/BurntSushi/toml" ) -// MappingsFile defines dependency mappings for a set of buildpacks -type MappingsFile struct { - BuildpackMappings []BuildpackMappings `toml:"buildpacks"` -} - -type BuildpackMappings struct { - BuildpackID string `toml:"id"` - Mappings []DependencyMapping `toml:"mappings"` -} - // DependencyMapping defines a new URI for a dependency with the given ID and Version type DependencyMapping struct { ID string `toml:"id"` @@ -39,16 +31,29 @@ type DependencyMapping struct { URI string `toml:"uri"` } -// ReadMappingsFile read MappingsFile from path -func ReadMappingsFile(path string) (MappingsFile, error) { - mappingsFile := MappingsFile{} - _, err := toml.DecodeFile(path, &mappingsFile) - if err != nil { - return MappingsFile{}, err +// ReadMappingsForBuildpack reads the mappings for the buildpack with ID buildpackID from the file at path +func ReadMappingsForBuildpack(path string, buildpackID string) ([]DependencyMapping, error) { + mappingsFile := struct { + Buildpacks []struct { + ID string `toml:"id"` + Mappings []DependencyMapping `toml:"mappings"` + } `toml:"buildpacks"` + }{} + if _, err := toml.DecodeFile(path, &mappingsFile); err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("unable to decode dependency mappings file%s\n%w", path, err) + } + for _, bps := range mappingsFile.Buildpacks { + if bps.ID == buildpackID { + return bps.Mappings, nil + } } - return mappingsFile, nil + return nil, nil } +// DefaultMappingsFilePath returns default path for mappings file func DefaultMappingsFilePath(platformDir string) string { return filepath.Join(platformDir, "dependencies", "mappings.toml") } diff --git a/effect/mocks/executor.go b/effect/mocks/executor.go index 7df9e80..28743bd 100644 --- a/effect/mocks/executor.go +++ b/effect/mocks/executor.go @@ -3,9 +3,8 @@ package mocks import ( - mock "github.com/stretchr/testify/mock" - effect "github.com/paketo-buildpacks/libpak/effect" + mock "github.com/stretchr/testify/mock" ) // Executor is an autogenerated mock type for the Executor type diff --git a/testdata/platform/dependencies/mappings.toml b/testdata/platform/dependencies/mappings.toml index 8dd6a1f..b3cb7e1 100644 --- a/testdata/platform/dependencies/mappings.toml +++ b/testdata/platform/dependencies/mappings.toml @@ -1,9 +1,11 @@ [[buildpacks]] id = "some-buildpack-id" + [[buildpacks.mappings]] id = "some-dependency-id" version = "some-dependency-version" uri = "some-uri" + [[buildpacks.mappings]] id = "other-dependency-id" version = "other-dependency-version" @@ -11,6 +13,7 @@ [[buildpacks]] id = "exclude-buildpack-id" + [[buildpacks.mappings]] id = "exclude-dependency-id" version = "exclude-dependency-version"