Merge pull request buildpacks#75 from buildpack/bp-id

Escape '/' chars in buildpack ID when creating builder
sabhiram · Dec 3, 2018 · 0397c8a · 0397c8a
2 parents 6e98b40 + 0319f42
commit 0397c8a
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 34 deletions.
diff --git a/build.go b/build.go
@@ -273,18 +273,16 @@ func (b *BuildConfig) copyBuildpacksToContainer(ctx context.Context, ctrID strin
 				return nil, fmt.Errorf("directory buildpacks are not implemented on windows")
 			}
 			var buildpackTOML struct {
-				Buildpack struct {
-					ID      string `toml:"id"`
-					Version string `toml:"version"`
-				} `toml:"buildpack"`
+				Buildpack Buildpack
 			}
+
 			_, err = toml.DecodeFile(filepath.Join(bp, "buildpack.toml"), &buildpackTOML)
 			if err != nil {
 				return nil, fmt.Errorf(`failed to decode buildpack.toml from "%s": %s`, bp, err)
 			}
 			id = buildpackTOML.Buildpack.ID
 			version = buildpackTOML.Buildpack.Version
-			bpDir := filepath.Join(buildpacksDir, id, version)
+			bpDir := filepath.Join(buildpacksDir, buildpackTOML.Buildpack.escapedID(), version)
 			ftr, errChan := b.FS.CreateTarReader(bp, bpDir, 0, 0)
 			if err := b.Cli.CopyToContainer(ctx, ctrID, "/", ftr, dockertypes.CopyToContainerOptions{}); err != nil {
 				return nil, errors.Wrapf(err, "copying buildpack '%s' to container", bp)
@@ -701,9 +699,9 @@ func (b *BuildConfig) Export(group *lifecycle.BuildpackGroup) error {
 					}
 					metadata.Buildpacks[index].Layers[layerName] = layer
 				} else {
-					b.Log.Printf("adding layer '%s/%s' with diffID '%s'\n", bp.ID, layerName, layer.SHA)
+					b.Log.Printf("adding %s layer '%s' with diffID '%s'\n", bp.ID, layerName, layer.SHA)
 					if err := img.AddLayer(filepath.Join(tmpDir, "pack-exporter", strings.TrimPrefix(layer.SHA, "sha256:")+".tar")); err != nil {
-						return errors.Wrapf(err, "add layer '%s/%s'", bp.ID, layerName)
+						return errors.Wrapf(err, "add %s layer '%s'", bp.ID, layerName)
 					}
 				}
 			}

diff --git a/buildpack.go b/buildpack.go
@@ -0,0 +1,17 @@
+package pack
+
+import (
+	"strings"
+)
+
+type Buildpack struct {
+	ID      string `toml:"id"`
+	URI     string `toml:"uri"`
+	Latest  bool   `toml:"latest"`
+	Dir     string
+	Version string
+}
+
+func (b *Buildpack) escapedID() string {
+	return strings.Replace(b.ID, "/", "_", -1)
+}
diff --git a/create_builder.go b/create_builder.go
@@ -21,12 +21,8 @@ import (
 )
 
 type BuilderTOML struct {
-	Buildpacks []struct {
-		ID     string `toml:"id"`
-		URI    string `toml:"uri"`
-		Latest bool   `toml:"latest"`
-	} `toml:"buildpacks"`
-	Groups []lifecycle.BuildpackGroup `toml:"groups"`
+	Buildpacks []Buildpack                `toml:"buildpacks"`
+	Groups     []lifecycle.BuildpackGroup `toml:"groups"`
 }
 
 type BuilderConfig struct {
@@ -35,11 +31,6 @@ type BuilderConfig struct {
 	Repo       image.Image
 	BuilderDir string //original location of builder.toml, used for interpreting relative paths in buildpack URIs
 }
-type Buildpack struct {
-	ID     string
-	Dir    string
-	Latest bool
-}
 
 type BuilderFactory struct {
 	Log          *log.Logger
@@ -91,11 +82,7 @@ func (f *BuilderFactory) BuilderConfigFromFlags(flags CreateBuilderFlags) (Build
 	return builderConfig, nil
 }
 
-func (f *BuilderFactory) resolveBuildpackURI(builderDir string, b struct {
-	ID     string `toml:"id"`
-	URI    string `toml:"uri"`
-	Latest bool   `toml:"latest"`
-}) (Buildpack, error) {
+func (f *BuilderFactory) resolveBuildpackURI(builderDir string, b Buildpack) (Buildpack, error) {
 
 	var dir string
 
@@ -119,7 +106,7 @@ func (f *BuilderFactory) resolveBuildpackURI(builderDir string, b struct {
 				return Buildpack{}, errors.Wrapf(err, "could not open file to untar: %q", path)
 			}
 			defer file.Close()
-			tmpDir, err := ioutil.TempDir("", fmt.Sprintf("create-builder-%s-", b.ID))
+			tmpDir, err := ioutil.TempDir("", fmt.Sprintf("create-builder-%s-", b.escapedID()))
 			if err != nil {
 				return Buildpack{}, fmt.Errorf(`failed to create temporary directory: %s`, err)
 			}
@@ -283,8 +270,8 @@ func (f *BuilderFactory) buildpackLayer(dest string, buildpack Buildpack, builde
 		return "", fmt.Errorf("buildpack.toml must provide version: %s", filepath.Join(buildpack.Dir, "!/buildpack.toml"))
 	}
 
-	tarFile := filepath.Join(dest, fmt.Sprintf("%s.%s.tar", buildpack.ID, bp.Version))
-	if err := f.FS.CreateTarFile(tarFile, dir, filepath.Join("/buildpacks", buildpack.ID, bp.Version), 0, 0); err != nil {
+	tarFile := filepath.Join(dest, fmt.Sprintf("%s.%s.tar", buildpack.escapedID(), bp.Version))
+	if err := f.FS.CreateTarFile(tarFile, dir, filepath.Join("/buildpacks", buildpack.escapedID(), bp.Version), 0, 0); err != nil {
 		return "", err
 	}
 	return tarFile, err
@@ -310,11 +297,11 @@ func (f *BuilderFactory) latestLayer(buildpacks []Buildpack, dest, builderDir st
 			if err != nil {
 				return "", err
 			}
-			err = os.Mkdir(filepath.Join(tmpDir, bp.ID), 0755)
+			err = os.Mkdir(filepath.Join(tmpDir, bp.escapedID()), 0755)
 			if err != nil {
 				return "", err
 			}
-			err = os.Symlink(filepath.Join("/", "buildpacks", bp.ID, data.BP.Version), filepath.Join(tmpDir, bp.ID, "latest"))
+			err = os.Symlink(filepath.Join("/", "buildpacks", bp.escapedID(), data.BP.Version), filepath.Join(tmpDir, bp.escapedID(), "latest"))
 			if err != nil {
 				fmt.Println("E")
 			}

diff --git a/create_builder_test.go b/create_builder_test.go
@@ -439,7 +439,7 @@ func checkGroups(t *testing.T, groups []lifecycle.BuildpackGroup) {
 				Version: "1.2.3",
 			},
 			{
-				ID:      "some.bp2",
+				ID:      "some/bp2",
 				Version: "1.2.4",
 			},
 		}},
@@ -462,12 +462,12 @@ func checkBuildpacks(t *testing.T, buildpacks []pack.Buildpack) {
 			// Latest will default to false
 		},
 		{
-			ID:     "some.bp2",
+			ID:     "some/bp2",
 			Dir:    filepath.Join("testdata", "some-path-2"),
 			Latest: false,
 		},
 		{
-			ID:     "some.bp2",
+			ID:     "some/bp2",
 			Dir:    filepath.Join("testdata", "some-latest-path-2"),
 			Latest: true,
 		},

diff --git a/testdata/builder.toml b/testdata/builder.toml
@@ -3,18 +3,18 @@ id = "some.bp1"
 uri = "some-path-1"
 
 [[buildpacks]]
-id = "some.bp2"
+id = "some/bp2"
 uri = "some-path-2"
 
 [[buildpacks]]
-id = "some.bp2"
+id = "some/bp2"
 uri = "some-latest-path-2"
 latest = true
 
 [[groups]]
 buildpacks = [
   { id = "some.bp1", version = "1.2.3" },
-  { id = "some.bp2", version = "1.2.4" },
+  { id = "some/bp2", version = "1.2.4" },
 ]
 
 [[groups]]