Add pod security policies for prometheus components #15055

wfernandes · 2019-06-25T19:54:33Z

What this PR does / why we need it:

This PR adds basic Pod Security Policies for the promethues components. This will allow this chart to be installed in an environment that has the PSP admission controller enabled.

Interestingly, there was a PSP included just for the node-exporter job. We just mimicked that PSP and added some for the other jobs.

Special notes for your reviewer:

This is a duplicate of #12825 which as already been approved. Unfortunately, it got stale and the bot closed the PR.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the README.md

k8s-ci-robot · 2019-06-25T19:54:46Z

Hi @wfernandes. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

wfernandes · 2019-06-25T19:56:30Z

@gianrubio @mgoodness As mentioned in the description, this PR is a duplicate of #12825 which was approved but it just had a merge conflict which I believe has been resolved.

wfernandes · 2019-07-02T17:54:49Z

/ok-to-test

k8s-ci-robot · 2019-07-02T17:55:16Z

@wfernandes: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

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.

wfernandes · 2019-07-17T00:56:50Z

@gianrubio @mgoodness As mentioned in the description, this PR is a duplicate of #12825 which was approved but it just had a merge conflict which I believe has been resolved.

paulczar · 2019-08-15T19:27:40Z

stable/prometheus/values.yaml

@@ -1,5 +1,7 @@
 rbac:
  create: true
+  podSecurityPolicy:


I'm not sure enabling PSP is an RBAC related thing, so it probably doesn't belong inside the rbac structure here?

paulczar · 2019-08-15T19:28:11Z

stable/prometheus/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
 name: prometheus
-version: 8.14.3
+version: 8.14.4


It's adding a new feature, so i would bump version to 8.15.0

paulczar · 2019-08-15T19:28:28Z

/ok-to-test

Benjamintf1 · 2019-08-15T21:43:07Z

We pulled PSP enablement outside of rbac, and upped to 9.0.0 (because moving psp property from under node-exporter to a global property is a breaking change).

paulczar · 2019-08-16T18:34:30Z

stable/prometheus/templates/node-exporter-role.yaml

@@ -1,5 +1,5 @@
 {{- if and .Values.nodeExporter.enabled .Values.rbac.create }}
-{{- if .Values.nodeExporter.podSecurityPolicy.enabled }}
+{{- if .Values.podSecurityPolicy.enabled }}


what if we did {{- if or (default .Values.nodeExporter.podSecurityPolicy.enabled false) (.Values.podSecurityPolicy.enabled) }} for these checks, and then put a deprecation message in the NOTES.txt ?

that way its not yet a breaking change, but is also preparing people for deprecation of the old value ?

or if we want the breaking change we could error if .Values.nodeExporter.podSecurityPolicy.enabled is set with a error message with upgrade instructions.

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

paulczar · 2019-08-17T16:51:09Z

/lgtm

k8s-ci-robot · 2019-08-17T16:51:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: paulczar, wfernandes

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:

~~stable/prometheus/OWNERS~~ [paulczar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io> Signed-off-by: Roland Gritzer <gritzer.roland@gmail.com>

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

nico-ulbricht · 2019-09-19T16:15:55Z

This PR seems to have introduced a bug whereas the following error occurs if rbac.enabled is true but podSecurity.enabled is false:

Error: rpc error: code = Unknown desc = validation failed: error validating "": error validating data: ValidationError(ClusterRole): missing required field "rules" in io.k8s.api.rbac.v1beta1.ClusterRole

Which seems to be caused by creating a ClusterRole without rules here: https://github.com/helm/charts/pull/15055/files#diff-055e5eaaea98daeb77e0619ee5effa11R9

Benjamintf1 · 2019-09-19T16:18:46Z

@nico-ulbricht ahh, that makes sense. It should probubly encompase the entire clusterrole. (at least, until another rule is added).

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Jun 25, 2019

k8s-ci-robot requested review from gianrubio and mgoodness June 25, 2019 19:54

helm-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 25, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 25, 2019

paulczar suggested changes Aug 15, 2019

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 15, 2019

paulczar self-assigned this Aug 15, 2019

Benjamintf1 force-pushed the prometheus-psps branch 2 times, most recently from d455386 to 92db526 Compare August 15, 2019 20:14

paulczar reviewed Aug 16, 2019

View reviewed changes

dtimm force-pushed the prometheus-psps branch 2 times, most recently from ab6f016 to 69b34f1 Compare August 16, 2019 22:01

Update psps to support all pods

f5fd181

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

dtimm force-pushed the prometheus-psps branch from 69b34f1 to f5fd181 Compare August 16, 2019 22:04

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2019

k8s-ci-robot merged commit af1e617 into helm:master Aug 17, 2019

ThoTischner pushed a commit to bitsbeats/charts that referenced this pull request Aug 19, 2019

Update psps to support all pods (helm#15055)

09b5de0

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

kengou pushed a commit to kengou/charts that referenced this pull request Sep 18, 2019

Update psps to support all pods (helm#15055)

a55acc1

- deprecate nodeExporter.podSecurityPolicy.enabled Signed-off-by: David Timm <dtimm@pivotal.io> Co-authored-by: David Timm <dtimm@pivotal.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pod security policies for prometheus components #15055

Add pod security policies for prometheus components #15055

wfernandes commented Jun 25, 2019 •

edited

k8s-ci-robot commented Jun 25, 2019

wfernandes commented Jun 25, 2019

wfernandes commented Jul 2, 2019

k8s-ci-robot commented Jul 2, 2019

wfernandes commented Jul 17, 2019

paulczar Aug 15, 2019

paulczar Aug 15, 2019

paulczar commented Aug 15, 2019

Benjamintf1 commented Aug 15, 2019

paulczar Aug 16, 2019

paulczar Aug 16, 2019

Benjamintf1 Aug 16, 2019

paulczar commented Aug 17, 2019

k8s-ci-robot commented Aug 17, 2019

nico-ulbricht commented Sep 19, 2019

Benjamintf1 commented Sep 19, 2019 •

edited

Add pod security policies for prometheus components #15055

Add pod security policies for prometheus components #15055

Conversation

wfernandes commented Jun 25, 2019 • edited

What this PR does / why we need it:

Special notes for your reviewer:

Checklist

k8s-ci-robot commented Jun 25, 2019

wfernandes commented Jun 25, 2019

wfernandes commented Jul 2, 2019

k8s-ci-robot commented Jul 2, 2019

wfernandes commented Jul 17, 2019

paulczar Aug 15, 2019

Choose a reason for hiding this comment

paulczar Aug 15, 2019

Choose a reason for hiding this comment

paulczar commented Aug 15, 2019

Benjamintf1 commented Aug 15, 2019

paulczar Aug 16, 2019

Choose a reason for hiding this comment

paulczar Aug 16, 2019

Choose a reason for hiding this comment

Benjamintf1 Aug 16, 2019

Choose a reason for hiding this comment

paulczar commented Aug 17, 2019

k8s-ci-robot commented Aug 17, 2019

nico-ulbricht commented Sep 19, 2019

Benjamintf1 commented Sep 19, 2019 • edited

wfernandes commented Jun 25, 2019 •

edited

Benjamintf1 commented Sep 19, 2019 •

edited