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

Fix seccomp error on helmchart deployment #6686

Closed
wants to merge 1 commit into from
Closed

Fix seccomp error on helmchart deployment #6686

wants to merge 1 commit into from

Conversation

janlauber
Copy link
Contributor

This PR faces the following issue:

It removes the securityContext templating implementation in the deployment.yaml at a place where it shouldn't belong.
Please checkout the issue and let me know if this PR works for you. I tested it locally and it works now and the replicaset can be created.

Signed-off-by: Jan Lauber jan.lauber@protonmail.ch

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 12, 2022

CLA Signed

The committers are authorized under a signed CLA.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 12, 2022
@k8s-ci-robot
Copy link
Contributor

Welcome @janlauber!

It looks like this is your first PR to kubernetes/dashboard 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/dashboard has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 12, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: janlauber
To complete the pull request process, please assign desaintmartin after the PR has been reviewed.
You can assign the PR to them by writing /assign @desaintmartin in a comment when ready.

The full list of commands accepted by this bot can be found 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

@janlauber janlauber changed the title remove securityContext seccomp on replicalevel Fix seccomp error on helmchart deployment Jan 12, 2022
@floreks
Copy link
Member

floreks commented Jan 12, 2022

This is not a correct way to fix this. securityContext can be set on the deployment level without any issues. If you do not need the securityContext to be set then modify the default values.

Most probably this is an issue with your Kubernetes cluster. You are using rancher and K8S v1.22 (which is not yet officialy supported by the Dashboard FYI). I think that seccomp is simply enabled inside your cluster and that's why you are unable to deploy the application with custom securityContext settings. You should either override it during deployment or update your seccomp configuration.

/close

@k8s-ci-robot
Copy link
Contributor

@floreks: Closed this PR.

In response to this:

This is not a correct way to fix this. securityContext can be set on the deployment level without any issues. If you do not need the securityContext to be set then modify the default values.

Most probably this is an issue with your Kubernetes cluster. You are using rancher and K8S v1.22 (which is not yet officialy supported by the Dashboard FYI). I think that seccomp is simply enabled inside your cluster and that's why you are unable to deploy the application with custom securityContext settings. You should either override it during deployment or update your seccomp configuration.

/close

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.

@janlauber
Copy link
Contributor Author

Hey @floreks
Thanks for your answer.

So there could be a helm value to disable the securityContexts in the deployment manifest?
Can I try to implement this?

@floreks
Copy link
Member

floreks commented Jan 12, 2022

Helm already provides a way to provide a custom values file or simply override a single value with the --set flag, right? Can't you just use that?

@janlauber
Copy link
Contributor Author

@floreks
I tried to do that, with the following values.yaml configuration but didn't work:

securityContext: {}

or

securityContext:
  seccompProfile:
    type: null

Now I've implemented a enabled: true value in the values.yaml file with an if statement of the block I've deleted in this closed PR. This works now for me pretty well and the default of securityContext is set to true, so there should nothing change for every other one using this helm chart. Just people with securityContext disabled in the cluster can now deploy it.
Would like to make a new PR to show this to you.
If this change is not welcome, please provide me an example to disable the securityContext with the Helm functionality you mentioned.

thanks and greez

@floreks
Copy link
Member

floreks commented Jan 12, 2022

Simply, remove the securityContext at all from your custom values.yaml file and it won't appear in the result deployment file.

@janlauber
Copy link
Contributor Author

@floreks
So I tried that but this doesn't work either.
When I get the deployment ressource with kubectl get deployments.apps kubernetes-dashboard-local -o yaml this is the output:

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "1"
    meta.helm.sh/release-name: kubernetes-dashboard-local
    meta.helm.sh/release-namespace: default
  creationTimestamp: "2022-01-12T16:09:34Z"
  generation: 1
  labels:
    app.kubernetes.io/component: kubernetes-dashboard
    app.kubernetes.io/instance: kubernetes-dashboard-local
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: kubernetes-dashboard
    app.kubernetes.io/version: 2.4.0
    helm.sh/chart: kubernetes-dashboard-5.1.0
  name: kubernetes-dashboard-local
  namespace: default
  resourceVersion: "1332552"
  uid: df7773bc-98a2-49ed-89ce-4dc2de9e4350
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app.kubernetes.io/component: kubernetes-dashboard
      app.kubernetes.io/instance: kubernetes-dashboard-local
      app.kubernetes.io/name: kubernetes-dashboard
  strategy:
    rollingUpdate:
      maxSurge: 0
      maxUnavailable: 1
    type: RollingUpdate
  template:
    metadata:
      creationTimestamp: null
      labels:
        app.kubernetes.io/component: kubernetes-dashboard
        app.kubernetes.io/instance: kubernetes-dashboard-local
        app.kubernetes.io/managed-by: Helm
        app.kubernetes.io/name: kubernetes-dashboard
        app.kubernetes.io/version: 2.4.0
        helm.sh/chart: kubernetes-dashboard-5.1.0
    spec:
      containers:
      - args:
        - --namespace=default
        - --auto-generate-certificates
        - --metrics-provider=none
        image: kubernetesui/dashboard:v2.4.0
        imagePullPolicy: IfNotPresent
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /
            port: 8443
            scheme: HTTPS
          initialDelaySeconds: 30
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 30
        name: kubernetes-dashboard
        ports:
        - containerPort: 8443
          name: https
          protocol: TCP
        resources:
          limits:
            cpu: "2"
            memory: 200Mi
          requests:
            cpu: 100m
            memory: 200Mi
        securityContext:
          allowPrivilegeEscalation: false
          readOnlyRootFilesystem: true
          runAsGroup: 2001
          runAsUser: 1001
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
        volumeMounts:
        - mountPath: /certs
          name: kubernetes-dashboard-certs
        - mountPath: /tmp
          name: tmp-volume
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: # here is the line who shouldn't get included, when not adding the securityContext value to the custom values.yaml
        seccompProfile:
          type: RuntimeDefault
      serviceAccount: kubernetes-dashboard-local
      serviceAccountName: kubernetes-dashboard-local
      terminationGracePeriodSeconds: 30
      volumes:
      - name: kubernetes-dashboard-certs
        secret:
          defaultMode: 420
          secretName: kubernetes-dashboard-local-certs
      - emptyDir: {}
        name: tmp-volume
status:
  conditions:
  - lastTransitionTime: "2022-01-12T16:09:34Z"
    lastUpdateTime: "2022-01-12T16:09:34Z"
    message: Created new replica set "kubernetes-dashboard-local-699c8b95fc"
    reason: NewReplicaSetCreated
    status: "True"
    type: Progressing
  - lastTransitionTime: "2022-01-12T16:09:34Z"
    lastUpdateTime: "2022-01-12T16:09:34Z"
    message: Deployment has minimum availability.
    reason: MinimumReplicasAvailable
    status: "True"
    type: Available
  - lastTransitionTime: "2022-01-12T16:09:34Z"
    lastUpdateTime: "2022-01-12T16:09:34Z"
    message: 'pods "kubernetes-dashboard-local-699c8b95fc-" is forbidden: PodSecurityPolicy:
      unable to admit pod: [pod.metadata.annotations[seccomp.security.alpha.kubernetes.io/pod]:
      Forbidden: seccomp may not be set pod.metadata.annotations[container.seccomp.security.alpha.kubernetes.io/kubernetes-dashboard]:
      Forbidden: seccomp may not be set]'
    reason: FailedCreate
    status: "True"
    type: ReplicaFailure
  observedGeneration: 1
  unavailableReplicas: 1

greez

@floreks
Copy link
Member

floreks commented Jan 12, 2022

Looks like it might be automatically set by the seccomp profile enabled inside your cluster. I don't remember all settings we are using but I am 90% sure now that it does not come from our config.

@janlauber
Copy link
Contributor Author

janlauber commented Jan 12, 2022

@floreks
So I deep dive analysed the rendering of the securityContext values and found a way to get it working when setting the following in the custom values.yaml file:

securityContext: null

So it is indeed your config of rendering this variables.
It has to be an if and not a with at the following line:

also the following line must contain the whole values path of rendering:

The changes are faced in a new PR.

greez

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants