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: add agent and operator hostPorts to PodSecurityPolicy #12175

Closed
wants to merge 1 commit into from
Closed

helm: add agent and operator hostPorts to PodSecurityPolicy #12175

wants to merge 1 commit into from

Conversation

cfrantsen
Copy link

@cfrantsen cfrantsen commented Jun 18, 2020

When config options that open hostPorts are enabled the PodeSecurityPolicy needs to reflect this or it will not be selected.

Fixes #12324

@cfrantsen cfrantsen requested review from a team as code owners June 18, 2020 10:15
@cfrantsen cfrantsen requested a review from a team June 18, 2020 10:15
@maintainer-s-little-helper
Copy link

Commit 0482555d900560f6bcca5d57f625ead2bcd6c20b does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 18, 2020
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

When config options that open hostPorts are enabled the
PodeSecurityPolicy needs to reflect this or it will not be selected.

Signed-off-by: Christian Frantsen <christian.frantsen@dom.se>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 18, 2020
@gandro gandro added the release-note/misc This PR makes changes that have no direct user impact. label Jun 18, 2020
@gandro gandro added area/helm Impacts helm charts and user deployment experience dont-merge/needs-release-note labels Jun 18, 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 for the PR! I wonder if these hostPorts are also required for the Cilium health checks on port 4240?

https://docs.cilium.io/en/v1.7/install/system_requirements/#firewall-rules

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

#10681 is also editing the PSP but we can't merge it until #11140 is fixed.

I understand that this PR is actually fixing a potential issue with the PSP but we can't test it in our CI. @cfrantsen do you have a way to replicate the issue this PR is fixing?

@cfrantsen
Copy link
Author

@aanm It should be possible to reproduce by enabling the options that add hostPorts to the pod spec and then deploying to a cluster with PSP enabled. In addition to this there must not exist RBAC that allows the use of any other PSP except cilium-psp/cilium-operator-psp.

@cfrantsen
Copy link
Author

@gandro I could not find any other place where hostPort was defined in the chart so I don't think the helath checks are affected by this.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 37.084% when pulling 149d17c on cfrantsen:psp into d28767b on cilium:master.

@aanm
Copy link
Member

aanm commented Jul 9, 2020

@cfrantsen I'm going to close this PR. PodSecurityPolicy will be removed from Kubernetes kubernetes/kubernetes#90603

@aanm aanm closed this Jul 9, 2020
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/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
In progress
Development

Successfully merging this pull request may close these issues.

PodSecurityPolicy does not allow hostPort for Prometheus
4 participants