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

Java Instrumentation with runAsNonRoot #1058

Closed
Yamakaky opened this issue Aug 25, 2022 · 9 comments · Fixed by #1273
Closed

Java Instrumentation with runAsNonRoot #1058

Yamakaky opened this issue Aug 25, 2022 · 9 comments · Fixed by #1273
Labels
area:auto-instrumentation Issues for auto-instrumentation discuss-at-sig This issue or PR should be discussed at the next SIG meeting

Comments

@Yamakaky
Copy link

I added instrumentation.opentelemetry.io/inject-java: "my-instrumentation" to a pod with runAsNonRoot enabled. I get this error: Error: container has runAsNonRoot and image will run as root (pod: "mypod-58f5d96bbc-tq77x_ns(2eb77116-d336-41ec-a064-eea6b6be10c0)", container: opentelemetry-auto-instrumentation). Removing runAsNonRoot fixes the issue, but that's not a proper fix. I guess the injected container should copy the securityContext from the main parent?

@pavolloffay pavolloffay added the area:auto-instrumentation Issues for auto-instrumentation label Aug 30, 2022
@pavolloffay
Copy link
Member

@Yamakaky thanks for reporting the issue.

I guess the injected container should copy the securityContext from the main parent?

I am not sure if I understand. The auto-instrumentation uses init-container to copy the auto-instrumentation binaries into the app container. the securityContext is defined on the pod level, not on the individual containers in the pod. Could you please be more specific how it should be configured?

@Yamakaky
Copy link
Author

Yamakaky commented Aug 31, 2022

Hum, I'm not completely sure how securityContext works with sidecars. I think the issue is that the init container doesn't define a user id to run as, so the pod configuration runAsNonRoot kicks in and prevents the launch.

Here is how vault-agent does it, it works fine with runAsNonRoot. a.SetSecurityContext is true by default, search agent-set-security-context at https://github.com/hashicorp/vault/blob/19fa7ea0aeec3e9bbca33cbe96f792151ae2de17/website/content/docs/platform/k8s/injector/annotations.mdx#L164.

https://github.com/hashicorp/vault-k8s/blob/bd9da8fbf249f2b6dce0da6b60f3b6820bfd248e/agent-inject/agent/container_init_sidecar.go#L90

https://github.com/hashicorp/vault-k8s/blob/bd9da8fbf249f2b6dce0da6b60f3b6820bfd248e/agent-inject/agent/container_sidecar.go#L187

@pavolloffay
Copy link
Member

Actually the security context is defined in pod level but as well on container level

The container level overrides the pod level. Could you please share your pod spec where the instrumentation was injected?

@Yamakaky
Copy link
Author

Yamakaky commented Aug 31, 2022

https://gist.github.com/Yamakaky/43580abf273d2214feaa17d6a4d21057

I removed some unrelated parts like like resource limits. Note that the docker image is defined with a fixed user id, otherwise I would have to configure securityContext at the container level.

@pavolloffay
Copy link
Member

The security context is defined on the pod level https://gist.github.com/Yamakaky/43580abf273d2214feaa17d6a4d21057#file-deployment-yml-L67 therefore I would say all containers would inherit from it.

@Yamakaky
Copy link
Author

Yes, but the init container will still try to run as root since a userid is not defined.

@vickas522
Copy link

Yes, but the init container will still try to run as root since a userid is not defined.

@Yamakaky , anyways to run opentelemetry auto instrumentor init container as nonroot user ? as since we want the main container to run with nonroot user.

@Itchimonji
Copy link

Itchimonji commented Jan 15, 2024

I also need a configuration for the security context of the init-container (kind: instrumentation) because I want to add labels to the namespace to fullfil the restricted mode of the PSA (https://kubernetes.io/docs/concepts/security/pod-security-admission/).

For this I need to add at least this:

  securityContext:
    runAsUser: 1005
    runAsNonRoot: true
    allowPrivilegeEscalation: false
    seccompProfile:
      type: RuntimeDefault
    capabilities:
      drop:
        - ALL

It is already possible for the OpenTelemetryCollector:

apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: sidecar
spec:
  mode: sidecar
  securityContext:
    runAsUser: 1005
    runAsNonRoot: true
    allowPrivilegeEscalation: false
    seccompProfile:
      type: RuntimeDefault
    capabilities:
      drop:
        - ALL

@matthenry87
Copy link

I also need a configuration for the security context of the init-container (kind: instrumentation) because I want to add labels to the namespace to fullfil the restricted mode of the PSA (https://kubernetes.io/docs/concepts/security/pod-security-admission/).

For this I need to add at least this:

  securityContext:
    runAsUser: 1005
    runAsNonRoot: true
    allowPrivilegeEscalation: false
    seccompProfile:
      type: RuntimeDefault
    capabilities:
      drop:
        - ALL

It is already possible for the OpenTelemetryCollector:

apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: sidecar
spec:
  mode: sidecar
  securityContext:
    runAsUser: 1005
    runAsNonRoot: true
    allowPrivilegeEscalation: false
    seccompProfile:
      type: RuntimeDefault
    capabilities:
      drop:
        - ALL

I also need to set the securityContext on the init container. Did you ever figure this out?

@jaronoff97 jaronoff97 added the discuss-at-sig This issue or PR should be discussed at the next SIG meeting label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:auto-instrumentation Issues for auto-instrumentation discuss-at-sig This issue or PR should be discussed at the next SIG meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants