Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve PodSecurityAdmission restrictions on 1.23+ for deprecated PodSecurityPolicy #5652

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

JeromeJu
Copy link
Member

@JeromeJu JeromeJu commented Oct 18, 2022

Changes

This commit fixes the issue where the securityContext are not restricted in PodSecurityAdmission(PSA). This removes the PodSeucrityPolicy, which is deprecated in Kubernetes v1.21 and removed from v1.25. This adds to the PSA restricted label with respective policies enforced by PSP but not covered by the restricted standard of PSA.

The previous settings in 101-podsecuritypolicy.yaml are restricting roles to be run as nonRoot, this is covered by the PodSecurityStandard of the restricted level.

/kind bug
Fixes #5603 #4112

Submitter Checklist

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

  • [n/a] Has Docs included if any changes are user facing
  • [n/a] 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
  • [n/a] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • [n/a] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Action required: If using Kubernetes 1.22, set PodSecurity flag to true to enforce a restricted pod security level in Tekton namespaces. See https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-graduated-or-deprecated-features for more information.

@tekton-robot tekton-robot added kind/bug Categorizes issue or PR as related to a bug. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Oct 18, 2022
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 18, 2022
@JeromeJu
Copy link
Member Author

/retest

test.yaml Outdated Show resolved Hide resolved
@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

@JeromeJu I'm still getting the same error - I used this kind config YAML:

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
name: kind1.23.12
nodes:
- role: control-plane
  image: kindest/node:v1.23.12@sha256:9402cf1330bbd3a0d097d2033fa489b2abe40d479cc5ef47d0b6a6960613148a
- role: worker
  image: kindest/node:v1.23.12@sha256:9402cf1330bbd3a0d097d2033fa489b2abe40d479cc5ef47d0b6a6960613148a

..and ran ko apply -R -f config. Then kc get deployment -n tekton-pipelines tekton-pipelines-controller -o yaml again, and got the same thing:

status:
  conditions:
  - lastTransitionTime: "2022-10-18T14:47:31Z"
    lastUpdateTime: "2022-10-18T14:47:31Z"
    message: Created new replica set "tekton-pipelines-controller-6854456f96"
    reason: NewReplicaSetCreated
    status: "True"
    type: Progressing
  - lastTransitionTime: "2022-10-18T14:47:31Z"
    lastUpdateTime: "2022-10-18T14:47:31Z"
    message: Deployment does not have minimum availability.
    reason: MinimumReplicasUnavailable
    status: "False"
    type: Available
  - lastTransitionTime: "2022-10-18T14:47:31Z"
    lastUpdateTime: "2022-10-18T14:47:31Z"
    message: 'pods "tekton-pipelines-controller-6854456f96-vbxdx" is forbidden: violates
      PodSecurity "restricted:latest": unrestricted capabilities (container "tekton-pipelines-controller"
      must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod
      or container "tekton-pipelines-controller" must set securityContext.runAsNonRoot=true),
      seccompProfile (pod or container "tekton-pipelines-controller" must set securityContext.seccompProfile.type
      to "RuntimeDefault" or "Localhost")'
    reason: FailedCreate
    status: "True"
    type: ReplicaFailure
  observedGeneration: 1
  unavailableReplicas: 1

@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

@JeromeJu Ok, with the following diff (also updates config/resolvers/100-namespace.yaml and config/resolvers/resolvers-deployment.yaml), it works:

diff --git a/config/controller.yaml b/config/controller.yaml
index ea2a09347..bfbc7553d 100644
--- a/config/controller.yaml
+++ b/config/controller.yaml
@@ -122,10 +122,13 @@ spec:
           allowPrivilegeEscalation: false
           capabilities:
             drop:
-            - all
+            - "ALL"
           # User 65532 is the nonroot user ID
           runAsUser: 65532
           runAsGroup: 65532
+          runAsNonRoot: true
+          seccompProfile:
+            type: RuntimeDefault
         ports:
         - name: metrics
           containerPort: 9090
diff --git a/config/resolvers/100-namespace.yaml b/config/resolvers/100-namespace.yaml
index bd2f4b803..08f05ca18 100644
--- a/config/resolvers/100-namespace.yaml
+++ b/config/resolvers/100-namespace.yaml
@@ -20,3 +20,4 @@ metadata:
     app.kubernetes.io/component: resolvers
     app.kubernetes.io/instance: default
     app.kubernetes.io/part-of: tekton-pipelines
+    pod-security.kubernetes.io/enforce: restricted
diff --git a/config/resolvers/resolvers-deployment.yaml b/config/resolvers/resolvers-deployment.yaml
index b9b0c541c..1a9cb28ac 100644
--- a/config/resolvers/resolvers-deployment.yaml
+++ b/config/resolvers/resolvers-deployment.yaml
@@ -101,4 +101,6 @@ spec:
           runAsNonRoot: true
           capabilities:
             drop:
-            - all
+            - "ALL"
+          seccompProfile:
+            type: RuntimeDefault
diff --git a/config/webhook.yaml b/config/webhook.yaml
index f6c0cb07f..6cfb2ef80 100644
--- a/config/webhook.yaml
+++ b/config/webhook.yaml
@@ -111,10 +111,13 @@ spec:
           allowPrivilegeEscalation: false
           capabilities:
             drop:
-            - all
+            - "ALL"
           # User 65532 is the distroless nonroot user ID
           runAsUser: 65532
           runAsGroup: 65532
+          runAsNonRoot: true
+          seccompProfile:
+            type: RuntimeDefault
         ports:
         - name: metrics
           containerPort: 9090

Yes, the - all to - "ALL" apparently matters, as does the seccompProfile.type=RuntimeDefault, which does appear to need to be on the container's securityContext, despite the error when it's not set saying it needs to be set on either the pod or the container.

Testing now on k8s 1.22...

@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

Ok, with that diff above applied to this branch, it works on k8s 1.22, 1.23, and 1.24.

@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

@JeromeJu Oops, my diff was missing this:

diff --git a/config/resolvers/100-namespace.yaml b/config/resolvers/100-namespace.yaml
index bd2f4b803..08f05ca18 100644
--- a/config/resolvers/100-namespace.yaml
+++ b/config/resolvers/100-namespace.yaml
@@ -20,3 +20,4 @@ metadata:
     app.kubernetes.io/component: resolvers
     app.kubernetes.io/instance: default
     app.kubernetes.io/part-of: tekton-pipelines
+    pod-security.kubernetes.io/enforce: restricted

The resolvers deployment will still come online without it, but the tekton-pipelines-resolvers namespace won't be restricted.

@JeromeJu JeromeJu force-pushed the PSA-1.23 branch 2 times, most recently from ca24641 to aab7dc3 Compare October 18, 2022 15:45
@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer

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:

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 Oct 18, 2022
@jerop jerop added this to the Pipelines v0.41 milestone Oct 18, 2022
@lbernick
Copy link
Member

@JeromeJu suggested rewording of the release notes:

"Action required: If using Kubernetes 1.22, set PodSecurity flag to true to enforce a restricted pod security level in Tekton namespaces. See https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-graduated-or-deprecated-features for more information."

Also, a more descriptive commit title would be "Replace PodSecurityPolicy with PodSecurityAdmission".
Happy to LGTM once this is rebased.

This commit fixes the issue where the securityContext are not restricted
in PodSecurityAdmission(PSA). This removes the PodSeucrityPolicy, which
is deprecated in Kubernetes v1.21 and removed from v1.25. This adds to the
PSA restricted label with respective policies enforced by PSP but not
covered by the restricted standard of PSA.
@JeromeJu
Copy link
Member Author

Thanks @lbernick , updated.

@JeromeJu suggested rewording of the release notes:

"Action required: If using Kubernetes 1.22, set PodSecurity flag to true to enforce a restricted pod security level in Tekton namespaces. See https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-graduated-or-deprecated-features for more information."

Also, a more descriptive commit title would be "Replace PodSecurityPolicy with PodSecurityAdmission". Happy to LGTM once this is rebased.

@lbernick
Copy link
Member

@abayer I know our CI runs e2e tests on kind w/ k8s version 1.23 now and I can see you've tested on 1.24 as well. Do we have a way of testing this on all the k8s versions we support via CI, or do you think it's OK to merge this?

@abayer
Copy link
Contributor

abayer commented Oct 18, 2022

There's no easy way to test them all in CI, sadly, but yeah, this is good to merge, IMO.

@lbernick
Copy link
Member

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2022
@tekton-robot tekton-robot merged commit 2ae8b6a into tektoncd:main Oct 18, 2022
dibyom added a commit to dibyom/triggers that referenced this pull request Nov 3, 2022
This commit drops the Triggers PodSecurityPolicy since its deprecated and is
going to be removed in Kubernetes 1.25 in favor of PodSecurityAdmission.

In addition, it adds the `securityContext` required for the "restricted"
PodSecurityAdmission levels. These changes are necessary for Triggers to work
with Pipelines v0.41 and higher because tektoncd/pipeline#5652  started
enforcing the restricted pod security level for all pods in the
`tekton-pipelines` namespace (which includes the triggers controller, webhook,
and core interceptor deployments).

Fixes tektoncd#1447 and required for tektoncd#1475

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/triggers that referenced this pull request Nov 3, 2022
This commit drops the Triggers PodSecurityPolicy since its deprecated and is
going to be removed in Kubernetes 1.25 in favor of PodSecurityAdmission.

In addition, it adds the `securityContext` required for the "restricted"
PodSecurityAdmission levels. These changes are necessary for Triggers to work
with Pipelines v0.41 and higher because tektoncd/pipeline#5652  started
enforcing the restricted pod security level for all pods in the
`tekton-pipelines` namespace (which includes the triggers controller, webhook,
and core interceptor deployments).

Fixes tektoncd#1447 and required for tektoncd#1475

Signed-off-by: Dibyo Mukherjee <[email protected]>
tekton-robot pushed a commit to tektoncd/triggers that referenced this pull request Nov 4, 2022
This commit drops the Triggers PodSecurityPolicy since its deprecated and is
going to be removed in Kubernetes 1.25 in favor of PodSecurityAdmission.

In addition, it adds the `securityContext` required for the "restricted"
PodSecurityAdmission levels. These changes are necessary for Triggers to work
with Pipelines v0.41 and higher because tektoncd/pipeline#5652  started
enforcing the restricted pod security level for all pods in the
`tekton-pipelines` namespace (which includes the triggers controller, webhook,
and core interceptor deployments).

Fixes #1447 and required for #1475

Signed-off-by: Dibyo Mukherjee <[email protected]>
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. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Container configurations need to be updated for PodSecurity on k8s 1.23+
5 participants