tektoncd / triggers

Event triggering with Tekton!
Apache License 2.0
545 stars 416 forks source link

Update securityContext to include required configurations when the flag el-security-context is enabled #1736

Closed savitaashture closed 1 month ago

savitaashture commented 1 month ago

As part of this PR, when the flag el-security-context is enabled the securityContext have all configuration except RunAsUser and RunAsGroup

Added new fields default-run-as-user and default-run-as-group to config-defaults-triggers configmap so that RunAsUser and RunAsGroup can be now configured through CM

This change handles cases in environments where user ID 65532 is not allowed, such as OpenShift.

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

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Release Notes

* el-security-context flag will set securityContext except RunAsUser and RunAsGroup
* Added new fields default-run-as-user and default-run-as-group to config-defaults-triggers configmap so that RunAsUser and RunAsGroup can be now configured through CM
tekton-robot commented 1 month ago

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

File Old Coverage New Coverage Delta
pkg/apis/config/default.go 92.3% 51.9% -40.5
pkg/reconciler/eventlistener/eventlistener.go 71.3% 71.5% 0.2
tekton-robot commented 1 month ago

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

File Old Coverage New Coverage Delta
pkg/apis/config/default.go 92.3% 81.5% -10.8
pkg/reconciler/eventlistener/eventlistener.go 71.3% 71.5% 0.2
tekton-robot commented 1 month ago

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

File Old Coverage New Coverage Delta
pkg/apis/config/default.go 92.3% 88.9% -3.4
pkg/reconciler/eventlistener/eventlistener.go 71.3% 71.5% 0.2
dibyom commented 1 month ago

One nit and looks like we need to fix the yaml files. Otherwise, LGTM

pkg/apis/config/testdata/config-defaults-triggers-empty-val.yaml
  24:1      error    too many blank lines (1 > 0)  (empty-lines)
pkg/apis/config/testdata/config-defaults-triggers.yaml
  24:1      error    too many blank lines (1 > 0)  (empty-lines)

/approve

tekton-robot commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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](https://github.com/tektoncd/triggers/blob/main/OWNERS)~~ [dibyom] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
tekton-robot commented 1 month ago

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

File Old Coverage New Coverage Delta
pkg/apis/config/default.go 92.3% 88.9% -3.4
pkg/reconciler/eventlistener/eventlistener.go 71.3% 71.5% 0.2
savitaashture commented 1 month ago

@khrm PTAL Thank you