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

helm: set hubble-ui securityContext #11475

Merged
merged 1 commit into from May 13, 2020
Merged

helm: set hubble-ui securityContext #11475

merged 1 commit into from May 13, 2020

Conversation

alex1989hu
Copy link
Contributor

@alex1989hu alex1989hu commented May 11, 2020

The image hubble-ui no longer needs run as root user

Special notes for your reviewer:
Do not merge it until new hubble-ui image is released, see corresponding pull request: cilium/hubble-ui#31. Tested on k8s (v1.18) with enforced restricted PSP.

UPDATE:
Set it as optional to not to break hubble-ui <= 0.5.0 deployments.

Signed-off-by: Alex Szakaly alex.szakaly@gmail.com

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes:
N/A

@alex1989hu alex1989hu requested a review from a team as a code owner May 11, 2020 17:32
@alex1989hu alex1989hu requested a review from a team May 11, 2020 17:32
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@alex1989hu
Copy link
Contributor Author

/cc @gandro - I don't know what is wrong with the label: 1st: filled release not 2nd: removed section => same result.

@gandro gandro added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label May 11, 2020
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR! As discussed on Slack, I believe the securityContext should be opt-in (via Helm values), to allow the chart to function with the existing v0.5 hubble-ui image.

I propose we add the following to the values.yaml of hubble-ui.

securityContext:
  enabled: false
  runAsUser: 1001

Once we have a new release of the hubble-ui container and we bump the default image tag, we can then update the securityContext.enabled=true by default to make it opt-out.

The image hubble-ui no longer needs run as root user

Signed-off-by: Alex Szakaly <alex.szakaly@gmail.com>
@coveralls
Copy link

coveralls commented May 11, 2020

Coverage Status

Coverage increased (+0.01%) to 37.887% when pulling 5a55b66 on alex1989hu:helm/hubble-ui-security-context into e24ab5b on cilium:master.

@gandro
Copy link
Member

gandro commented May 11, 2020

test-me-please

@gandro gandro added the area/helm Impacts helm charts and user deployment experience label May 11, 2020
@gandro
Copy link
Member

gandro commented May 12, 2020

test-me-please

@gandro
Copy link
Member

gandro commented May 12, 2020

retest-net-next

@gandro gandro merged commit 05b3518 into cilium:master May 13, 2020
1.8.0 automation moved this from In progress to Merged May 13, 2020
@alex1989hu alex1989hu deleted the helm/hubble-ui-security-context branch May 29, 2020 16:36
@alex1989hu
Copy link
Contributor Author

Thanks a lot for the PR! As discussed on Slack, I believe the securityContext should be opt-in (via Helm values), to allow the chart to function with the existing v0.5 hubble-ui image.

I propose we add the following to the values.yaml of hubble-ui.

securityContext:
  enabled: false
  runAsUser: 1001

Once we have a new release of the hubble-ui container and we bump the default image tag, we can then update the securityContext.enabled=true by default to make it opt-out.

FYI: new hubble-ui has ben released: https://github.com/cilium/hubble-ui/releases/tag/v0.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Impacts helm charts and user deployment experience release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants