-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 support for psp #4383
Add support for psp #4383
Conversation
Welcome @otnielvh! |
Hi @otnielvh. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@otnielvh thank you for your contribution. Please move the doc and yaml files to the example section. |
Also, it would be great if you can add an e2e test using PSP to avoid regressions in the future. |
@aledbf, thank you for your feedback. I moved the files into the example section, however I'm not sure how to address e2e testing. AFAIK enabling PSP requires changes in the cluster that are not supported via kubectl (I'm running in GKE and use gcloud). From a brief look the other tests don't require such fundamental changes. Could you please point me in the direction of how to write a test that changes the cluster not through kubectl or client api? If you can recall any examples that would be a great help. Thank you again for you time. |
@otnielvh I forgot I already wrote one 🤦♂️ https://github.com/kubernetes/ingress-nginx/blob/master/test/e2e/settings/pod_security_policy.go |
/ok-to-test |
Also, please add a link to this new example in this section https://github.com/kubernetes/ingress-nginx/blob/master/mkdocs.yml#L66 |
Codecov Report
@@ Coverage Diff @@
## master #4383 +/- ##
========================================
Coverage ? 58.5%
========================================
Files ? 87
Lines ? 6528
Branches ? 0
========================================
Hits ? 3819
Misses ? 2281
Partials ? 428 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I was also thinking, maybe it can be added to https://github.com/kubernetes/ingress-nginx/blob/master/docs/deploy/index.md for user's to view before deploying.
9724275
to
3b34d56
Compare
Squashed the commits and added the links mentioned. I still have two more points:
|
/retest |
/test pull-ingress-nginx-test |
/lgtm |
@otnielvh thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, otnielvh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR adds the yaml file needed for running nginx when port security policy (PSP) is enabled on the Kubernetes cluster. Me and my colleagues struggled with it somewhat, and felt it would be nice to have it together with the rest of nginx.