Add new flag set-read-only-root-filesystem to set readOnlyRootFilesys… #1742

kristofferchr · 2024-07-02T22:13:32Z

Solves: #1741
Support to set readOnlyRootFilesystem for EventListener container

Changes

Add new flag set-read-only-root-filesystem.
Sets container securityContext readOnlyRootFilesystem to true when new flag is set to true

Prior to this it was not possible to set readOnlyRootFilesystem for EventListener container. Aligns security towards azure aks best practices and industry

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

Added a new flag `el-read-only-root-filesystem` to the `tekton-triggers-controller` container. This flag, which is set to `true` by default, configures the `EventListener` container's `securityContext.readOnlyRootFilesystem` to `true`. This change aligns with Azure AKS best practices and enhances security.

tekton-robot · 2024-07-02T22:13:42Z

Hi @kristofferchr. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

kristofferchr · 2024-07-02T22:20:39Z

/kind feature

kristofferchr · 2024-07-02T22:22:34Z

Unsure if this should have docs. Could not find docs for flag el-security-context

khrm

/ok-to-test

kristofferchr · 2024-07-15T11:09:26Z

@khrm and/or @savitaashture does any of you have time to spare to review this?

khrm · 2024-07-15T13:53:25Z

I would review this tomorrow. Sorry for the delay.

khrm

/lgtm

Thanks for the PR.

@savitaashture I think we can merge this.

khrm · 2024-07-24T12:23:05Z

@savitaashture Let's review this.

savitaashture · 2024-07-24T12:26:05Z

Hi @kristofferchr 👋
Sorry for the delay
Will take a look at it

kristofferchr · 2024-07-25T07:45:00Z

Hi @kristofferchr 👋

Sorry for the delay

Will take a look at it

Hi 😃 great!

savitaashture · 2024-07-30T08:42:58Z

pkg/reconciler/eventlistener/resources/container.go

+	if *c.SetReadOnlyRootFilesystem {
+		containerSecurityContext.ReadOnlyRootFilesystem = ptr.Bool(true)
+	}


can we move this under SetSecurityContext block itself

I mean if SetSecurityContext

if *c.SetSecurityContext { if *c.SetReadOnlyRootFilesystem { containerSecurityContext.ReadOnlyRootFilesystem = ptr.Bool(true) } containerSecurityContext = corev1.SecurityContext{ AllowPrivilegeEscalation: ptr.Bool(false), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, RunAsNonRoot: ptr.Bool(cfg.Defaults.DefaultRunAsNonRoot), SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, }, } }

Fine by me. I will update the flag docs to note that the flag can only be used when el-security-context is set.

Updated @savitaashture

…em for EventListener Add new flag set-read-only-root-filesystem. Sets container securityContext readOnlyRootFilesystem to true when new flag is set to true Prior to this it was not possible to set readOnlyRootFilesystem for EventListener container. Aligns security towards azure aks best practices and industry

savitaashture

Thank you @kristofferchr

tekton-robot · 2024-08-02T07:26:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: savitaashture

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~~ [savitaashture]

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

savitaashture · 2024-08-02T07:26:32Z

@savitaashture Let's review this.

@khrm can you do lgtm again

khrm

/lgtm

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Jul 2, 2024

tekton-robot requested review from khrm and savitaashture July 2, 2024 22:13

tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 2, 2024

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Jul 2, 2024

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 2, 2024

khrm reviewed Jul 3, 2024

View reviewed changes

kristofferchr force-pushed the 3-set-readonlyrootfilesystem-to-true-in-securitycontext-for-podtemplate-to-eventlistener branch from 1623b5d to c2adf62 Compare July 15, 2024 11:05

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 15, 2024

khrm reviewed Jul 17, 2024

View reviewed changes

tekton-robot assigned khrm Jul 17, 2024

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

savitaashture reviewed Jul 30, 2024

View reviewed changes

kristofferchr force-pushed the 3-set-readonlyrootfilesystem-to-true-in-securitycontext-for-podtemplate-to-eventlistener branch from c2adf62 to 5311d75 Compare August 1, 2024 10:48

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2024

savitaashture approved these changes Aug 2, 2024

View reviewed changes

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

khrm reviewed Aug 5, 2024

View reviewed changes

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

tekton-robot merged commit 2474af6 into tektoncd:main Aug 5, 2024
6 checks passed

kristofferchr deleted the 3-set-readonlyrootfilesystem-to-true-in-securitycontext-for-podtemplate-to-eventlistener branch August 5, 2024 12:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new flag set-read-only-root-filesystem to set readOnlyRootFilesys… #1742

Add new flag set-read-only-root-filesystem to set readOnlyRootFilesys… #1742

kristofferchr commented Jul 2, 2024 •

edited

Loading

tekton-robot commented Jul 2, 2024

kristofferchr commented Jul 2, 2024

kristofferchr commented Jul 2, 2024 •

edited

Loading

khrm left a comment

kristofferchr commented Jul 15, 2024

khrm commented Jul 15, 2024

khrm left a comment

khrm commented Jul 24, 2024

savitaashture commented Jul 24, 2024

kristofferchr commented Jul 25, 2024

savitaashture Jul 30, 2024 •

edited

Loading

kristofferchr Aug 1, 2024

kristofferchr Aug 1, 2024

savitaashture left a comment

tekton-robot commented Aug 2, 2024

savitaashture commented Aug 2, 2024

khrm left a comment

Add new flag set-read-only-root-filesystem to set readOnlyRootFilesys… #1742

Add new flag set-read-only-root-filesystem to set readOnlyRootFilesys… #1742

Conversation

kristofferchr commented Jul 2, 2024 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jul 2, 2024

kristofferchr commented Jul 2, 2024

kristofferchr commented Jul 2, 2024 • edited Loading

khrm left a comment

Choose a reason for hiding this comment

kristofferchr commented Jul 15, 2024

khrm commented Jul 15, 2024

khrm left a comment

Choose a reason for hiding this comment

khrm commented Jul 24, 2024

savitaashture commented Jul 24, 2024

kristofferchr commented Jul 25, 2024

savitaashture Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

kristofferchr Aug 1, 2024

Choose a reason for hiding this comment

kristofferchr Aug 1, 2024

Choose a reason for hiding this comment

savitaashture left a comment

Choose a reason for hiding this comment

tekton-robot commented Aug 2, 2024

savitaashture commented Aug 2, 2024

khrm left a comment

Choose a reason for hiding this comment

kristofferchr commented Jul 2, 2024 •

edited

Loading

kristofferchr commented Jul 2, 2024 •

edited

Loading

savitaashture Jul 30, 2024 •

edited

Loading