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.

Sign up for GitHub

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

Jump to bottom

chore: document minimal security context settings #1278

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: document minimal security context settings #1278

wants to merge 1 commit into from

Conversation

jawnsy
Copy link
Contributor

@jawnsy jawnsy commented Nov 24, 2022

Add documentation that describes how to use opentelemetry-operator in restrictive clusters, such as enabling runAsRoot, dropping capabilities, and configuring seccomp confinement.

Closes: #1264

@jawnsy jawnsy requested a review from a team November 24, 2022 19:23
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 24, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: jawnsy / name: Jonathan Yu (a39a5e0)

jawnsy
jawnsy commented Nov 24, 2022
View reviewed changes
README.md Outdated

You can optionally configure `runAsUser` and set it to `10001`, as this is the `USER` defined in the [opentelemetry-collector Dockerfile](https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/distributions/otelcol/Dockerfile). In OpenShift, however, configuring this explicitly will conflict with the default `restricted` Security Context Constraint, which runs pods with a project/namespace-specific User ID (UID).

For a full list of settings, consult the type definition in [opentelemetrycollector_types.go](./apis/v1alpha1/opentelemetrycollector_types.go) or [API docs](./docs/api.md).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line might not be necessary, since the API docs are mentioned at the top. Happy to remove it according to your preference!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we can remove this line

@jawnsy jawnsy marked this pull request as draft November 24, 2022 19:26
pavolloffay
pavolloffay reviewed Nov 26, 2022
View reviewed changes
Copy link
Member

@pavolloffay pavolloffay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR ❤️

README.md Outdated

You can optionally configure `runAsUser` and set it to `10001`, as this is the `USER` defined in the [opentelemetry-collector Dockerfile](https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/distributions/otelcol/Dockerfile). In OpenShift, however, configuring this explicitly will conflict with the default `restricted` Security Context Constraint, which runs pods with a project/namespace-specific User ID (UID).

For a full list of settings, consult the type definition in [opentelemetrycollector_types.go](./apis/v1alpha1/opentelemetrycollector_types.go) or [API docs](./docs/api.md).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we can remove this line

frzifus
frzifus reviewed Jan 31, 2023
View reviewed changes
Copy link
Member

@frzifus frzifus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May i ask, whats the current state here? :)

@jawnsy
Copy link
Contributor Author

jawnsy commented Jan 31, 2023

Ah, apologies, I need to clear up the CLA issue before I can proceed

@jawnsy jawnsy force-pushed the document-restricted-security-context branch from 588c80a to f5ecbe1 Compare May 27, 2023 18:09
@jawnsy jawnsy marked this pull request as ready for review May 27, 2023 18:14
@jawnsy
Copy link
Contributor Author

jawnsy commented May 27, 2023

Sorry for the long delay, and thanks for your patience. I've rebased and addressed comments (removed the reference to the API file), and this pull request is now ready for another review.

@TylerHelmuth TylerHelmuth added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 30, 2023
@R011y
Copy link

R011y commented Aug 24, 2023

Any update on this?

@jawnsy
chore: document minimal security context settings
a39a5e0 
Add documentation that describes how to use opentelemetry-operator
in restrictive clusters, such as enabling runAsRoot, dropping
capabilities, and configuring seccomp confinement.
@jawnsy jawnsy force-pushed the document-restricted-security-context branch from f5ecbe1 to a39a5e0 Compare August 24, 2023 23:28
@jawnsy
Copy link
Contributor Author

jawnsy commented Aug 24, 2023

@pavolloffay @frzifus Hey, I've rebased and this should be ready for review when you have a moment!

@frzifus
Copy link
Member

frzifus commented Aug 25, 2023

Awesome thanks @jawnsy

@R011y
Copy link

R011y commented Aug 25, 2023

Thanks guys. Greatly appreciated.

@R011y
Copy link

R011y commented Aug 28, 2023

@jawnsy Still need one more for this to get merged <3

@jawnsy
Copy link
Contributor Author

jawnsy commented Aug 28, 2023

Still need one more for this to get merged <3

@R011y Sorry, can you please clarify -- what are the next steps here? Do I need to make more changes, or are we waiting for another review?

@R011y
Copy link

R011y commented Aug 28, 2023

Still need one more for this to get merged <3

@R011y Sorry, can you please clarify -- what are the next steps here? Do I need to make more changes, or are we waiting for another review?

Just needs another review. No changes requested.

@R011y
Copy link

R011y commented Aug 29, 2023

Just for awareness in case others come across this issue:
Otel operator CAN be run with restrictive security context without issue. It functions correctly with both allowPrivilegeEscalation: false and readOnlyRootFilesystem: true. The documentation update in this PR will just cement the existing ability to run Otel operator with restrictive security context. That said, it's always best to test thoroughly in your environment as there may be conditions or variability that present issues not identified here.

@R011y
Copy link

R011y commented Aug 29, 2023

Add me as a reviewer if you want @jawnsy, if that's possible.

@jaronoff97
Copy link
Contributor

@R011y mind reviewing this so i can merge this finally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frzifus frzifus frzifus approved these changes

@pavolloffay pavolloffay Awaiting requested review from pavolloffay

Assignees
No one assigned
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation for restricted securityContext
6 participants
@jawnsy @R011y @frzifus @jaronoff97 @pavolloffay @TylerHelmuth