Add #RemoveLabel and ensure #Labels returns a copy

The remote/local #Labels is now returning a copy so the behavior is the same. Added #RemoveLabel to mutate the map/remote config so consumers can remove labels. Signed-off-by: Jesse Brown <[email protected]>
buildpacks · Aug 31, 2020 · 4882147 · 4882147
1 parent 108a5c5
commit 4882147
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 2 deletions.
diff --git a/fakes/image.go b/fakes/image.go
@@ -68,7 +68,11 @@ func (i *Image) Label(key string) (string, error) {
 }
 
 func (i *Image) Labels() (map[string]string, error) {
-	return i.labels, nil
+	copiedLabels := make(map[string]string)
+	for i, l := range i.labels {
+		copiedLabels[i] = l
+	}
+	return copiedLabels, nil
 }
 
 func (i *Image) OS() (string, error) {
@@ -108,6 +112,11 @@ func (i *Image) SetLabel(k string, v string) error {
 	return nil
 }
 
+func (i *Image) RemoveLabel(key string) error {
+	delete(i.labels, key)
+	return nil
+}
+
 func (i *Image) SetEnv(k string, v string) error {
 	i.env[k] = v
 	return nil

diff --git a/image.go b/image.go
@@ -32,6 +32,7 @@ type Image interface {
 	Label(string) (string, error)
 	Labels() (map[string]string, error)
 	SetLabel(string, string) error
+	RemoveLabel(string) error
 	Env(key string) (string, error)
 	SetEnv(string, string) error
 	SetEntrypoint(...string) error

diff --git a/local/local.go b/local/local.go
@@ -106,7 +106,11 @@ func (i *Image) Label(key string) (string, error) {
 }
 
 func (i *Image) Labels() (map[string]string, error) {
-	return i.inspect.Config.Labels, nil
+	copiedLabels := make(map[string]string)
+	for i, l := range i.inspect.Config.Labels {
+		copiedLabels[i] = l
+	}
+	return copiedLabels, nil
 }
 
 func (i *Image) Env(key string) (string, error) {
@@ -238,6 +242,11 @@ func (i *Image) SetLabel(key, val string) error {
 	return nil
 }
 
+func (i *Image) RemoveLabel(key string) error {
+	delete(i.inspect.Config.Labels, key)
+	return nil
+}
+
 func (i *Image) SetEnv(key, val string) error {
 	ignoreCase := i.inspect.Os == "windows"
 	for idx, kv := range i.inspect.Config.Env {

diff --git a/local/local_test.go b/local/local_test.go
@@ -499,6 +499,60 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
 		})
 	})
 
+	when("#RemoveLabel", func() {
+		var (
+			img           imgutil.Image
+			repoName      = newTestImageName()
+			baseImageName = newTestImageName()
+		)
+
+		it.After(func() {
+			h.AssertNil(t, h.DockerRmi(dockerClient, repoName, baseImageName))
+		})
+
+		when("image exists", func() {
+			it("removes matching label on img object", func() {
+				var err error
+
+				baseImage, err := local.NewImage(baseImageName, dockerClient)
+				h.AssertNil(t, err)
+				h.AssertNil(t, baseImage.SetLabel("my.custom.label", "old-value"))
+				h.AssertNil(t, baseImage.Save())
+
+				img, err = local.NewImage(repoName, dockerClient, local.FromBaseImage(baseImageName))
+				h.AssertNil(t, err)
+
+				h.AssertNil(t, img.RemoveLabel("my.custom.label"))
+				h.AssertNil(t, img.Save())
+
+				labels, err := img.Labels()
+				h.AssertNil(t, err)
+				_, exists := labels["my.custom.label"]
+				h.AssertEq(t, exists, false)
+			})
+
+			it("saves removal of the label", func() {
+				var err error
+
+				baseImage, err := local.NewImage(baseImageName, dockerClient)
+				h.AssertNil(t, err)
+				h.AssertNil(t, baseImage.SetLabel("my.custom.label", "old-value"))
+				h.AssertNil(t, baseImage.Save())
+
+				img, err = local.NewImage(repoName, dockerClient, local.FromBaseImage(baseImageName))
+				h.AssertNil(t, err)
+
+				h.AssertNil(t, img.RemoveLabel("my.custom.label"))
+				h.AssertNil(t, img.Save())
+
+				inspect, _, err := dockerClient.ImageInspectWithRaw(context.TODO(), repoName)
+				h.AssertNil(t, err)
+				_, exists := inspect.Config.Labels["my.custom.label"]
+				h.AssertEq(t, exists, false)
+			})
+		})
+	})
+
 	when("#SetEnv", func() {
 		var repoName = newTestImageName()
 		var skipCleanup bool

diff --git a/remote/remote.go b/remote/remote.go
@@ -278,6 +278,17 @@ func (i *Image) SetLabel(key, val string) error {
 	return err
 }
 
+func (i *Image) RemoveLabel(key string) error {
+	cfg, err := i.image.ConfigFile()
+	if err != nil || cfg == nil {
+		return fmt.Errorf("failed to get config file for image '%s'", i.repoName)
+	}
+	config := *cfg.Config.DeepCopy()
+	delete(config.Labels, key)
+	i.image, err = mutate.Config(i.image, config)
+	return err
+}
+
 func (i *Image) SetEnv(key, val string) error {
 	configFile, err := i.image.ConfigFile()
 	if err != nil {

diff --git a/remote/remote_test.go b/remote/remote_test.go
@@ -451,6 +451,50 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
 		})
 	})
 
+	when("#RemoveLabel", func() {
+		when("image exists", func() {
+			var baseImageName = newTestImageName()
+
+			it.Before(func() {
+				baseImage, err := remote.NewImage(baseImageName, authn.DefaultKeychain)
+				h.AssertNil(t, err)
+				h.AssertNil(t, baseImage.SetLabel("custom.label", "new-val"))
+				h.AssertNil(t, baseImage.Save())
+			})
+
+			it("removes label on img object", func() {
+				img, err := remote.NewImage(repoName, authn.DefaultKeychain, remote.FromBaseImage(baseImageName))
+				h.AssertNil(t, err)
+
+				h.AssertNil(t, img.RemoveLabel("custom.label"))
+
+				labels, err := img.Labels()
+				h.AssertNil(t, err)
+				_, exists := labels["my.custom.label"]
+				h.AssertEq(t, exists, false)
+			})
+
+			it("saves removal of label", func() {
+				img, err := remote.NewImage(repoName, authn.DefaultKeychain, remote.FromBaseImage(baseImageName))
+				h.AssertNil(t, err)
+
+				h.AssertNil(t, img.RemoveLabel("custom.label"))
+				h.AssertNil(t, img.Save())
+
+				testImg, err := remote.NewImage(
+					"test",
+					authn.DefaultKeychain,
+					remote.FromBaseImage(repoName),
+				)
+				h.AssertNil(t, err)
+
+				remoteLabel, err := testImg.Label("custom.label")
+				h.AssertNil(t, err)
+				h.AssertEq(t, remoteLabel, "")
+			})
+		})
+	})
+
 	when("#SetEnv", func() {
 		it("sets the environment", func() {
 			img, err := remote.NewImage(repoName, authn.DefaultKeychain)