Added tests to tests/gh-actions to enable baseline and restricted PSS

[biswajit-9776]

Pull Request Template for Kubeflow manifests Issues

✏️ A brief description of the changes

I added PSS labels to kubeflow namespace install_multi_tenancy.sh and fixed indentation.

📦 List any dependencies that are required for this change

My PR depends on #

🐛 If this PR is related to an issue, please put the link to the issue here.

The following issues are related, because ...

✅ Contributor checklist

• Make sure you have tested with kustomize. See Installation Prerequisites
• All the commits have been signed-off (To pass the DCO check)
• Submit the Contributor License Agreements (To pass the cla/google check)

---

You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

[biswajit-9776]

The profile-controller deployment does fail for PSS labels enforce=baseline and enforce=restricted as follows:

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "restricted:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: allowPrivilegeEscalation != false, unrestricted capabilities, runAsNonRoot != true, runAsUser=0, seccompProfile
namespace/kubeflow labeled

and

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "baseline:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: non-default capabilities
namespace/kubeflow labeled

I suppose we will configure these labels as components or patches for the deployment.

[juliusvonkohout]

I think we should apply all the patches for all namespaces from what you added to example/kustomization.yaml

So what you referenced here.

# Pod Security Standards
# https://kubernetes.io/docs/concepts/security/pod-security-standards/
# Uncomment to enable baseline level standards
# - ../contrib/security/PSS/static/baseline
# Uncomment to enable restricted level standards
# - ../contrib/security/PSS/static/restricted
# Uncomment to enable baseline level standards for dynamic namespaces
# - ../contrib/security/PSS/dynamic/baseline
# Uncomment to enable restricted level standards for dynamic namespaces
# - ../contrib/security/PSS/dynamic/restricted

Since they are components you need to link to all the patches referenced in the components.

[juliusvonkohout]

Here is the reference https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/

[biswajit-9776]

Since the rest of the patches in PSS/static/baseline are of istio-system, auth and oauth2-proxy; let's maybe add only apply the patch for the kubeflow namespace in the ./tests/gh-actions/install_multi_tenancy.sh

[juliusvonkohout]

Yes, then please add them to the other workflows. E.g. Istio to the istio Workflow.

[juliusvonkohout]

The profile-controller deployment does fail for PSS labels enforce=baseline and enforce=restricted as follows:

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "restricted:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: allowPrivilegeEscalation != false, unrestricted capabilities, runAsNonRoot != true, runAsUser=0, seccompProfile
namespace/kubeflow labeled

and

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "baseline:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: non-default capabilities
namespace/kubeflow labeled

I suppose we will configure these labels as components or patches for the deployment.

Very interesting,

please post

The profile-controller deployment does fail for PSS labels enforce=baseline and enforce=restricted as follows:

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "restricted:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: allowPrivilegeEscalation != false, unrestricted capabilities, runAsNonRoot != true, runAsUser=0, seccompProfile
namespace/kubeflow labeled

and

Warning: existing pods in namespace "kubeflow" violate the new PodSecurity enforce level "baseline:latest"
Warning: profiles-deployment-79f5cf977d-vfmhb: non-default capabilities
namespace/kubeflow labeled

I suppose we will configure these labels as components or patches for the deployment.

Please post the yaml from the cluster including the securitycontext i need to see which capabilities are set.

Please also check https://github.com/kubeflow/manifests/blob/master/apps/profiles/upstream/manager/manager.yaml

[biswajit-9776]

Below is the yaml generated for the deployment profiles-deployment

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "1"
  labels:
    kustomize.component: profiles
  name: profiles-deployment
  namespace: kubeflow
  resourceVersion: "1475"
  uid: 11d662da-0459-4de3-910a-96ef9e3b8d92
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      kustomize.component: profiles
  strategy:
    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%
    type: RollingUpdate
  template:
    metadata:
      annotations:
        sidecar.istio.io/inject: "true"
      creationTimestamp: null
      labels:
        kustomize.component: profiles
    spec:
      containers:
      - command:
        - /access-management
        - -cluster-admin
        - $(ADMIN)
        - -userid-header
        - $(USERID_HEADER)
        - -userid-prefix
        - $(USERID_PREFIX)
        envFrom:
        - configMapRef:
            name: profiles-config-5h9m86f79f
        image: docker.io/kubeflownotebookswg/kfam:v1.9.0
        imagePullPolicy: IfNotPresent
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /metrics
            port: 8081
            scheme: HTTP
          initialDelaySeconds: 30
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 1
        name: kfam
        ports:
        - containerPort: 8081
          name: kfam-http
          protocol: TCP
        resources: {}
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
      - command:
        - /manager
        - -userid-header
        - $(USERID_HEADER)
        - -userid-prefix
        - $(USERID_PREFIX)
        - -workload-identity
        - $(WORKLOAD_IDENTITY)
        envFrom:
        - configMapRef:
            name: profiles-config-5h9m86f79f
        image: docker.io/kubeflownotebookswg/profile-controller:v1.9.0
        imagePullPolicy: IfNotPresent
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz
            port: 9876
            scheme: HTTP
          initialDelaySeconds: 15
          periodSeconds: 20
          successThreshold: 1
          timeoutSeconds: 1
        name: manager
        ports:
        - containerPort: 9876
          protocol: TCP
        readinessProbe:
          failureThreshold: 3
          httpGet:
            path: /readyz
            port: 9876
            scheme: HTTP
          initialDelaySeconds: 5
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        resources: {}
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
        volumeMounts:
        - mountPath: /etc/profile-controller
          name: namespace-labels
          readOnly: true
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      serviceAccount: profiles-controller-service-account
      serviceAccountName: profiles-controller-service-account
      terminationGracePeriodSeconds: 30
      volumes:
      - configMap:
          defaultMode: 420
          name: namespace-labels-data-4df5t8mdgf
        name: namespace-labels

[biswajit-9776]

Both kfam and manager seem to be missing the securityContext option. Added securityContext.allowPrivilegeEscalation: false to manager.yaml and it does counter the warnings as follows:

$ kubectl rollout restart deployment profiles-deployment -n kubeflow
Warning: would violate PodSecurity "restricted:latest": allowPrivilegeEscalation != false (container "kfam" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (containers "kfam", "manager" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "kfam", "manager" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "kfam", "manager" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
deployment.apps/profiles-deployment restarted

[juliusvonkohout]

Both kfam and manager seem to be missing the securityContext option. Added securityContext.allowPrivilegeEscalation: false to manager.yaml and it does counter the warnings as follows:

$ kubectl rollout restart deployment profiles-deployment -n kubeflow
Warning: would violate PodSecurity "restricted:latest": allowPrivilegeEscalation != false (container "kfam" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (containers "kfam", "manager" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "kfam", "manager" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "kfam", "manager" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
deployment.apps/profiles-deployment restarted

Then please patch this in contrib/security for the time being and create an upstream PR in kubeflow/kubeflow to properly set the Security context.

[juliusvonkohout]

Check if the namespace if it exists, if yes patch it. Start with baseline (static + dynamic) now and deal with restricted later in follow up PRs

[juliusvonkohout]

The patch for the securitycontext of the profile controller belongs to /contrib/security/PSS/patches as well and needs to be applied here. And list the patches in example/kustomization.yaml

[biswajit-9776]

Let me push the scripts in this PR itself for baseline labels of static and dynamic namespaces.

[juliusvonkohout]

I think you should loop over the array instead of writing it 5 times.

[biswajit-9776]

Okay

[biswajit-9776]

Our security/PSS/dynamic directory structure looks as follows:

dynamic/
├── baseline/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml
├── restricted/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml

The above kustomization.yaml files are what we use in the components: section of example/kustomization.yaml.

Do you think it would be a good idea to modify the above directory structure as follows:

dynamic/
├── baseline/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file
├── restricted/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file

After changing the structure, we can now apply the PSS labels by applying the dynamic/baseline/kustomization.yaml in scripts at tests/gh-actions and can place dynamic/baseline/components in our components: section of example/kustomization.yaml

[juliusvonkohout]

Our security/PSS/dynamic directory structure looks as follows:

dynamic/
├── baseline/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml
├── restricted/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml

The above kustomization.yaml files are what we use in the components: section of example/kustomization.yaml.

Do you think it would be a good idea to modify the above directory structure as follows:

dynamic/
├── baseline/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file
├── restricted/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file

After changing the structure, we can now apply the PSS labels by applying the dynamic/baseline/kustomization.yaml in scripts at tests/gh-actions and can place dynamic/baseline/components in our components: section of example/kustomization.yaml

I am confused. Now I see twice as many kustomization.yamls.
Are these files under the proposed components folder really kustomize components? If not, then the name does not fit. If they are patches, then use the name patches for the folder.

[biswajit-9776]

Our security/PSS/dynamic directory structure looks as follows:

dynamic/
├── baseline/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml
├── restricted/
│   ├── kustomization.yaml   <--- component with configMapGeneraotr
│   ├── namespace-labels.yaml

The above kustomization.yaml files are what we use in the components: section of example/kustomization.yaml.
Do you think it would be a good idea to modify the above directory structure as follows:

dynamic/
├── baseline/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file
├── restricted/
│   ├─-components/
│   │    ├-─kustomization.yaml   <--- merging the configMapGeneraotr
│   │    ├── namespace-labels.yaml
│   ├── kustomization.yaml   <--- kustomization file

After changing the structure, we can now apply the PSS labels by applying the dynamic/baseline/kustomization.yaml in scripts at tests/gh-actions and can place dynamic/baseline/components in our components: section of example/kustomization.yaml

I am confused. Now I see twice as many kustomization.yamls. Are these files under the proposed components folder really kustomize components? If not, then the name does not fit. If they are patches, then use the name patches for the folder.

Yes, they're kustomize components indeed. But, testing it locally doesn't override the workloads to point to the new configMap so maybe it's not a good idea. I thought of restructuring so as to apply our dynamic components in tests/gh-actions

[juliusvonkohout]

"I thought of restructuring so as to apply our dynamic components in tests/gh-actions" Yes lets try to not duplicate code or manifests. In the end we need to have this ready soon and proper patches for all misbehaving pods/deployments

[juliusvonkohout]

/lgtm
/approve

Lets continue in a new PR, this one here is too old. In the next PR you should check in the test whether pods are blocked by PSS.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

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 [juliusvonkohout]

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