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

[jaeger] Add security context to deployment charts in jaeger #37

Merged
merged 2 commits into from
Jan 25, 2020

Conversation

mehta-ankit
Copy link
Member

Signed-off-by: Ankit Mehta [email protected]

When deploying this chart in our cluster we found that we had to add security context to the deployments for them to run as non-root user.
So i created this PR where securityContext is added to the template/charts for deployment and its default value is provided in values.yaml file. These can be overwritten if someone uses jaeger chart as a dependency in their own values.yaml file.

@mehta-ankit
Copy link
Member Author

mehta-ankit commented Jan 22, 2020

@naseemkullah @pavelnikolov @dvonthenen Can i get a review on this please.

charts/jaeger/values.yaml Outdated Show resolved Hide resolved
@naseemkullah naseemkullah changed the title Add security context to deployment charts in jaeger [jaeger] Add security context to deployment charts in jaeger Jan 23, 2020
@mehta-ankit mehta-ankit force-pushed the addSecurityContext branch 2 times, most recently from 769ea70 to 863953d Compare January 24, 2020 15:00
@naseemkullah
Copy link
Member

Please gpg sign all you commits @mehta-ankit

charts/jaeger/values.yaml Outdated Show resolved Hide resolved
@mehta-ankit mehta-ankit force-pushed the addSecurityContext branch 3 times, most recently from 4fe79ac to 9091bff Compare January 25, 2020 14:27
@mehta-ankit
Copy link
Member Author

@naseemkullah I rebased to resolve conflicts and updated the version to 0.19.1.
Can you approve and merge please. 🙏 👍

@naseemkullah
Copy link
Member

naseemkullah commented Jan 25, 2020

Sure but we added a new component "ingester" could you add values for it please?

@mehta-ankit
Copy link
Member Author

mehta-ankit commented Jan 25, 2020

Sure but we added a new component "ingester" could you add values for it please?

Sure. Added it to the ingester. Should be ready now.

@naseemkullah
Copy link
Member

Great, thanks!

@naseemkullah naseemkullah merged commit b836ad8 into jaegertracing:master Jan 25, 2020
@mehta-ankit mehta-ankit deleted the addSecurityContext branch March 4, 2021 15:35
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