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

Add permissions to read Job owner #1066

Closed
wants to merge 2 commits into from

Conversation

janario
Copy link
Contributor

@janario janario commented Mar 6, 2024

Together with open-telemetry/opentelemetry-operator#2716

In order to be able to collect the owner of a Job the manager would need to have the proper permissions.

In this PR we add the read Job permissions so in future release of Operator we can improve to get the Job owner.

@janario janario requested a review from Allex1 as a code owner March 6, 2024 08:25
@janario janario requested a review from a team March 6, 2024 08:25
@Allex1
Copy link
Contributor

Allex1 commented Mar 11, 2024

Thanks for your contribution @janario but I think you would need to have this change merged first in https://github.com/open-telemetry/opentelemetry-operator/blob/main/config/rbac/role.yaml

@janario janario changed the title Add permissions to read Job owner Draft: Add permissions to read Job owner Mar 23, 2024
@janario janario closed this Apr 3, 2024
@janario janario force-pushed the feature/job-permissions branch from 93fdc57 to 0d56957 Compare April 3, 2024 09:51
@janario janario reopened this Apr 3, 2024
@janario
Copy link
Contributor Author

janario commented Apr 3, 2024

Back on this one.

@Allex1 now that the operator is merged open-telemetry/opentelemetry-operator#2716

Doesn't the rbac get updated by some process while updating the operator or is it handled manually?

@Allex1
Copy link
Contributor

Allex1 commented Apr 3, 2024

@janario we currently update the role manually from the operator repo . Can you please bump the chart version and run make generate-examples CHARTS=opentelemetry-operator and make update-operator-crds

@janario janario changed the title Draft: Add permissions to read Job owner Add permissions to read Job owner Apr 3, 2024
@janario
Copy link
Contributor Author

janario commented Apr 3, 2024

@janario we currently update the role manually from the operator repo . Can you please bump the chart version and run make generate-examples CHARTS=opentelemetry-operator and make update-operator-crds

done ✅

Copy link
Contributor

@Allex1 Allex1 left a comment

Choose a reason for hiding this comment

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

make sure you rebase

@janario janario force-pushed the feature/job-permissions branch from 5fb29f2 to 0f1de87 Compare April 3, 2024 18:49
@janario
Copy link
Contributor Author

janario commented Apr 3, 2024

done 👍

@TylerHelmuth
Copy link
Member

@janario I believe this RBAC change is only relevent on the newly released 0.97.0 version of the collector, is that correct? If so, @jaronoff97 you'll need these changes in #1113

@janario
Copy link
Contributor Author

janario commented Apr 3, 2024

@janario I believe this RBAC change is only relevent on the newly released 0.97.0 version of the collector, is that correct? If so, @jaronoff97 you'll need these changes in #1113

yes, correct

Fine for me to go in the other PR too,
it is just these few lines https://github.com/open-telemetry/opentelemetry-helm-charts/pull/1066/files#diff-bd2633cf0804f0badd4644cb029eaad518031ade4cd79143ec943860e9cf11d9R81-R88

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Apr 3, 2024

superseded by #1113. Thank you @janario for staying on top of this!

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.

3 participants