Update triggers default configmap for runAsUser and runAsGroup to handle restricted securityContext for Triggers #2125

savitaashture · 2024-04-23T12:15:37Z

Context:

As part of addressing https://issues.redhat.com/browse/OCPSTRAT-487, there's a plan to enable restricted security context by default starting from Openshift 4.17.
Once this becomes the default setting, existing Triggers functionality may break.
This is because we currently set security context to false, and the pipelines-scc security context constraint (SCC) doesn't have seccompProfiles: runtime/default, which is required when restricted security context is enabled by default because by default triggers set runAsUser and runAsGroup` to 65532.

So Updated Triggers only to accept values for runAsUser and runAsGroupthrough CM and using Operator we are setting to""` so that Openshift can set those random values.

Tested below scenarios:

1 set security context to restricted for a namespace (oc label ns test pod-security.kubernetes.io/enforce=restricted --overwrite=true)

No seccompProfiles: runtime/default to pipelines-scc
and Created EL, POD is up and running

securityContext:
 allowPrivilegeEscalation: false
 capabilities:
   drop:
   - ALL
 runAsNonRoot: true
 runAsUser: 1000730000
 seccompProfile:
   type: RuntimeDefault

2 remove security context to restricted for a namespace (oc label ns test pod-security.kubernetes.io/enforce-)

No seccompProfiles: runtime/default to pipelines-scc
and Created EL, POD is up and running

securityContext:
 allowPrivilegeEscalation: false
 capabilities:
   drop:
   - ALL
 runAsNonRoot: true
 runAsUser: 1000710000
 seccompProfile:
   type: RuntimeDefault

Fixes : https://issues.redhat.com/browse/SRVKP-4372

Signed-off-by: Savita Ashture sashture@redhat.com

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Run make test lint before submitting a PR
Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

savitaashture · 2024-04-23T12:22:03Z

@tektoncd/operator-maintainers Will bring this issue in WG call also wanted to talk set-security-context related to Pipeline

savitaashture · 2024-04-26T15:27:16Z

@piyush-garg
I have verified the question which you bought during WG call

When el-security-context is true
EL pod have below securityContext

    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        drop:
        - ALL
        - MKNOD
      runAsGroup: 65532
      runAsNonRoot: true
      runAsUser: 65532
      seccompProfile:
        type: RuntimeDefault
    terminationMessagePath: /dev/termination-log

And pod is up and running with those securityContext values

$ kubectl get pods 
NAME                                  READY   STATUS      RESTARTS   AGE
el-github-listener-7ff9675c4d-jpctb   1/1     Running     0          41s
github-run-fhggv-pod                  0/1     Completed   0          74m

I could even send the requests

curl -v -H 'X-GitHub-Event: pull_request' -H 'X-Hub-Signature: sha1=ba0cdc263b3492a74b601d240c27efe81c4720cb' -H 'Content-Type: application/json' -d '{"action": "opened", "pull_request":{"head":{"sha": "28911bbb5a3e2ea034daf1f6be0a822d50e31e73"}},"repository":{"clone_url": "https://github.com/tektoncd/triggers.git"}}' http://localhost:8080

The taskrun created

$ kubectl get pods 
NAME                                  READY   STATUS      RESTARTS   AGE
el-github-listener-7ff9675c4d-jpctb   1/1     Running     0          62s
github-run-fhggv-pod                  0/1     Completed   0          74m
github-run-k55bm-pod                  1/1     Running     0          4s

savitaashture · 2024-04-26T15:27:59Z

cc @khrm

khrm

Did you test this with OpenShift <=4.15?

savitaashture · 2024-05-06T10:08:00Z

Did you test this with OpenShift <=4.15?

Yes

khrm

/approve
@jkandasa

jkandasa

cc @piyush-garg

jkandasa · 2024-05-07T05:37:14Z

@savitaashture Please update the release notes

piyush-garg · 2024-05-07T07:38:52Z

i dont know how 65532 is working on openshift, and thing which used to work before is not working now. I need to disucss this more

savitaashture · 2024-05-07T11:36:17Z

i dont know how 65532 is working on openshift, and thing which used to work before is not working now. I need to disucss this more

Sure @piyush-garg
should we discuss in Operator WG ? or we can schedule adhoc call and discuss as Operator call is on Thursday which is very late

piyush-garg · 2024-05-07T11:43:05Z

i dont know how 65532 is working on openshift, and thing which used to work before is not working now. I need to disucss this more

Sure @piyush-garg should we discuss in Operator WG ? or we can schedule adhoc call and discuss as Operator call is on Thursday which is very late

Lets setup a adhoc call and try few things which i have in mind, i think it should work fine with the original flag

savitaashture · 2024-05-08T11:58:35Z

When el-security-context set to true

 securityContext:
            allowPrivilegeEscalation: false
            # User 65532 is the distroless nonroot user ID
            runAsUser: 65532
            runAsGroup: 65532
            runAsNonRoot: true
            capabilities:
              drop:
                - "ALL"
            seccompProfile:
              type: RuntimeDefault

all above things will be set but 65532 is not within the OpenShift range.

Therefore, I need to try out a couple of things by making changes in triggers and operators.
As a result, we'll be holding this PR for now, and it won't be included in the 1.15 OSP release.

tekton-robot · 2024-07-08T09:43:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	46.3%	-40.6

tekton-robot · 2024-07-08T15:56:37Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

savitaashture · 2024-07-08T16:01:04Z

please squash the commit into one

squashed

tekton-robot · 2024-07-08T16:08:35Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

savitaashture · 2024-07-08T17:14:28Z

@jkandasa @piyush-garg addressed all the review comments
PTAL
Thank you

jkandasa

Thanks @savitaashture LGTM
cc @piyush-garg

tekton-robot · 2024-07-08T18:19:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jkandasa, khrm, vdemeester

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:

~~OWNERS~~ [jkandasa,vdemeester]

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

piyush-garg · 2024-07-09T06:25:48Z

pkg/apis/operator/v1alpha1/tektontrigger_types.go

@@ -92,5 +92,7 @@ type TriggersProperties struct {
 // OptionalTriggersProperties defines the fields which are to be
 // defined for triggers only if user pass them
 type OptionalTriggersProperties struct {
-	DefaultServiceAccount string `json:"default-service-account,omitempty"`
+	DefaultServiceAccount string  `json:"default-service-account,omitempty"`
+	DefaultRunAsUser      *string `json:"default-run-as-user,omitempty"`


we should not add it here, its a configmap thing and it can be done through options.

we have already stopped adding more configs

@savitaashture ah I just notice, it is applicable for kubernets too.
Will it work without runAsUser and runAsGroup in kubernetes?

we should not add it in spec

we just remove those two values in openshift profile only

user should be able to configure through options in both profile

okay
got it
then in that case i want to set default value as "" for both runAsUser and runAsGroup should i add under Options of Triggers 🤔

user should be able to configure through options in both profile

right but to set default values what is the best way in Operator
Is it through adding under options in tektonConfig ?
I mean shall i add default values here https://github.com/tektoncd/operator/blob/main/config/crs/openshift/config/all/operator_v1alpha1_config_cr.yaml directly

just call the tranformer in openshift extension to set the default value

pkg/reconciler/openshift/tektontrigger/extension.go

tekton-robot · 2024-07-09T11:15:10Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

tekton-robot · 2024-07-09T11:33:57Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

piyush-garg · 2024-07-09T11:35:44Z

pkg/reconciler/openshift/tektontrigger/extension.go

+	// Updating the default values of runAsUser and runAsGroup to an empty string
+	// to ensure compatibility with OpenShift's requirements for managing these settings
+	// in Triggers Eventlistener containers SCC.
+	type triggersProperties struct {


can we define the struct out of this function

moved out of the function

piyush-garg · 2024-07-09T11:36:01Z

pkg/reconciler/openshift/tektontrigger/extension.go

+		DefaultRunAsUser  *string `json:"default-run-as-user,omitempty"`
+		DefaultRunAsGroup *string `json:"default-run-as-group,omitempty"`
+	}
+	triggersData := triggersProperties{


this can also be defined like a constant outside

updated a bit please check now

tekton-robot · 2024-07-09T12:16:28Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

…ult cm for triggers to handle restricted securityContext Context: As part of addressing https://issues.redhat.com/browse/OCPSTRAT-487, there's a plan to enable restricted security context by default starting from Openshift 4.16. Once this becomes the default setting, existing Triggers functionality may break. This is because we currently set security context to false, and the pipelines-scc security context constraint (SCC) doesn't have seccompProfiles: runtime/default, which is required when restricted security context is enabled by default.

tekton-robot · 2024-07-09T12:30:00Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/openshift/tektontrigger/transformers.go	87.0%	82.6%	-4.3

piyush-garg · 2024-07-09T13:56:01Z

/lgtm

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Apr 23, 2024

tekton-robot requested review from piyush-garg and PuneetPunamiya April 23, 2024 12:15

tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 23, 2024

savitaashture changed the title ~~[WIP] Update scc to add seccompProfiles to handle restricted securityContext~~ Update scc to add seccompProfiles to handle restricted securityContext for Triggers Apr 30, 2024

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2024

khrm reviewed May 6, 2024

View reviewed changes

jkandasa approved these changes May 7, 2024

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 7, 2024

savitaashture changed the title ~~Update scc to add seccompProfiles to handle restricted securityContext for Triggers~~ [WIP] Update scc to add seccompProfiles to handle restricted securityContext for Triggers May 8, 2024

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2024

vdemeester approved these changes May 21, 2024

View reviewed changes

savitaashture force-pushed the fix_scc_when_restricted branch from f7a28b8 to 5decdc6 Compare July 8, 2024 09:38

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 8, 2024

savitaashture changed the title ~~[WIP] Update scc to add seccompProfiles to handle restricted securityContext for Triggers~~ Update scc to add seccompProfiles to handle restricted securityContext for Triggers Jul 8, 2024

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2024

savitaashture force-pushed the fix_scc_when_restricted branch from 5decdc6 to a7085c0 Compare July 8, 2024 10:18

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2024

savitaashture force-pushed the fix_scc_when_restricted branch from 19e5656 to 5286871 Compare July 8, 2024 15:51

savitaashture force-pushed the fix_scc_when_restricted branch from 5286871 to 987520c Compare July 8, 2024 16:00

jkandasa approved these changes Jul 8, 2024

View reviewed changes

piyush-garg reviewed Jul 9, 2024

View reviewed changes

savitaashture force-pushed the fix_scc_when_restricted branch from 987520c to 8c05647 Compare July 9, 2024 11:08

tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 9, 2024

jkandasa reviewed Jul 9, 2024

View reviewed changes

pkg/reconciler/openshift/tektontrigger/extension.go Outdated Show resolved Hide resolved

savitaashture force-pushed the fix_scc_when_restricted branch from 8c05647 to d553b6f Compare July 9, 2024 11:26

piyush-garg reviewed Jul 9, 2024

View reviewed changes

savitaashture force-pushed the fix_scc_when_restricted branch 2 times, most recently from 5f27bc5 to 3d3dd91 Compare July 9, 2024 12:09

savitaashture force-pushed the fix_scc_when_restricted branch from 3d3dd91 to 8bba2da Compare July 9, 2024 12:23

tekton-robot assigned piyush-garg Jul 9, 2024

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2024

tekton-robot merged commit 23fb02a into tektoncd:main Jul 9, 2024
7 of 8 checks passed

savitaashture mentioned this pull request Aug 19, 2024

Update triggers default configmap for FSGroup to handle restricted securityContext for Triggers #2285

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update triggers default configmap for runAsUser and runAsGroup to handle restricted securityContext for Triggers #2125

Update triggers default configmap for runAsUser and runAsGroup to handle restricted securityContext for Triggers #2125

savitaashture commented Apr 23, 2024 •

edited

Loading

savitaashture commented Apr 23, 2024

savitaashture commented Apr 26, 2024

savitaashture commented Apr 26, 2024

khrm left a comment

savitaashture commented May 6, 2024

khrm left a comment

jkandasa left a comment

jkandasa commented May 7, 2024

piyush-garg commented May 7, 2024

savitaashture commented May 7, 2024 •

edited

Loading

piyush-garg commented May 7, 2024 •

edited

Loading

savitaashture commented May 8, 2024

tekton-robot commented Jul 8, 2024

tekton-robot commented Jul 8, 2024

savitaashture commented Jul 8, 2024

tekton-robot commented Jul 8, 2024

savitaashture commented Jul 8, 2024

jkandasa left a comment

tekton-robot commented Jul 8, 2024

piyush-garg Jul 9, 2024

piyush-garg Jul 9, 2024

jkandasa Jul 9, 2024

piyush-garg Jul 9, 2024

savitaashture Jul 9, 2024 •

edited

Loading

piyush-garg Jul 9, 2024

tekton-robot commented Jul 9, 2024

tekton-robot commented Jul 9, 2024

piyush-garg Jul 9, 2024

savitaashture Jul 9, 2024

piyush-garg Jul 9, 2024

savitaashture Jul 9, 2024

tekton-robot commented Jul 9, 2024

tekton-robot commented Jul 9, 2024

piyush-garg commented Jul 9, 2024

Update triggers default configmap for runAsUser and runAsGroup to handle restricted securityContext for Triggers #2125

Update triggers default configmap for runAsUser and runAsGroup to handle restricted securityContext for Triggers #2125

Conversation

savitaashture commented Apr 23, 2024 • edited Loading

Submitter Checklist

Release Notes

savitaashture commented Apr 23, 2024

savitaashture commented Apr 26, 2024

savitaashture commented Apr 26, 2024

khrm left a comment

Choose a reason for hiding this comment

savitaashture commented May 6, 2024

khrm left a comment

Choose a reason for hiding this comment

jkandasa left a comment

Choose a reason for hiding this comment

jkandasa commented May 7, 2024

piyush-garg commented May 7, 2024

savitaashture commented May 7, 2024 • edited Loading

piyush-garg commented May 7, 2024 • edited Loading

savitaashture commented May 8, 2024

tekton-robot commented Jul 8, 2024

tekton-robot commented Jul 8, 2024

savitaashture commented Jul 8, 2024

tekton-robot commented Jul 8, 2024

savitaashture commented Jul 8, 2024

jkandasa left a comment

Choose a reason for hiding this comment

tekton-robot commented Jul 8, 2024

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

savitaashture Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Jul 9, 2024

tekton-robot commented Jul 9, 2024

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

tekton-robot commented Jul 9, 2024

tekton-robot commented Jul 9, 2024

piyush-garg commented Jul 9, 2024

savitaashture commented Apr 23, 2024 •

edited

Loading

savitaashture commented May 7, 2024 •

edited

Loading

piyush-garg commented May 7, 2024 •

edited

Loading

savitaashture Jul 9, 2024 •

edited

Loading