injector: add new injectedAnnotations to support PSP

[howardjohn]

Add a new injectedAnnotations setting for the injector. Any annotations
here will be added to the final pod spec. This is required for PSPs to
work. See #17331 for details.

Note - a lot of changes here are to existing tests just to allow setting
all config options on the webhook rather than just the template.

Fixes #17331

[morvencao]

remove this debugging message?

[rshriram]

this is his secret plan for becoming famous and getting his name printed on all the elasticsearches and splunks in the planet :P

[howardjohn]

You caught me... just reverted

[howardjohn]

/retest

[howardjohn]

@rshriram mind approving the changes to injector? (we should probably add an owner to this)

[howardjohn]

/test gencheck_istio

[howardjohn]

/retest

[thecodejunkie]

@howardjohn

I can't for the life of me figure out the syntax for passing in values to the sidecarInjectorWebhook.injectedAnnotations parameter, to set the apparmor annotation 🙂My issue is that the "key" part contains dot-notation so it attempts to split it up no matter how I try to encapsulate it

Using Helm I think the proper way is to encapsulate it in quotes and escape dots

istioctl manifest generate \
    --set cni.enabled=true \
    --set values.sidecarInjectorWebhook.injectedAnnotations."container\.apparmor\.security\.beta\.kubernetes\.io/istio-proxy"=runtime/default \
    --output ./generated

However this results in a mess in the actual manifest

injectedAnnotations:
  "container\": "map[apparmor\:map[security\:map[beta\:map[kubernetes\:map[io/istio-proxy:runtime/default]]]]]"

Doesn't matter if I try to escape the dots or not

Thanks!

[howardjohn]

@thecodejunkie adding complex stuff like this is really painful with helm --set and may not even be possible with istioctl.. I am not sure. What I would recommend is just to use a file. This will work:

ik manifest generate -f <(echo 'apiVersion: install.istio.io/v1alpha2
kind: IstioControlPlane
spec:
  values:
    sidecarInjectorWebhook:
      injectedAnnotations:
        container.apparmor.security.beta.kubernetes.io/istio-proxy: runtime/default')

@richardwxn is this the expected behavior?

[thecodejunkie]

@howardjohn thanks for the update! I believe setting it with helm --set should work if you use the syntax that I used above https://stackoverflow.com/a/50399612 but it might be that istioctl incorrectly (?) handles the escaping? You offered solution definitly works as it saves me from having to manually edit the configmap afterwards (eww to manual steps in a setup process 😄)

[howardjohn]

Yeah the istioctl set is not as complete as helm, might not be supported. Let me open an issue for this so we don't loose the info here

[thecodejunkie]

@howardjohn awesome! Thanks

What would be the proper way to move the cni.enabled=true config into that file? Straight under values?

k manifest generate -f <(echo 'apiVersion: install.istio.io/v1alpha2
kind: IstioControlPlane
spec:
  values:
    cni:
      enabled: true
    sidecarInjectorWebhook:
      injectedAnnotations:
        container.apparmor.security.beta.kubernetes.io/istio-proxy: runtime/default')

[howardjohn]

@thecodejunkie youll want to do

k manifest generate -f <(echo 'apiVersion: install.istio.io/v1alpha2
kind: IstioControlPlane
spec:
  cni:
    enabled: true
  values:
    sidecarInjectorWebhook:
      injectedAnnotations:
        container.apparmor.security.beta.kubernetes.io/istio-proxy: runtime/default')

The --set path is basically just a flattened version of the ICP CRD. It is admittedly a bit confusing where things should be under values and when things shouldn't, we are still iterating on this and trying to improve

[thecodejunkie]

Thanks! I guess one way to think about it is if it's in a "sub-chart" then it goes under values