Issue-468: Run pravega segmentstore sts with runAsUser = 0 #469

Prabhaker24 · 2020-10-20T19:33:27Z

Signed-off-by: prabhaker24 [email protected]

Change log description

Pravega with ECS as LTS is failing to deploy in Openshift environment, as container needs to inject cert bundle into trust store, but the security context selected by openshift is preventing it.
To solve the above problem we have to set runAsUser = 0 for pravega segmentstore sts.

Purpose of the change

Fixes #468

What the code does

These changes allow the segmentstore pod's to run as root user. For enabling this user has to give the following things in the manifest file while deploying it manually

 pravega:
   segmenStoreSecurityContext: 
    runAsUser: 0

If he is using charts he needs to uncomment the line in the values.yaml in the pravega charts:-

segmentStoreSecurityContext: 
  runAsUser: 0

For controller he needs to set like this:-

controllerSecurityContext:
   runAsUser: 0

How to verify it

Deploy the pravega cluster with runAsUser: 0 and check the segment store sts and controller deployment post-deployment it should have the following in it's described output

      securityContext:
        runAsUser: 0

Signed-off-by: prabhaker24 <[email protected]>

sarlaccpit · 2020-10-20T20:52:28Z

pkg/apis/pravega/v1beta1/pravega.go

@@ -167,6 +168,9 @@ type PravegaSpec struct {

 	// SegmentStoreExternalTrafficPolicy defines the ExternalTrafficPolicy it can have cluster or local
 	SegmentStoreExternalTrafficPolicy string `json:"segmentStoreExternalTrafficPolicy,omitempty"`
+
+	// SecurityContext holds security configuration that will be applied to a container
+	SecurityContext *corev1.PodSecurityContext `json:"securityContext,omitempty"`


The PodSecurityContext is getting applied to the segment store only in this PR. And indeed the controller pods don't necessarily need the same PSC. (in particular, SegmentStores pod have more requirements in terms of requiring access to NFS for example, versus controllers which require less).

While the changes look correct, perhaps calling out that this is only getting applied to the SS, (renaming it accordingly?) , or also adding a separate one for the controller pods as well, might help avoid confusion?

I have changed the name and added securitycontext for controller as well.

gaddas3 · 2020-10-20T23:57:19Z

deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml

@@ -392,6 +392,128 @@ spec:
                      to the Pravega processes as JAVA_OPTS. See the following file
                      for a complete list of options: https://github.com/pravega/pravega/blob/master/config/config.properties'
                    type: object
+                  securityContext:


Is this from https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#securitycontext-v1-core?

yes, it's from this only.

…e ss security context Signed-off-by: prabhaker24 <[email protected]>

Signed-off-by: prabhaker24 <[email protected]>

anishakj · 2020-10-21T09:16:55Z

charts/pravega/README.md

@@ -99,3 +99,5 @@ The following table lists the configurable parameters of the pravega chart and t
 | `storage.cache.className` | Storage class for cache volume | `` |
 | `storage.cache.size` | Storage requests for cache volume | `20Gi` |
 | `options` | List of Pravega options | |
+| `segmentStoreSecurityContext.runAsUser` | The UID to run the entrypoint of the container process | `0` |


Can we mention that by default we are not setting any value for security context.

Signed-off-by: prabhaker24 <[email protected]>

codecov-io · 2020-10-21T10:11:53Z

Codecov Report

Merging #469 into master will decrease coverage by 2.71%.
The diff coverage is 20.24%.

@@            Coverage Diff             @@
##           master     #469      +/-   ##
==========================================
- Coverage   75.29%   72.58%   -2.72%     
==========================================
  Files          15       15              
  Lines        3068     3297     +229     
==========================================
+ Hits         2310     2393      +83     
- Misses        669      805     +136     
- Partials       89       99      +10

Impacted Files	Coverage Δ
pkg/apis/pravega/v1beta1/pravega.go	`97.36% <ø> (ø)`
pkg/apis/pravega/v1beta1/pravegacluster_types.go	`26.20% <5.71%> (+3.46%)`	⬆️
...roller/pravegacluster/pravegacluster_controller.go	`50.45% <22.95%> (-10.46%)`	⬇️
pkg/controller/pravega/pravega_controller.go	`99.00% <50.00%> (-1.00%)`	⬇️
pkg/controller/pravega/pravega_segmentstore.go	`99.42% <50.00%> (-0.58%)`	⬇️
pkg/apis/pravega/v1beta1/zz_generated.deepcopy.go	`97.08% <100.00%> (+0.10%)`	⬆️
pkg/util/pravegacluster.go	`97.03% <100.00%> (+0.09%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c5f0ce...b3100d5. Read the comment docs.

Signed-off-by: prabhaker24 <[email protected]>

anishakj

LGTM

added support to run as root user

d3116e5

Signed-off-by: prabhaker24 <[email protected]>

Prabhaker24 requested review from gaddamas and anishakj October 20, 2020 19:33

fixed formatting errors

ce223c0

Signed-off-by: prabhaker24 <[email protected]>

Prabhaker24 requested a review from Ranganaths8 October 20, 2020 19:38

fixed comment

da91185

Signed-off-by: prabhaker24 <[email protected]>

sarlaccpit reviewed Oct 20, 2020

View reviewed changes

gaddas3 reviewed Oct 20, 2020

View reviewed changes

gaddas3 approved these changes Oct 20, 2020

View reviewed changes

prabhaker24 added 4 commits October 21, 2020 13:14

added security context for controller as well and changed name for th…

f54444a

…e ss security context Signed-off-by: prabhaker24 <[email protected]>

fixed formatting

d0b368a

Signed-off-by: prabhaker24 <[email protected]>

fixed deep copy and added ut

6630c37

Signed-off-by: prabhaker24 <[email protected]>

fixed comment

bbd8524

Signed-off-by: prabhaker24 <[email protected]>

anishakj requested changes Oct 21, 2020

View reviewed changes

modified readme file

958ab5c

Signed-off-by: prabhaker24 <[email protected]>

ut's for controller.go and segmentstore.go file added

b3100d5

Signed-off-by: prabhaker24 <[email protected]>

anishakj approved these changes Oct 21, 2020

View reviewed changes

gaddas3 approved these changes Oct 21, 2020

View reviewed changes

sarlaccpit approved these changes Oct 21, 2020

View reviewed changes

anishakj merged commit 95c21d3 into master Oct 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue-468: Run pravega segmentstore sts with runAsUser = 0 #469

Issue-468: Run pravega segmentstore sts with runAsUser = 0 #469

Prabhaker24 commented Oct 20, 2020 •

edited

Loading

sarlaccpit Oct 20, 2020

Prabhaker24 Oct 21, 2020

gaddas3 Oct 20, 2020

Prabhaker24 Oct 21, 2020

anishakj Oct 21, 2020

Prabhaker24 Oct 21, 2020

codecov-io commented Oct 21, 2020 •

edited

Loading

anishakj left a comment

Issue-468: Run pravega segmentstore sts with runAsUser = 0 #469

Issue-468: Run pravega segmentstore sts with runAsUser = 0 #469

Conversation

Prabhaker24 commented Oct 20, 2020 • edited Loading

Change log description

Purpose of the change

What the code does

How to verify it

sarlaccpit Oct 20, 2020

Choose a reason for hiding this comment

Prabhaker24 Oct 21, 2020

Choose a reason for hiding this comment

gaddas3 Oct 20, 2020

Choose a reason for hiding this comment

Prabhaker24 Oct 21, 2020

Choose a reason for hiding this comment

anishakj Oct 21, 2020

Choose a reason for hiding this comment

Prabhaker24 Oct 21, 2020

Choose a reason for hiding this comment

codecov-io commented Oct 21, 2020 • edited Loading

Codecov Report

anishakj left a comment

Choose a reason for hiding this comment

Prabhaker24 commented Oct 20, 2020 •

edited

Loading

codecov-io commented Oct 21, 2020 •

edited

Loading