Add sideEffects field to Webhooks

[Pothulapati]

Fixes #2850

This PR adds the sideEffects field to the webhooks by default. As sideEffects field is supported from kubernetes version ge 1.12, the PR also bumps the minSupported version for check to 1.12

For users, to run on clusters < 1.12, they can use the --omit-webhook-side-effects to omit the sideEffects during installation, and this is flag is persisted during upgrades.

@ihcsim @grampelberg

[ihcsim]

Thanks for this PR. See comments below.

[Pothulapati]

Updated. :) @ihcsim

[ihcsim]

Thanks!

I am using this branch to do some testing on upgrade atm. I notice that we need to update the config.Global protobuf to include this new field too. Otherwise, pre-1.12 users will have to keep providing the --omit-webhook-side-effect flag every time they perform an upgrade.

Here's a diff for the needed change:

diff --git a/cli/cmd/install.go b/cli/cmd/install.go
index 53c4f406..359bfab8 100644
--- a/cli/cmd/install.go
+++ b/cli/cmd/install.go
@@ -761,10 +761,11 @@ func (options *installOptions) configs(identity *pb.IdentityContext) *pb.All {
 
 func (options *installOptions) globalConfig(identity *pb.IdentityContext) *pb.Global {
 	return &pb.Global{
-		LinkerdNamespace: controlPlaneNamespace,
-		CniEnabled:       options.noInitContainer,
-		Version:          options.controlPlaneVersion,
-		IdentityContext:  identity,
+		LinkerdNamespace:       controlPlaneNamespace,
+		CniEnabled:             options.noInitContainer,
+		Version:                options.controlPlaneVersion,
+		IdentityContext:        identity,
+		OmitWebhookSideEffects: options.omitWebhookSideEffects,
 	}
 }
 
diff --git a/proto/config/config.proto b/proto/config/config.proto
index 19e3320f..5ee8975d 100644
--- a/proto/config/config.proto
+++ b/proto/config/config.proto
@@ -23,6 +23,8 @@ message Global {
   IdentityContext identity_context = 4;
 
   AutoInjectContext auto_inject_context = 6 [deprecated=true];
+
+  bool omitWebhookSideEffects = 7;
 }
 
 message Proxy {

I omitted the unit tests diff because they are too long. You can update them with go test ./cli/... --update.

[ihcsim]

@Pothulapati Thanks for the quick turnaround.

I discovered a tricky issue in upgrade. If I install the control plane with the --omit-webhook-side-effects=true option (pretending I'm on k8s 1.11), and then try to upgrade it with --omit-webhook-side-effects=false (say, now I'm on k8s 1.15), the config.Global.OmitWebhookSideEffects config becomes out-of-sync.

To reproduce:

bin/linkerd version
Client version: git-7e06dde6
Server version: unavailable

# omit the webhook side effects field
bin/linkerd install --omit-webhook-side-effects=true |k apply -f -

# these are correct
k -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%
k -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%
k -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": true

# do an upgrade to enable the field
bin/linkerd upgrade --omit-webhook-side-effects=false|k apply -f -

# this is out-of-sync; it should be false
k -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": true

# these are correct
k -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%
k -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%

We just need to make sure that upgrade picks up the new value of the flag:

diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
index ee5c43e9..94f0e430 100644
--- a/cli/cmd/upgrade.go
+++ b/cli/cmd/upgrade.go
@@ -218,6 +218,7 @@ func (options *upgradeOptions) validateAndBuild(stage string, k kubernetes.Inter
 		configs.GetGlobal().Version = options.controlPlaneVersion
 	}
 	configs.GetInstall().Flags = options.recordedFlags
+	configs.GetGlobal().OmitWebhookSideEffects = options.omitWebhookSideEffects
 
 	var identity *installIdentityValues
 	idctx := configs.GetGlobal().GetIdentityContext()

[Pothulapati]

Understood and updated :)
@ihcsim

[ihcsim]

LGTM 👍

(Maybe my kubectl is an older version, the --server-dry-run doesn't seem to do much on my local cluster.)

Here are the tests I've run (with auto-inject on emojivoto):

New installation:

bin/linkerd version --client
Client version: git-c74881e

bin/linkerd install |k apply -f -

kubectl -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": false
kubectl -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%
kubectl -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%

Upgrade from edge-19.6.2:

linkerd version --client
Client version: edge-19.6.2

kubectl -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
kubectl -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%
kubectl -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%

bin/linkerd upgrade | kubectl apply -f -
bin/linkerd version
Client version: git-7e06dde6
Server version: git-7e06dde6

kubectl -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": false
kubectl -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%
kubectl -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%

Install with --omit-webhook-side-effects=true

bin/linkerd version
Client version: git-7e06dde6
Server version: unavailable

bin/linkerd install --omit-webhook-side-effects=true |k apply -f -
kubectl -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%
kubectl -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
Unknown%
kubectl -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": true

bin/linkerd upgrade --omit-webhook-side-effects=false | kubectl apply -f -
kubectl -n linkerd get cm linkerd-config -ojsonpath='{.data.global}' | jq . | grep -i omitwebhooksideeffects
  "omitWebhookSideEffects": false
kubectl -n linkerd get mutatingwebhookconfigurations.admissionregistration.k8s.io linkerd-proxy-injector-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%
kubectl -n linkerd get validatingwebhookconfigurations.admissionregistration.k8s.io linkerd-sp-validator-webhook-config -ojsonpath='{.webhooks[*].sideEffects}'
None%

[Pothulapati]

@ihcsim dry-run dosen't effect anything with emojivoto because, it is a deployment and our webhooks mutate only pods as in dry-run the deployment isnt actually run, dry-run without sideEffects:None dosent fail, try with a pod deployment with linkerd injection label, it will fail when dry-run is enabled and sideEffects:None is not present :)

[ihcsim]

@Pothulapati Ha! Thanks for the tips. You are right. The --server-dry-run is in-effect only with pod workloads, because the Deployment is never persisted by the api server. Hence, our proxy injector isn't triggered, as no pods are created.

E.g. on a PSP-enabled cluster with limit range defined, I can use

kubectl apply --server-dry-run -f my-pod.yaml -o yaml

to see the final injected YAML of my pod. Also, with service profile validator, I can see the sp-validator picks up the admission requests in dry run mode, instead of rejecting it.