Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #957

bashofmann · 2021-01-26T14:32:35Z

Add PodSecurityPolicies for rancher-istio, rancher-tracing and rancher-kiali-server.

Addresses: rancher/rancher#30744
Rebase of #930

haswalt · 2021-01-26T17:44:36Z

Just hit I'm assuming this issue on my test cluster. Deployed with RKE using the permissive PSP.

Blocks any sort of ISTIO deployment on a vaguely sensible set of defaults.

Would be great to see this available ASAP

aiyengar2 · 2021-02-08T22:45:46Z

@bashofmann I pointed your current PR back to dev-v2.5-source since we deprecated the -alt branches just now

aiyengar2

LGTM content-wise, but this needs to be rebased on the current Istio chart(s) + scripts.

@brendarearden can you help get this PR updated / merged in?

packages/rancher-istio/charts/Chart.yaml

packages/rancher-istio/charts/templates/istio-install-psp.yaml

bashofmann · 2021-02-09T12:17:19Z

I rebased the PR and added the requested newline

aiyengar2

LGTM, nit changes suggested

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml

packages/rancher-tracing/package.yaml

brendarearden

LGTM!! Thank you for doing this change!!

pennyscissors · 2021-02-26T18:27:53Z

@bashofmann @brendarearden @aiyengar2 Is this ready to merge after a rebase? I'm cleaning up this repo and just want to make sure we don't lose track of this.

aiyengar2 · 2021-02-27T05:34:38Z

@pennyscissors thanks for bumping this. Let’s hold off till 2.5.6 is released.

@bashofmann sorry to ask once more, but can you rebase this PR one more time? Since this should (presumably) be targeted for 2.5.7, we’ll need to bump all the packageVersions by 1 and reset all the releaseCandidateVersions of charts that are being modified to 00.

@brendarearden could you help merge this in as soon as we’re clear to merge 2.5.7 changes so we can avoid another rebase? We should probably also check with QA / management w.r.t. whether QA has bandwidth to test this issue for the next release.

haswalt · 2021-02-27T13:17:53Z

Can I asked whats the reason for waiting until 2.5.7? This is currently broken for production for everybody that runs PSPs enabled. Assuming 2.5.7 will likely be a way off yet

aiyengar2

LGTM w.r.t. the content of the PR, but I have some questions regarding Istio / Tracing / Kiali conventions. Will approve and leave those choices up to Brenda.

packages/rancher-istio/charts/templates/istio-cni-psp.yaml

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml

aiyengar2 · 2021-03-01T19:41:26Z

packages/rancher-tracing/package.yaml

@@ -1,3 +1,3 @@
 url: local
-packageVersion: 02
+packageVersion: 03


@brendarearden should we be bumping the packageVersion here or should we just modify the Chart.yaml version to release a new version and reset these to 00 / 00?

@cbron this might also be a question for you: for local charts, what should our convention be w.r.t. bumping packageVersion v.s. bumping the major.minor.patch version? For pushprox, I'm leaning towards never touching the packageVersion and only touching the major.minor.patch + releaseCandidate version, re: this comment.

aiyengar2 · 2021-03-01T19:50:22Z

@haswalt we need QA bandwidth to test the contents of this PR before release (outside of a developer review) to ensure that it fully addresses all test cases in the original issue

haswalt · 2021-03-01T21:46:24Z

@aiyengar2 understood thank you

brendarearden · 2021-03-17T22:08:38Z

I am about to split istio into 1.8 and 1.9 version and I am hoping we can merge this ASAP so that both version work in an airgap environment. @bashofmann how likely is it that we could get the nits fixed and then updated with the current branch by tomorrow?

bashofmann · 2021-03-17T22:17:48Z

Should be doable, I have a bit of time tomorrow morning.

bashofmann · 2021-03-18T15:25:27Z

I replaced the hardcoded names and namespace with the appropriate templates.

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml

brendarearden

The naming on the resources needs fixed, if you want to go ahead and fix the new line nit at the same time I think we will be good to go.

…-kiali-server Signed-off-by: Bastian Hofmann <[email protected]>

bashofmann · 2021-03-18T16:15:47Z

Sorry, I missed that. Done.

brendarearden

LGTM

aiyengar2

LGTM.

FYI @brendarearden I don't want to block this PR on nit changes, but we should eventually make changes to Istio / Kiali / Tracing to follow consistent Helm chart conventions.

Some changes I think we should consider:

Istio uses raw names (i.e. psp-istio-cni) across the chart, whereas tracing + kiali use helper template. I think we should move Istio towards using helper template based naming as well.
labels like app / heritage / release should also be in a helper template, since it's copy/pasted across several resources within tracing. Maybe we should also add those labels to Istio / Kiali

Let's get this merged! 🎉

nickgerace · 2021-03-19T17:16:05Z

packages/rancher-istio/charts/Chart.yaml

@@ -13,7 +13,7 @@ annotations:
  catalog.cattle.io/release-name: rancher-istio
  catalog.cattle.io/ui-component: istio
  catalog.cattle.io/provides-gvr: networking.istio.io.virtualservice/v1beta1
-  catalog.cattle.io/auto-install: rancher-kiali-server-crd=1.29.000-rc00
+  catalog.cattle.io/auto-install: rancher-kiali-server-crd=1.29.001-rc00


This is less for review and more for understanding: what does this annotation do?

rancher-istio has a dependency on rancher-kiali-server, which has a CRD chart. Since this means that rancher-istio needs to install rancher-kiali-server-crd automatically on the UI in order to have access, we need to add this annotation.

This type of dependency on another package's CRD chart is not very neatly handled by the charts-build-scripts today, which is why we need to manually add this annotation.

Added an issue at rancher/charts-build-scripts#28 to address this.

Gotcha - thanks for the echo issue. Both the manual approach and the proposed one in the issue make sense.

nickgerace · 2021-03-19T17:17:31Z

packages/rancher-istio/charts/templates/istio-cni-psp.yaml

+apiVersion: policy/v1beta1
+kind: PodSecurityPolicy
+metadata:
+  name: psp-istio-cni


Wondering if we should expose this in values.yaml
(and its references throughout the other file(s))

Do you mean exposing a field that allows you to enable or disable deploying PSPs? I kinda agree with this idea since I think this is what rancher-monitoring does too:

charts/charts/rancher-monitoring/rancher-monitoring/9.4.203/values.yaml

Line 330 in aae93d0

pspEnabled: true

I mean both the naming and that :)
I think it may be a good idea.

nickgerace · 2021-03-19T17:18:38Z

packages/rancher-istio/charts/templates/istio-cni-psp.yaml

+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: psp-istio-cni


Wondering if we should expose this in values.yaml
(and its references throughout the other file(s))

w.r.t. naming, I think we just need to introduce a helper template instead of hard-coding the name, re: my comment in #957 (review). Not worth blocking this PR for though.

What else would you want to expose in the values.yaml?

I'm hesitant to open up values.yaml fields unless absolutely necessary since it just adds more stuff we need to test / support per release.

Agreed on not blocking the PR for it, but the template may suffice. Agreed on the values.yaml though; just want to anticipate potential tech debt if naming is a bottleneck or a user wants to bring their own RBAC (unlikely)

packages/rancher-istio/charts/templates/istio-cni-psp.yaml

packages/rancher-istio/charts/templates/istio-install-psp.yaml

packages/rancher-istio/charts/templates/istio-psp.yaml

nickgerace · 2021-03-19T17:19:43Z

packages/rancher-istio/charts/templates/istio-psp.yaml

+  seLinux:
+      rule: RunAsAny


This should probably be overrideable in values.yaml

I'm not sure if we want to expose fields to configure the PSP used by Istio itself; is this something that other Helm charts expose today?

I'm moreso curious if we should expose SELinux values. This might change dependent on how the node is configured and the customer use case. However, this might be a paranoid concern

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml

packages/rancher-tracing/charts/templates/psp.yaml

bashofmann · 2021-05-03T11:41:05Z

Closing because PSPs have been merged with #1071

bashofmann mentioned this pull request Jan 26, 2021

Add PodSecurityPolicies for istio, tracing and kiali #930

Closed

bashofmann force-pushed the rancher-istio-psp-source-alt branch 2 times, most recently from 27f7c7f to e8c95c6 Compare January 26, 2021 14:48

aiyengar2 changed the base branch from dev-v2.5-source-alt to dev-v2.5-source February 8, 2021 22:45

aiyengar2 requested review from brendarearden and aiyengar2 February 8, 2021 22:45

aiyengar2 suggested changes Feb 8, 2021

View reviewed changes

packages/rancher-istio/charts/Chart.yaml Outdated Show resolved Hide resolved

packages/rancher-istio/charts/templates/istio-install-psp.yaml Outdated Show resolved Hide resolved

bashofmann force-pushed the rancher-istio-psp-source-alt branch from e8c95c6 to a7b5af6 Compare February 9, 2021 12:16

aiyengar2 previously approved these changes Feb 9, 2021

View reviewed changes

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml Outdated Show resolved Hide resolved

packages/rancher-tracing/package.yaml Outdated Show resolved Hide resolved

brendarearden previously approved these changes Feb 10, 2021

View reviewed changes

bashofmann dismissed stale reviews from brendarearden and aiyengar2 via aed1108 March 1, 2021 18:08

bashofmann force-pushed the rancher-istio-psp-source-alt branch 2 times, most recently from aed1108 to 3422f84 Compare March 1, 2021 18:11

aiyengar2 previously approved these changes Mar 1, 2021

View reviewed changes

bashofmann dismissed aiyengar2’s stale review via 1ddcaf6 March 18, 2021 14:30

bashofmann force-pushed the rancher-istio-psp-source-alt branch from 3422f84 to 1ddcaf6 Compare March 18, 2021 14:30

brendarearden reviewed Mar 18, 2021

View reviewed changes

packages/rancher-kiali-server/generated-changes/overlay/templates/psp.yaml Outdated Show resolved Hide resolved

brendarearden reviewed Mar 18, 2021

View reviewed changes

brendarearden requested a review from aiyengar2 March 18, 2021 16:12

Add PodSecurityPolicies for rancher-istio,rancher-tracing and rancher…

4096e77

…-kiali-server Signed-off-by: Bastian Hofmann <[email protected]>

bashofmann force-pushed the rancher-istio-psp-source-alt branch from 1ddcaf6 to 4096e77 Compare March 18, 2021 16:15

brendarearden approved these changes Mar 18, 2021

View reviewed changes

aiyengar2 approved these changes Mar 18, 2021

View reviewed changes

brendarearden requested review from cbron and nickgerace and removed request for cbron March 18, 2021 19:34

nickgerace reviewed Mar 19, 2021

View reviewed changes

aiyengar2 mentioned this pull request Mar 19, 2021

[DNM] [Source] Bump charts-build-scripts to v0.1.0 #1045

Closed

brendarearden mentioned this pull request Apr 1, 2021

Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #1071

Merged

bashofmann closed this May 3, 2021

bashofmann deleted the rancher-istio-psp-source-alt branch May 3, 2021 11:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #957

Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #957

bashofmann commented Jan 26, 2021

haswalt commented Jan 26, 2021

aiyengar2 commented Feb 8, 2021

aiyengar2 left a comment •

edited

Loading

bashofmann commented Feb 9, 2021

aiyengar2 left a comment

brendarearden left a comment

pennyscissors commented Feb 26, 2021

aiyengar2 commented Feb 27, 2021 •

edited

Loading

haswalt commented Feb 27, 2021

aiyengar2 left a comment

aiyengar2 Mar 1, 2021

aiyengar2 commented Mar 1, 2021

haswalt commented Mar 1, 2021

brendarearden commented Mar 17, 2021

bashofmann commented Mar 17, 2021

bashofmann commented Mar 18, 2021

brendarearden left a comment

bashofmann commented Mar 18, 2021

brendarearden left a comment

aiyengar2 left a comment •

edited

Loading

nickgerace Mar 19, 2021

aiyengar2 Mar 19, 2021

nickgerace Mar 19, 2021

nickgerace Mar 19, 2021

aiyengar2 Mar 19, 2021

nickgerace Mar 19, 2021

nickgerace Mar 19, 2021

aiyengar2 Mar 19, 2021

aiyengar2 Mar 19, 2021

nickgerace Mar 19, 2021

nickgerace Mar 19, 2021

aiyengar2 Mar 19, 2021

nickgerace Mar 19, 2021

bashofmann commented May 3, 2021

Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #957

Add PodSecurityPolicies for rancher-istio,rancher-tracing and kiali #957

Conversation

bashofmann commented Jan 26, 2021

haswalt commented Jan 26, 2021

aiyengar2 commented Feb 8, 2021

aiyengar2 left a comment • edited Loading

Choose a reason for hiding this comment

bashofmann commented Feb 9, 2021

aiyengar2 left a comment

Choose a reason for hiding this comment

brendarearden left a comment

Choose a reason for hiding this comment

pennyscissors commented Feb 26, 2021

aiyengar2 commented Feb 27, 2021 • edited Loading

haswalt commented Feb 27, 2021

aiyengar2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aiyengar2 commented Mar 1, 2021

haswalt commented Mar 1, 2021

brendarearden commented Mar 17, 2021

bashofmann commented Mar 17, 2021

bashofmann commented Mar 18, 2021

brendarearden left a comment

Choose a reason for hiding this comment

bashofmann commented Mar 18, 2021

brendarearden left a comment

Choose a reason for hiding this comment

aiyengar2 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bashofmann commented May 3, 2021

aiyengar2 left a comment •

edited

Loading

aiyengar2 commented Feb 27, 2021 •

edited

Loading

aiyengar2 left a comment •

edited

Loading