[Question] Supplying TLS files to jaeger-agent sidecar using jaeger-operator

[Saad-Hussain1]

The jaeger-agent has CLI flags to enable TLS between it and the collector, e.g. --reporter.grpc.tls.ca, which requires supplying a file. Since the jaeger-agent sidecar doesn't have any capability to mount volumes when using the jaeger-operator, how would one supply this file?

A follow-up question to that - why doesn't the sidecar read kubernetes configurations under spec.agent? For example, for volume mounts, if this spec:

jaeger-operator/pkg/inject/sidecar.go

Line 197 in fba067f

volumesAndMountsSpec := &v1.JaegerCommonSpec{}

was set to jaeger.Spec.Agent.JaegerCommonSpec, the agent could mount volumes and it would only be done if someone specifically puts a field under spec.agent in the yaml, which would be very intentional. It seems ideal if it were to do this for all the fields here: https://www.jaegertracing.io/docs/1.18/operator/#finer-grained-configuration (the documentation made me think this was the case anyway, but it isn't with the sidecar)

[objectiser]

@jpkrohling Thoughts? I think the control over the volume/mounts was to prevent definitions at the top level inadvertently being added to the app deployments, however as @Saad-Hussain1 says, if the volumes/mounts are explcitly placed under the agent node, then they are more controlled.

@Saad-Hussain1 In terms of your second point, resources are currently applied but you are correct the other fields are not used. Would you be interested in submitting a PR to include the relevant fields?

[jpkrohling]

Looks like it has been introduced by #1056. @pavolloffay, do you remember if there are any reasons why .Spec.Agent.JaegerCommonSpec hasn't been used?

[Saad-Hussain1]

My main problem is that at the moment, it seems I am unable to supply the files needed (certs/keys) to the agent in order to enable TLS between the agent and collector (e.g. for arg --reporter.grpc.tls.ca). The only way I can think of doing this is through volumeMounts but I don't see how I can do it in this case. If there is a way to do this and I'm missing something, please do let me know.

Otherwise, I was thinking of simply incorporating a very small change like this: https://github.com/jaegertracing/jaeger-operator/pull/1102/files

That way, I can mount a volume in the agent sidecar that can house my TLS files by supplying the volume/mount details under jaeger.Spec.Agent.Volumes/jaeger.Spec.Agent.VolumeMounts. Does this sound like something we could do?

[pavolloffay]

Looks like it has been introduced by #1056. @pavolloffay, do you remember if there are any reasons why .Spec.Agent.JaegerCommonSpec hasn't been used?

I am not sure if I get the question right. The OTEL config wasn't added to .Spec.Agent.JaegerCommonSpec because not all components support OTEL config (e.g. query).

If the question is about volumes on the sidecar then there is a comment https://github.com/jaegertracing/jaeger-operator/pull/1056/files#diff-acd8a9fe2d2997867f151eb11780379bR193.

	// Use a different common spec for volumes and mounts.
	// We don't want to mount all Jaeger internal volumes into user's deployments

[jpkrohling]

// We don't want to mount all Jaeger internal volumes into user's deployments

What's a Jaeger internal volume? If a user specifies a volume as part of the Jaeger CR, it should surely be propagated to the agent's sidecar, no ?

[Saad-Hussain1]

@pavolloffay Hm so let me ask my main question again because I hope I'm not missing something - is there a way to supply files to the jaeger-agent when it's deployed as a sidecar using the jaeger-operator? For example if trying to enable TLS between the agent and collector, the CLI flag --reporter.grpc.tls.ca requires a file with a CA cert.

The only way I can think of is through volumes/volume mounts, but those aren't supported on the agent sidecar.

I was thinking we could add this change: https://github.com/jaegertracing/jaeger-operator/pull/1102/files

As for this comment:

        // Use a different common spec for volumes and mounts.
	// We don't want to mount all Jaeger internal volumes into user's deployments

I don't think the PR I just linked necessarily opposes that idea; any volumes under the jaeger common spec still aren't added to the deployment which the sidecar is inject to - only volumes that are put under the spec of the agent node are added to the deployment, which would be a very intentional decision to add a volume imo.

[jpkrohling]

@Saad-Hussain1: +1. My previous question was for @pavolloffay. Unless he had other concerns in mind, your solution sounds good to me.

[Saad-Hussain1]

@jpkrohling Oh haha, didn't see your reply when I messaged.

Maybe I can share my guess regarding your question though?

My guess was that "Jaeger internal volumes" refers to volumes that are defined under the spec in a Jaeger strategy yaml, as opposed to those that are defined under spec.<component>.

According to https://www.jaegertracing.io/docs/1.18/operator/#finer-grained-configuration,
"When a common definition (for all Jaeger components) is required, it is defined under the spec node. When the definition relates to an individual component, it is placed under the spec/<component> node."

So for a Jaeger strategy yaml like this:

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: simple-prod
spec:
  strategy: production
...
  collector:
    volumeMounts:
    - name: collector-volume
      mountPath: /usr/app
    volumes:
      - name: collector-volume
        configMap:
          name: collector-config
  volumeMounts:
    - name: general-volume
      mountPath: /etc/config
  volumes:
    - name: general-volume
      configMap:
        name: general-config

general-volume is added to the deployment for all components, whereas collector-volume is only added to the deployment for the collector. Since the operator creates and manages the deployment of the collector, this seems fair. But for the sidecar, it can be injected into deployments that are not managed by the jaeger-operator, so this may be undesirable (volumes are added at the deployment level, not container level).

At least that's how I understood it, which is why I thought that if someone were to specify a volume under spec.agent it would be intentional enough to warrant modifying the sidecar's deployment.

[pavolloffay]

You are definitely right, there should be a way to add certs for the agent's reporter and that is supported via the common spec at the moment. In the PR #1056 I was rather conservative and added only volumes that I know are required.

[tillig]

It looks like the latest JaegerAgentSpec includes JaegerCommonSpec so maybe this is fixed?