Setting istio annotation on invalid resource

[xM8WVqaG]

According to the istio annotation docs, the sidecar.istio.io/inject annotation should only be applied to pods - see: https://istio.io/latest/docs/reference/config/annotations/.

$ istioctl analyze -A
Warn [IST0107] (Deployment jaeger-collector.observability) Misplaced annotation: sidecar.istio.io/inject can only be applied to Pod
Warn [IST0107] (Deployment jaeger-query.observability) Misplaced annotation: sidecar.istio.io/inject can only be applied to Pod

Reference error: https://preliminary.istio.io/latest/docs/reference/config/analysis/IST0107/

sidecar.istio.io/inject: false appears on the deployment resources of both jaeger-collector and jaeger-query:

• /metadata/annotations/"sidecar.istio.io/inject": "false" (unexpected)
• /spec/template/metadata/annotations/"sidecar.istio.io/inject": "false" (expected)

$ kubectl get deployment/jaeger-query -n observability -o yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "5"
    linkerd.io/inject: disabled
    prometheus.io/port: "16687"
    prometheus.io/scrape: "true"
    sidecar.istio.io/inject: "false"  # invalid
    sidecar.jaegertracing.io/inject: jaeger
  creationTimestamp: "2020-08-14T10:33:11Z"
  generation: 7
  labels:
    app: jaeger
    app.kubernetes.io/component: query
    app.kubernetes.io/instance: jaeger
    app.kubernetes.io/managed-by: jaeger-operator
    app.kubernetes.io/name: jaeger-query
    app.kubernetes.io/part-of: jaeger
    sidecar.jaegertracing.io/injected: jaeger
  name: jaeger-query
  namespace: observability
  ownerReferences:
  - apiVersion: jaegertracing.io/v1
    controller: true
    kind: Jaeger
    name: jaeger
    uid: 805cea96-0541-4207-b47f-f629860b79e8
  resourceVersion: "51586237"
  selfLink: /apis/apps/v1/namespaces/observability/deployments/jaeger-query
  uid: 50677ba3-a156-42eb-9fbf-45ce42919787
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: jaeger
      app.kubernetes.io/component: query
      app.kubernetes.io/instance: jaeger
      app.kubernetes.io/managed-by: jaeger-operator
      app.kubernetes.io/name: jaeger-query
      app.kubernetes.io/part-of: jaeger
  strategy:
    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%
    type: RollingUpdate
  template:
    metadata:
      annotations:
        linkerd.io/inject: disabled
        prometheus.io/port: "16687"
        prometheus.io/scrape: "true"
        sidecar.istio.io/inject: "false"  # valid
        sidecar.jaegertracing.io/inject: jaeger
...

[jpkrohling]

Nice catch. Would you like to contribute a fix?

[xM8WVqaG]

I am not particularly confident with Golang but having looked through the codebase it looks quite straightforward to change if you'd rather take a PR for this. I expect the hardest part of this change is the most appropriate place to add this logic without being disgusting or complicated.

For example, if I just remove the sidecar.istio.io/inject annotation from the deployment on the baseCommonSpec; should I just deepcopy them after they have been merged with commonSpec and then append it there?

Like this:

podAnnotations := make(map[string]string)

for k, v := range commonSpec.Annotations {
  podAnnotations[k] = v
}

podAnnotations["sidecar.istio.io/inject"] = "false"

[jpkrohling]

Yes, something like that would certainly work. Send in a PR and we can check together if it needs to be done before or after the merge of the base common spec with the common spec.

[primeroz]

Hitting this as well, i was wondering if there would be situations where we might want the sidecar to be enabled ?

[jpkrohling]

Is this causing an actual problem? I thought this was mostly a linting issue?

[primeroz]

linting only ... just cleaning up my istioctl analyze and keep having to add exclusion for things ( not just this ;) )

[jpkrohling]

Thanks for the clarification! Would you be able to send us a PR?

[xM8WVqaG]

Apologies for never submitting the PR last year. tl;dr I was struggling to get the development environment installed locally and didn't want to submit a PR without running tests on it first.

Here is the diff if you want to take this over. I don't expect I will have the time to try and get this working again for a few weeks.

diff --git a/pkg/deployment/collector.go b/pkg/deployment/collector.go
index 672f07e8..fec1410d 100644
--- a/pkg/deployment/collector.go
+++ b/pkg/deployment/collector.go
@@ -46,16 +46,23 @@ func (c *Collector) Get() *appsv1.Deployment {

    baseCommonSpec := v1.JaegerCommonSpec{
        Annotations: map[string]string{
-           "prometheus.io/scrape":    "true",
-           "prometheus.io/port":      strconv.Itoa(int(adminPort)),
-           "sidecar.istio.io/inject": "false",
-           "linkerd.io/inject":       "disabled",
+           "prometheus.io/scrape": "true",
+           "prometheus.io/port":   strconv.Itoa(int(adminPort)),
+           "linkerd.io/inject":    "disabled",
        },
        Labels: labels,
    }

    commonSpec := util.Merge([]v1.JaegerCommonSpec{c.jaeger.Spec.Collector.JaegerCommonSpec, c.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})

+   podAnnotations := make(map[string]string)
+
+   for k, v := range commonSpec.Annotations {
+       podAnnotations[k] = v
+   }
+
+   podAnnotations["sidecar.istio.io/inject"] = "false"
+
    var envFromSource []corev1.EnvFromSource
    if len(c.jaeger.Spec.Storage.SecretName) > 0 {
        envFromSource = append(envFromSource, corev1.EnvFromSource{
@@ -119,7 +126,7 @@ func (c *Collector) Get() *appsv1.Deployment {
            Template: corev1.PodTemplateSpec{
                ObjectMeta: metav1.ObjectMeta{
                    Labels:      commonSpec.Labels,
-                   Annotations: commonSpec.Annotations,
+                   Annotations: podAnnotations,
                },
                Spec: corev1.PodSpec{
                    Containers: []corev1.Container{{
diff --git a/pkg/deployment/query.go b/pkg/deployment/query.go
index fdb9ce51..0da6491b 100644
--- a/pkg/deployment/query.go
+++ b/pkg/deployment/query.go
@@ -41,10 +41,9 @@ func (q *Query) Get() *appsv1.Deployment {

    baseCommonSpec := v1.JaegerCommonSpec{
        Annotations: map[string]string{
-           "prometheus.io/scrape":    "true",
-           "prometheus.io/port":      strconv.Itoa(int(adminPort)),
-           "sidecar.istio.io/inject": "false",
-           "linkerd.io/inject":       "disabled",
+           "prometheus.io/scrape": "true",
+           "prometheus.io/port":   strconv.Itoa(int(adminPort)),
+           "linkerd.io/inject":    "disabled",
        },
        Labels: labels,
    }
@@ -63,6 +62,14 @@ func (q *Query) Get() *appsv1.Deployment {

    commonSpec := util.Merge([]v1.JaegerCommonSpec{q.jaeger.Spec.Query.JaegerCommonSpec, q.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})

+   podAnnotations := make(map[string]string)
+
+   for k, v := range commonSpec.Annotations {
+       podAnnotations[k] = v
+   }
+
+   podAnnotations["sidecar.istio.io/inject"] = "false"
+
    options := allArgs(q.jaeger.Spec.Query.Options,
        q.jaeger.Spec.Storage.Options.Filter(q.jaeger.Spec.Storage.Type.OptionsPrefix()))

@@ -110,7 +117,7 @@ func (q *Query) Get() *appsv1.Deployment {
            Template: corev1.PodTemplateSpec{
                ObjectMeta: metav1.ObjectMeta{
                    Labels:      commonSpec.Labels,
-                   Annotations: commonSpec.Annotations,
+                   Annotations: podAnnotations,
                },
                Spec: corev1.PodSpec{
                    Containers: []corev1.Container{{

[primeroz]

I can have a look at it and send a pr

Thanks !

[Kirial]

This seems to still be an issue. What is the status of this PR @primeroz

[primeroz]

I am moved onto other things in the meantime so not really looking at doing 5his anytime soon