banzaicloud/istio-operator

Ability to configure modifications to default injection templates

tiswanso opened this issue · 3 comments

Is your feature request related to a problem? Please describe.
The istio-operator's built in injection templates are not able to be modified without applying annotations to each application deployment spec's pod template. For istio-admins to be able to add custom settings in the injection templates for non-annotated workloads, a mechanism is needed to customize the default injection templates.

Describe the solution you'd like to see
The current istio community operator has the following mechanism to merge custom settings with the default sidecar injection template https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/#custom-templates-experimental

The following is the configuration interface for the istio.io operator to specify what to merge into the sidecar injection template:

apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  name: istio
spec:
  values:
    sidecarInjectorWebhook:
      templates:
        custom: |
          spec:
            containers:
            - name: istio-proxy
              env:
              - name: GREETING
                value: hello-world

Still the templates annotation at the app deployment/pod level is needed to specify the merge:

For example, to apply the default template and our customization, you can set inject.istio.io/templates=sidecar,custom

It'd be a better admin experience to allow a similar mechanism to actually specify customizations to the default injection templates. Possibly the config interface would be better if it followed a similar pattern as the istio pod annotations, e.g. something like:

apiVersion: servicemesh.cisco.com/v1alpha1
  kind: IstioControlPlane
  metadata:
   ...
  spec:
     sidecarInjector:
        templateCustomization:
          sidecar:
             userVolume:
                 - name: custom-id
                    secret:
                       optional: true
                       {{- if (isset .ObjectMeta.Annotations `sidecar.istio.io/istioCustomIdSecretName`) }}
                       secretName: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/istioCustomIdSecretName` }}"
                       {{- else }}
                       secretName: placeholder
                       {{- end }}
            userVolumeMount:
                 - name: custom-id
                    mountPath: /etc/istio/custom-id
                    readOnly: true
          gateway:
             proxyImage:  ghcr.io/myorg/special-gateway-proxy:vX.y.z

The above would apply customizations to the actual injection templates sidecar and gateway, respectively. The proposed behavior is the settings would function in the same way as the current Istio behavior of having pod deployments with the pod annotations:

sidecar.istio.io/userVolume
sidecar.istio.io/userVolumeMount

And for the gateway deployment (inject.istio.io/templates=gateway:

sidecar.istio.io/proxyImage

Describe alternatives you've considered
With no mechanism to modify the default templates at runtime, the admin experience is to have to rebuild the istio-operator with the assets modified to include the injection template customizations. This makes it difficult to provide customization options for different admin scenarios.

Additional context

Thanks @tiswanso for bringing this up!

We would like to allow the users to make changes to these templates without providing the annotations on deployment/pod levels, that's completely fine. Your proposed solution should include a merge logic on our side to the original sidecar and gateway templates should be merged with the user provided ones. Also, it involves creating a new API, which should be learned by the user and implemented by us with the internal merge logic.

Counter proposal for the implementation:

We can specify the default templates for the created resources. These are sidecar and gateway by default, but could be something like sidecar, sidecarOverrides and gateway, gatewayOverrides. By default the overrides could be empty, which would cause essentially the original behaviour (other option is to not even include the overrides by default, probably doable with helm templating and only include the overrides when custom settings are provided).

The user could provide something like this:

apiVersion: servicemesh.cisco.com/v1alpha1
  kind: IstioControlPlane
  metadata:
   ...
  spec:
    sidecarInjector:
      templates:
        sidecar: |
          spec:
            containers:
            - name: istio-proxy
              volumeMounts:
              - name: custom-id
                mountPath: /etc/istio/custom-id
                readOnly: true
            volumes:
            - name: custom-id
              secret:
                  optional: true
                  {{- if (isset .ObjectMeta.Annotations `sidecar.istio.io/istioCustomIdSecretName`) }}
                  secretName: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/istioCustomIdSecretName` }}"
                  {{- else }}
                  secretName: placeholder
                  {{- end }}
        custom: |
          spec:
            containers:
            - name: foo
              image: bar 

The sidecar option would the the same as in your proposal from the user's perspective, would be the overriden default injection config without needing to provide any annotation on deplyments/pods. But it uses the usual deployment spec format, no need to learn a new API for the user. Also, no need to implement a merge logic on our side, because this would be used as the sidecarOverride template and would utilize Istio's already existing merging logic, because the default templates would be sidecar, sidecarOverrides as suggested above (these overrides would be implementation details only, not exposed to the user on the API).

Same logic would apply to the gateway template.

Also, the user could potentially define custom templates as in the example above. Those would not be added to the defaults, but could be referenced by the user with annotations just like in upstream.

Cool... so the sidecar section maps to sidecarOverride internally and merge logic would be equivalent to all pods having annotations like inject.istio.io/templates=sidecar,sidecarOverrid, right?

That works for me :-)

Yes, exactly.