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

Refactor the target allocator build to not run it as root #1345

Merged
merged 7 commits into from
Jan 9, 2023

Conversation

iblancasa
Copy link
Contributor

@iblancasa iblancasa commented Jan 3, 2023

Signed-off-by: Israel Blancas [email protected]

Fixes #1346

This change makes the target allocator being run as non root. The Dockerfile follows the same structure than the one from the OTEL Collector Operator.

Steps performed to ensure the issue is fixed

  1. Create an OCP cluster (version 4.11 in my case)
  2. Build and push to a registry the OTEL Collector Operator and Target Allocator container images
  3. $ kubectl create namespace oteltest
  4. $ kubectl -n oteltest create rolebinding default-view-oteltest --role=pod-view --serviceaccount=oteltest:ta
  5. $ kubectl create -f tests/e2e/targetallocator-features/00-install.yaml -n oteltest:ta
  6. Check that the pods are created properly ($ kubectl get pods -n oteltest)

@iblancasa iblancasa requested a review from a team January 3, 2023 16:48
Signed-off-by: Israel Blancas <[email protected]>
@iblancasa iblancasa requested a review from a team January 3, 2023 16:48
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

How was this tested?

.chloggen/1345-run-ta-nonroot.yaml Outdated Show resolved Hide resolved
Signed-off-by: Israel Blancas <[email protected]>
@iblancasa
Copy link
Contributor Author

iblancasa commented Jan 4, 2023

@jaronoff97 thanks for your review!

I created #1346 to track this issue. There, you can see how I found the issue and how I checked everything was working properly after applying the changes from this PR.

Please, let me know if further changes need to be done :)

@iblancasa iblancasa closed this Jan 4, 2023
@iblancasa iblancasa reopened this Jan 4, 2023
@iblancasa iblancasa requested a review from jaronoff97 January 4, 2023 09:08
@jaronoff97
Copy link
Contributor

Thanks for making the issue! Could you add how you tested this to the description of the PR?

@iblancasa
Copy link
Contributor Author

@jaronoff97 done

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.

Otherwise lgtm :) Thanks @iblancasa

cmd/otel-allocator/Dockerfile Outdated Show resolved Hide resolved
@pavolloffay pavolloffay merged commit e1cecbf into open-telemetry:main Jan 9, 2023
@iblancasa iblancasa deleted the run-ta-nonroot branch January 10, 2023 09:36
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…etry#1345)

* Refactor the target allocator build to not run it as root

Signed-off-by: Israel Blancas <[email protected]>

* Add missing changelog

Signed-off-by: Israel Blancas <[email protected]>

* Fix issue number in changelog

Signed-off-by: Israel Blancas <[email protected]>

* Trigger Build

Signed-off-by: Israel Blancas <[email protected]>

* Use scratch image as base image for the target allocator

Signed-off-by: Israel Blancas <[email protected]>

Signed-off-by: Israel Blancas <[email protected]>
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.

[target allocator] Not run the target allocator as root
5 participants