Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for configuring the injected sidecar container's securityContext #1186

Closed
chgl opened this issue Sep 8, 2020 · 2 comments · Fixed by #1190
Closed

Allow for configuring the injected sidecar container's securityContext #1186

chgl opened this issue Sep 8, 2020 · 2 comments · Fixed by #1190
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest

Comments

@chgl
Copy link
Contributor

chgl commented Sep 8, 2020

Currently, the securityContext of the injected jaeger-agent container can not be configured, causing the agent container to fail with "container has runAsNonRoot and image will run as root" when running in a cluster with strict PSPs.

We could reuse the securityContext from the common jaeger spec (as in #1166) or add a a securityContext element to the Spec.Agent type.

In addition, a lot of headaches may be avoided by setting the the jaeger-agent container user to a non-root one by default. This should be realtively straight-forward as I don't think the jaeger-agent requires any privileges or disk access. I'd like to create a PR for this, but I can't find the jaeger-agent image's Dockerfile.

@ghost ghost added the needs-triage New issues, in need of classification label Sep 8, 2020
@jpkrohling
Copy link
Contributor

If there's a security context for individual containers in a pod, the Jaeger Agent container should indeed use the security context from the Jaeger's CR. The fix would be somewhere around here:

return corev1.Container{
Image: util.ImageName(jaeger.Spec.Agent.Image, "jaeger-agent-image"),
Name: "jaeger-agent",
Args: args,
Env: []corev1.EnvVar{
{
Name: envVarPodName,
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{
FieldPath: "metadata.name",
},
},
},
{
Name: envVarHostIP,
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{
FieldPath: "status.hostIP",
},
},
},
},
Ports: []corev1.ContainerPort{
{
ContainerPort: zkCompactTrft,
Name: "zk-compact-trft",
Protocol: corev1.ProtocolUDP,
},
{
ContainerPort: configRest,
Name: "config-rest",
},
{
ContainerPort: jgCompactTrft,
Name: "jg-compact-trft",
Protocol: corev1.ProtocolUDP,
},
{
ContainerPort: jgBinaryTrft,
Name: "jg-binary-trft",
Protocol: corev1.ProtocolUDP,
},
{
ContainerPort: adminPort,
Name: "admin-http",
},
},
Resources: commonSpec.Resources,
VolumeMounts: volumesAndMountsSpec.VolumeMounts,
}

And here's the Dockerfile for Jaeger Agent: https://github.com/jaegertracing/jaeger/blob/master/cmd/agent/Dockerfile

@jpkrohling jpkrohling added bug Something isn't working good first issue Good for newcomers hacktoberfest and removed needs-triage New issues, in need of classification labels Sep 9, 2020
@chgl
Copy link
Contributor Author

chgl commented Sep 9, 2020

Sounds good! I'll create a PR to add the securityContext and will also propose to update the Dockerfile.

@mergify mergify bot closed this as completed in #1190 Sep 28, 2020
mergify bot pushed a commit that referenced this issue Sep 28, 2020
Unfortunately, I can't just use the Jaeger CommonSpec's securityContext as that is a `PodSecurityContext` whereas the sidecar can only have a `SecurityContext`. This required me to add an additional config option `containerSecurityContext`. This does add some redundancy as the `Jaeger.Spec.SecurityContext` is not reused.

Is `sidecarContainerSecurityContext` a better name for this config option as the other agent deployments do use the default pod security context?

Resolves #1186
prageethw pushed a commit to prageethw/jaeger-operator that referenced this issue Oct 1, 2020
Unfortunately, I can't just use the Jaeger CommonSpec's securityContext as that is a `PodSecurityContext` whereas the sidecar can only have a `SecurityContext`. This required me to add an additional config option `containerSecurityContext`. This does add some redundancy as the `Jaeger.Spec.SecurityContext` is not reused.

Is `sidecarContainerSecurityContext` a better name for this config option as the other agent deployments do use the default pod security context?

Resolves jaegertracing#1186

Signed-off-by: Prageeth Warnak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants