Patch.go refactoring

- Created the pkg/inject package to hold the new injection shared lib. - Moved patch.go there. - Added the ability to add pod labels and annotations without having to specify the already existing ones. For that purpose, added `patch.addPodLabelsRoot()` and `addPodAnnotationsRoot()`, and repurposed `addPodLabels()` into `addPodLabel()`, and `addPodAnnotations()` into `addPodAnnotation()` - Removed `addDeploymentLabels()` because I couldn't find the need for it. At least `linkerd inject` wasn't setting those labels. Please confirm in case I'm wrong. Still to-do: have `patch_test.go` depend on fixtures separate from the ones under `controller/proxy-injector` Ref #1748 Signed-off-by: Alejandro Pedraza <[email protected]>
linkerd · Feb 26, 2019 · 61a7104 · 61a7104
1 parent ec5a0ca
commit 61a7104
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 30 deletions.
diff --git a/controller/proxy-injector/fake/factory.go b/controller/proxy-injector/fake/factory.go
@@ -30,8 +30,8 @@ type Factory struct {
 }
 
 // NewFactory returns a new instance of Fixture.
-func NewFactory() *Factory {
-	return &Factory{rootDir: filepath.Join("fake", "data")}
+func NewFactory(rootDir string) *Factory {
+	return &Factory{rootDir: rootDir}
 }
 
 // HTTPRequestBody returns the content of the specified file as a slice of

diff --git a/controller/proxy-injector/server_test.go b/controller/proxy-injector/server_test.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -33,9 +34,8 @@ func init() {
 		panic(err)
 	}
 	log.SetOutput(ioutil.Discard)
-	factory = fake.NewFactory()
+	factory = fake.NewFactory(filepath.Join("fake", "data"))
 
-	factory = fake.NewFactory()
 	testServer = &WebhookServer{nil, webhook}
 }
 

diff --git a/controller/proxy-injector/patch.go → pkg/inject/patch.go b/controller/proxy-injector/patch.go → pkg/inject/patch.go
@@ -1,6 +1,8 @@
-package injector
+package inject
 
 import (
+	"strings"
+
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -67,30 +69,42 @@ func (p *Patch) addVolume(volume *corev1.Volume) {
 	})
 }
 
-func (p *Patch) addPodLabels(label map[string]string) {
+func (p *Patch) addPodLabelsRoot() {
 	p.patchOps = append(p.patchOps, &patchOp{
 		Op:    "add",
 		Path:  patchPathPodLabels,
-		Value: label,
+		Value: map[string]string{},
+	})
+}
+
+func (p *Patch) addPodLabel(key, value string) {
+	p.patchOps = append(p.patchOps, &patchOp{
+		Op:    "add",
+		Path:  patchPathPodLabels + "/" + escapeKey(key),
+		Value: value,
 	})
 }
 
-func (p *Patch) addPodAnnotations(annotation map[string]string) {
+func (p *Patch) addPodAnnotationsRoot() {
 	p.patchOps = append(p.patchOps, &patchOp{
 		Op:    "add",
 		Path:  patchPathPodAnnotations,
-		Value: annotation,
+		Value: map[string]string{},
 	})
 }
 
-func (p *Patch) addDeploymentLabels(label map[string]string) {
+func (p *Patch) addPodAnnotation(key, value string) {
 	p.patchOps = append(p.patchOps, &patchOp{
 		Op:    "add",
-		Path:  patchPathDeploymentLabels,
-		Value: label,
+		Path:  patchPathPodAnnotations + "/" + escapeKey(key),
+		Value: value,
 	})
 }
 
+func escapeKey(str string) string {
+	return strings.Replace(str, "/", "~1", -1)
+}
+
 // patchOp represents a RFC 6902 patch operation.
 type patchOp struct {
 	Op    string      `json:"op"`

diff --git a/controller/proxy-injector/patch_test.go → pkg/inject/patch_test.go b/controller/proxy-injector/patch_test.go → pkg/inject/patch_test.go
@@ -1,6 +1,7 @@
-package injector
+package inject
 
 import (
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -10,7 +11,7 @@ import (
 )
 
 func TestPatch(t *testing.T) {
-	fixture := fake.NewFactory()
+	fixture := fake.NewFactory(filepath.Join("..", "..", "controller", "proxy-injector", "fake", "data"))
 
 	trustAnchors, err := fixture.Volume("inject-trust-anchors-volume-spec.yaml")
 	if err != nil {
@@ -44,15 +45,8 @@ func TestPatch(t *testing.T) {
 	actual.addVolumeRoot()
 	actual.addVolume(trustAnchors)
 	actual.addVolume(secrets)
-	actual.addPodLabels(map[string]string{
-		k8sPkg.ControllerNSLabel: controllerNamespace,
-	})
-	actual.addDeploymentLabels(map[string]string{
-		k8sPkg.ControllerNSLabel: controllerNamespace,
-	})
-	actual.addPodAnnotations(map[string]string{
-		k8sPkg.CreatedByAnnotation: createdBy,
-	})
+	actual.addPodLabel(k8sPkg.ControllerNSLabel, controllerNamespace)
+	actual.addPodAnnotation(k8sPkg.CreatedByAnnotation, createdBy)
 
 	expected := NewPatch()
 	expected.patchOps = []*patchOp{
@@ -62,13 +56,8 @@ func TestPatch(t *testing.T) {
 		{Op: "add", Path: patchPathVolumeRoot, Value: []*corev1.Volume{}},
 		{Op: "add", Path: patchPathVolume, Value: trustAnchors},
 		{Op: "add", Path: patchPathVolume, Value: secrets},
-		{Op: "add", Path: patchPathPodLabels, Value: map[string]string{
-			k8sPkg.ControllerNSLabel: controllerNamespace,
-		}},
-		{Op: "add", Path: patchPathDeploymentLabels, Value: map[string]string{
-			k8sPkg.ControllerNSLabel: controllerNamespace,
-		}},
-		{Op: "add", Path: patchPathPodAnnotations, Value: map[string]string{k8sPkg.CreatedByAnnotation: createdBy}},
+		{Op: "add", Path: patchPathPodLabels + "/" + escapeKey(k8sPkg.ControllerNSLabel), Value: controllerNamespace},
+		{Op: "add", Path: patchPathPodAnnotations + "/" + escapeKey(k8sPkg.CreatedByAnnotation), Value: createdBy},
 	}
 
 	if !reflect.DeepEqual(actual, expected) {