Skip to content

Conversation

@willdollman
Copy link
Contributor

@willdollman willdollman commented Aug 26, 2024

Run the otel-collector and otel-agent containers as the sourcegraph user rather than root.

One of our customers is asking us to ensure that containers don't run as root.

I haven't been able to identify the original reason why these containers were running as root, so it seems that this was simply an oversight when the containers were originally created. This PR drops the runAs user + group IDs to sourcegraph:sourcegraph.

otel-collector seems to be broken currently, due to incorrect configuration with jaeger - see #542 + linked slack discussion for a fix.

Reviewer notes

Testing locally requires setting jaeger.enabled: true in values.yaml, and applying the fix linked above to ensure telemetry is sent to jaeger.

I've followed the checklist below as best as I can - would appreciate some input on whether this requires any more work on my part.

Checklist

  • Follow the manual testing process
    • Linted, manually tested, and checked rendered output
    • Running helm unittest --helm3 ./charts/sourcegraph/. gives error unknown flag: --helm3
    • Unit tests fail even on main - are they expected to pass?
  • Update changelog
  • Update Kubernetes update doc
    • I don't think this change requires an entry

Test plan

  • Created helm deployment locally with jaeger + opentelemetry enabled, and ran with otel-collector & otel-agent running first as root and then sourcegraph users. No difference in tracing behaviour was observed.
  • Reviewed containers and configuration for any potential issues, such as file permissions, relating to change of user. Our opentelemetry containers don't store any state, and all binary + config files are read/executable as needed by non-root users.

@willdollman willdollman self-assigned this Aug 26, 2024
@willdollman willdollman requested review from a team and Chickensoupwithrice and removed request for a team August 26, 2024 14:52
@willdollman willdollman marked this pull request as ready for review August 26, 2024 14:53
@willdollman willdollman enabled auto-merge (squash) August 27, 2024 09:42
@willdollman willdollman disabled auto-merge August 27, 2024 11:36
@willdollman willdollman merged commit 788d32f into main Aug 28, 2024
@willdollman willdollman deleted the will/otel-remove-root branch August 28, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants