Skip to content

Allow users to set custom securityContext in EventListener spec#1832

Merged
tekton-robot merged 1 commit intotektoncd:mainfrom
savitaashture:add_scc_support
May 5, 2025
Merged

Allow users to set custom securityContext in EventListener spec#1832
tekton-robot merged 1 commit intotektoncd:mainfrom
savitaashture:add_scc_support

Conversation

@savitaashture
Copy link
Contributor

  1. Users can now define their own securityContext under the EventListener YAML.
    ex:
spec:
  serviceAccountName: tekton-triggers-example-sa
  resources:
    kubernetesResource:
      spec:
        template:
          spec:
            securityContext:
              runAsNonRoot: true
            containers:
              - resources:
                  requests:
                    memory: "64Mi"
                    cpu: "250m"
                  limits:
                    memory: "128Mi"
                    cpu: "500m"
                securityContext:
                  readOnlyRootFilesystem: true
  1. When el-security-context is true
    • If the user sets a custom securityContext, it is give priority and used same.
    • If not, a default securityContext is applied.

Changes

Submitter Checklist

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

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Add support for setting securityContext as part of EventListener spec

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 21, 2025
@savitaashture savitaashture requested a review from khrm April 21, 2025 07:33
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 21, 2025
@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

@khrm
Copy link
Contributor

khrm commented Apr 23, 2025

/test pull-tekton-triggers-integration-tests

@tekton-robot
Copy link

@khrm: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-tekton-triggers-build-tests
  • /test tekton-triggers-unit-tests

The following commands are available to trigger optional jobs:

  • /test pull-tekton-triggers-go-coverage

Use /test all to run all jobs.

Details

In response to this:

/test pull-tekton-triggers-integration-tests

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.

@khrm khrm closed this Apr 24, 2025
@khrm khrm reopened this Apr 24, 2025
@khrm
Copy link
Contributor

khrm commented Apr 24, 2025

/test pull-tekton-triggers-integration-tests

@tekton-robot
Copy link

@khrm: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-tekton-triggers-build-tests
  • /test tekton-triggers-unit-tests

The following commands are available to trigger optional jobs:

  • /test pull-tekton-triggers-go-coverage

Use /test all to run all jobs.

Details

In response to this:

/test pull-tekton-triggers-integration-tests

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.

@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

1. Users can now define their own securityContext under the EventListener YAML.
2. When  is true
   - If the user sets a custom securityContext, it is give priority and used same.
   - If not, a default securityContext is applied.

Signed-off-by: savitaashture <sashture@redhat.com>
@tekton-robot
Copy link

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/triggers/v1beta1/event_listener_validation.go 97.4% 97.4% 0.0
pkg/reconciler/eventlistener/resources/deployment.go 98.6% 98.6% 0.0

@khrm khrm requested a review from Copilot May 5, 2025 09:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for allowing users to define a custom securityContext in the EventListener spec. Key changes include:

  • Removing the indirection (using pointers) for securityContext fields in tests and resources.
  • Updating validation logic to include and propagate a custom securityContext.
  • Aligning documentation and examples to support the new securityContext option.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/reconciler/eventlistener/resources/deployment_test.go Updated tests to use direct pointer values for securityContext
pkg/reconciler/eventlistener/resources/deployment.go Changed baseSecurityPolicy and related functions to use pointer types; updated securityContext assignment in Deployment creation
pkg/reconciler/eventlistener/resources/container.go Modified MakeContainer to conditionally use provided securityContext or create a default one using pointers
pkg/apis/triggers/v1beta1/event_listener_validation.go Adjusted field mask functions to allow custom securityContext validation
docs/eventlisteners.md Updated documentation to include securityContext in examples and resource specification
Comments suppressed due to low confidence (3)

pkg/reconciler/eventlistener/resources/deployment.go:125

  • Double-check that the fallback to getStrongerSecurityPolicy is applied only when a custom securityContext is not provided. This ordering appears proper but confirm that there is no unintended nil propagation.
if (*c.SetSecurityContext && securityContext == nil) {

pkg/apis/triggers/v1beta1/event_listener_validation.go:299

  • The removal of setting SecurityContext to nil in podSpecMask is intentional to allow custom security contexts. Ensure that this change is clearly explained in the release notes to avoid confusion.
out.SecurityContext = nil

pkg/reconciler/eventlistener/resources/container.go:61

  • The added check and default assignment for containerSecurityContext provides a good fallback when no custom securityContext is set. Confirm that the flag c.SetSecurityContext reliably represents the user’s intent.
if (*c.SetSecurityContext && containerSecurityContext == nil) {

Copy link
Contributor

@khrm khrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2025
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khrm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 5, 2025
@tekton-robot
Copy link

tekton-robot commented May 5, 2025

@savitaashture: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-tekton-triggers-integration-tests 973ffce link true /test pull-tekton-triggers-integration-tests

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@khrm
Copy link
Contributor

khrm commented May 5, 2025

/test tekton-triggers-unit-tests

@tekton-robot tekton-robot merged commit 82f44db into tektoncd:main May 5, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants