Add support for proxy injector reinvocation

Fixes #3750 and partially #6267 As of k8s 1.15 mutating webhooks can be reinvoked whenever another mutating webhook running after the current one mutates the pod being persisted. Enabling reinvocation for the injector will allow configuring the proxy with annotations generated by such other mutating webhooks. The implementation consists on adding "remove" statements into the json patch returned by the injector, implemented mostly in the new file `pkg/inject/pod_patch.go` which now holds the `podPatch`, moved from `pkg/inject/inject.go`. This also updates the "reinvocation" integration test introduced in #6309 to properly verify the reinvocation is happening. And this also means the "existing proxy sidecar" check is no longer relevant, and has been limited to "existing 3rd party sidecar". Possible followup: refactor the CLI inject and uninject commands to properly leverage the cleanup done by this patch.
linkerd · Jun 22, 2021 · 2532298 · 2532298
1 parent 9b7a548
commit 2532298
Show file tree

Hide file tree

Showing 30 changed files with 226 additions and 81 deletions.
diff --git a/charts/linkerd2/templates/proxy-injector-rbac.yaml b/charts/linkerd2/templates/proxy-injector-rbac.yaml
@@ -94,6 +94,7 @@ webhooks:
     caBundle: {{ ternary (b64enc (trim $ca.Cert)) (b64enc (trim .Values.proxyInjector.caBundle)) (empty .Values.proxyInjector.caBundle) }}
   failurePolicy: {{.Values.webhookFailurePolicy}}
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]

diff --git a/charts/patch/templates/patch.json b/charts/patch/templates/patch.json
@@ -1,5 +1,38 @@
 {{ $prefix := .Values.pathPrefix -}}
+{{ $alreadyInjected := .Values.alreadyInjected -}}
 [
+  {{- if $alreadyInjected }}
+  {
+    "op": "remove",
+    "path": "/metadata/annotations/linkerd.io~1created-by"
+  },
+  {
+    "op": "remove",
+    "path": "/metadata/annotations/linkerd.io~1identity-mode"
+  },
+  {
+    "op": "remove",
+    "path": "/metadata/annotations/linkerd.io~1proxy-version"
+  },
+  {{- if gt .Values.proxyInitIndex -1.0 }}
+  {
+    "op": "remove",
+    "path": "/spec/initContainers/{{ .Values.proxyInitIndex }}"
+  },
+  {{- end }}
+  {{ range .Values.containerIndices }}
+  {
+    "op": "remove",
+    "path": "/spec/containers/{{ . }}"
+  },
+  {{- end }}
+  {{ range .Values.volumeIndices }}
+  {
+    "op": "remove",
+    "path": "{{$prefix}}/spec/volumes/{{ . }}"
+  },
+  {{- end }}
+  {{- end }}
   {{- if .Values.addRootMetadata }}
   {
     "op": "add",
@@ -29,6 +62,12 @@
   },
   {{- end }}
   {{- range $label, $value := .Values.labels }}
+  {{- if $alreadyInjected }}
+  {
+    "op": "remove",
+    "path": "{{$prefix}}/metadata/labels/{{$label | replace "/" "~1"}}"
+  },
+  {{- end }}
   {
     "op": "add",
     "path": "{{$prefix}}/metadata/labels/{{$label | replace "/" "~1"}}",

diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go
@@ -259,7 +259,7 @@ func (resourceTransformerInject) generateReport(reports []inject.Report, output
 			warningsPrinted = true
 		}
 
-		if r.Sidecar {
+		if r.OtherSidecar {
 			sidecar = append(sidecar, r.ResName())
 			warningsPrinted = true
 		}

diff --git a/cli/cmd/testdata/inject_emojivoto_istio.golden.stderr b/cli/cmd/testdata/inject_emojivoto_istio.golden.stderr
@@ -1,2 +1,2 @@
 Error transforming resources:
-failed to inject deployment/web: pod has a sidecar injected already
+failed to inject deployment/web: pod has a 3rd party sidecar injected already
diff --git a/cli/cmd/testdata/inject_emojivoto_istio.golden.stderr.verbose b/cli/cmd/testdata/inject_emojivoto_istio.golden.stderr.verbose
@@ -1,2 +1,2 @@
 Error transforming resources:
-failed to inject deployment/web: pod has a sidecar injected already
+failed to inject deployment/web: pod has a 3rd party sidecar injected already
diff --git a/cli/cmd/testdata/install_controlplane_tracing_output.golden b/cli/cmd/testdata/install_controlplane_tracing_output.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1613,7 +1614,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_custom_domain.golden b/cli/cmd/testdata/install_custom_domain.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1611,7 +1612,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: bce2277a89759f9ac7669e8043ef265aca604e32fa847929ea3bfa3327905042
+        checksum/config: 947f6765b3d905bb3039518bc62e75e793b71c35077a41e998a447cbcdd0dd59
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_custom_registry.golden b/cli/cmd/testdata/install_custom_registry.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1611,7 +1612,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_default.golden b/cli/cmd/testdata/install_default.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1611,7 +1612,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_default_override_dst_get_nets.golden b/cli/cmd/testdata/install_default_override_dst_get_nets.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1611,7 +1612,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_ha_output.golden b/cli/cmd/testdata/install_ha_output.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Fail
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1744,7 +1745,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 67e053df1cc859aa15c82ff6e5e65c055041bb36ade50655e9bf44976521018f
+        checksum/config: bfad37112b96b6e013dc41570c1392d15e39a86b5045069ff2f3be121b879bce
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_ha_with_overrides_output.golden b/cli/cmd/testdata/install_ha_with_overrides_output.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Fail
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1744,7 +1745,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 67e053df1cc859aa15c82ff6e5e65c055041bb36ade50655e9bf44976521018f
+        checksum/config: bfad37112b96b6e013dc41570c1392d15e39a86b5045069ff2f3be121b879bce
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_heartbeat_disabled_output.golden b/cli/cmd/testdata/install_heartbeat_disabled_output.golden
@@ -620,6 +620,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1490,7 +1491,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_helm_output.golden b/cli/cmd/testdata/install_helm_output.golden
@@ -703,6 +703,7 @@ webhooks:
     caBundle: dGVzdC1wcm94eS1pbmplY3Rvci1jYS1idW5kbGU=
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1595,7 +1596,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 20a7b3bda0bfdd9b7cf388efb0b438612caa444c23e015168dae1cdb0109e34e
+        checksum/config: 43b44889d13d2f72c242740c80e9e1f34d7b4972e4c34ea2ca763d3c7db03bb7
         linkerd.io/created-by: linkerd/helm linkerd-version
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: test-proxy-version

diff --git a/cli/cmd/testdata/install_helm_output_ha.golden b/cli/cmd/testdata/install_helm_output_ha.golden
@@ -703,6 +703,7 @@ webhooks:
     caBundle: dGVzdC1wcm94eS1pbmplY3Rvci1jYS1idW5kbGU=
   failurePolicy: Fail
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1728,7 +1729,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: cf6b4520e0ad2b0010db5a18443bd632c87ac0216f8f118e723e0f33f5842d7e
+        checksum/config: 25d57778ef08f0488f65425442e17e4d8d3f8dde38cf6c1967b1934f34e09f5f
         linkerd.io/created-by: linkerd/helm linkerd-version
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: test-proxy-version

diff --git a/cli/cmd/testdata/install_helm_output_ha_labels.golden b/cli/cmd/testdata/install_helm_output_ha_labels.golden
@@ -703,6 +703,7 @@ webhooks:
     caBundle: dGVzdC1wcm94eS1pbmplY3Rvci1jYS1idW5kbGU=
   failurePolicy: Fail
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1744,7 +1745,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: cf6b4520e0ad2b0010db5a18443bd632c87ac0216f8f118e723e0f33f5842d7e
+        checksum/config: 25d57778ef08f0488f65425442e17e4d8d3f8dde38cf6c1967b1934f34e09f5f
         linkerd.io/created-by: linkerd/helm linkerd-version
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: test-proxy-version

diff --git a/cli/cmd/testdata/install_helm_output_ha_namespace_selector.golden b/cli/cmd/testdata/install_helm_output_ha_namespace_selector.golden
@@ -703,6 +703,7 @@ webhooks:
     caBundle: dGVzdC1wcm94eS1pbmplY3Rvci1jYS1idW5kbGU=
   failurePolicy: Fail
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1728,7 +1729,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 5e2cfa2bd882cd79c2104f822ff8062620bd9103e176aa47064940e2e0fbcff6
+        checksum/config: f96549a803c94c26c0b6096c94c24437ecbcc3dc661c2002ee3699e5ade4f39d
         linkerd.io/created-by: linkerd/helm linkerd-version
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: test-proxy-version

diff --git a/cli/cmd/testdata/install_no_init_container.golden b/cli/cmd/testdata/install_no_init_container.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1532,7 +1533,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_output.golden b/cli/cmd/testdata/install_output.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: WebhookFailurePolicy
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1615,7 +1616,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 54ac8689f1236f2c97fa3bb59f9ac21abab03e06e06eca84b1fd9ba035dce1e8
+        checksum/config: dfac38d566deb1fb3525a79a967508edd6ceb50c719b6f62127e3e6bbdd9dae7
         linkerd.io/created-by: CliVersion
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: ProxyVersion

diff --git a/cli/cmd/testdata/install_proxy_ignores.golden b/cli/cmd/testdata/install_proxy_ignores.golden
@@ -689,6 +689,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1611,7 +1612,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: 31ca92e63a48870d9b7cad9d7a613e2164b4b016577720744b2f0a1da3a3c844
+        checksum/config: 7f146039db892a5d5c7c8a9e53b3140889705c6176226af971ea9801741e43b9
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/cli/cmd/testdata/install_values_file.golden b/cli/cmd/testdata/install_values_file.golden
@@ -675,6 +675,7 @@ webhooks:
     caBundle: cHJveHkgaW5qZWN0b3IgQ0EgYnVuZGxl
   failurePolicy: Ignore
   admissionReviewVersions: ["v1", "v1beta1"]
+  reinvocationPolicy: IfNeeded
   rules:
   - operations: [ "CREATE" ]
     apiGroups: [""]
@@ -1597,7 +1598,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: bce2277a89759f9ac7669e8043ef265aca604e32fa847929ea3bfa3327905042
+        checksum/config: 947f6765b3d905bb3039518bc62e75e793b71c35077a41e998a447cbcdd0dd59
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version

diff --git a/controller/proxy-injector/webhook_test.go b/controller/proxy-injector/webhook_test.go
@@ -229,6 +229,10 @@ func TestGetPodPatch(t *testing.T) {
 
 		fakeReq := getFakePodReq(deployment)
 		conf := confNsDisabled().WithKind(fakeReq.Kind.Kind)
+		_, err = conf.ParseMetaAndYAML(fakeReq.Object.Raw)
+		if err != nil {
+			t.Fatalf("Unexpected ParseMetaAndYAML error: %s", err)
+		}
 		patchJSON, err := conf.GetPodPatch(true)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)

diff --git a/controller/webhook/server.go b/controller/webhook/server.go
@@ -160,7 +160,6 @@ func (s *Server) processReq(ctx context.Context, data []byte) *admissionv1beta1.
 		return admissionReview
 	}
 	log.Infof("received admission review request %s", admissionReview.Request.UID)
-	log.Debugf("admission request: %+v", admissionReview.Request)
 
 	admissionResponse, err := s.handler(ctx, s.api, admissionReview.Request, s.recorder)
 	if err != nil {