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

Make security-profiles-operator daemonset configurable #327

Closed
voigt opened this issue Feb 23, 2021 · 7 comments · Fixed by #336
Closed

Make security-profiles-operator daemonset configurable #327

voigt opened this issue Feb 23, 2021 · 7 comments · Fixed by #336
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@voigt
Copy link

voigt commented Feb 23, 2021

What would you like to be added:

A mechanism to make the creation of the security-profiles-operator daemonset configurable. At this point in time, the entire Daemonset is hardcoded.

Why is this needed:

As an infrastructure SRE there are a couple of scenarios, where I'd like to change the configuration of the daemonset:

  • change container image source
  • change nodeselectors and tolerations
  • change resource requests and limits
  • securitycontexts
  • ... you name it

User story covered

As an infrastructure SRE, Kirsti wants to make sure that common profiles are available to all users, so folks can deploy applications securely. ↗️

@voigt voigt added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 23, 2021
@voigt
Copy link
Author

voigt commented Feb 25, 2021

Are there any opinion how DS configuration should look like?

Here are some thoughts to get the discussion started:

If I get it correctly, there are different daemonsets per use case, which are created at runtime:

  • seccomp-spod
  • selinux-spod
  • recorder-spod
  • and in future something like apparmor-spod

I think it should be possible to configure them independently from each other. This increases flexibility for clusters with inhomogeneous nodes (e.g. some of them have AppArmor activated, some do not).

As the whole story is only about configuration on cluster resource level, I don't think a detailed configuration via flags makes sense here. Therefore I'd suggest a flag for the operator like --config, -C or --resource-config, -R which optionally references a yaml file.

The yaml file then could follow (more or less) the resource spec:

seccomp-spod:
  spec:
    template:
      spec:
        tolerations:
        - effect: NoSchedule
          key: role
          operator: Equal
          value: infra
---
selinux-spod:
  spec:
    template:
      containers:
        - args:
            - [...]
          image: [...]
          imagePullPolicy: Always
---
recorder-spod:
...
---
apparmor-spod:
...

I assume this would be straight forward to parse and apply and would give a lot of freedom about the DS.

Though, with this approach comes a drawback: it would be relatively easy to break the things or make them insecure (e.g. spec.template.spec.containers.securityContext)...

Just some thoughts. Happy to read about your opinion!

@JAORMX
Copy link
Contributor

JAORMX commented Feb 25, 2021

@voigt that's an interesting point; do you have a inhomogeneous nodes in your environment?

One thing to note is that Seccomp tends to be a constant, so the combinations would normally be Seccomp + SELinux or Seccomp + Apparmor; perhaps the way it could be arranged is that we could move the configuration of the SPO to be a CRD (SecurityProfilesDaemon?), and each instance of the daemon would deploy an instance of the spod daemonset (with the name matching the name given to the Custom Resource); This way, one could configure each instance as needed.

@jhrozek
Copy link
Contributor

jhrozek commented Feb 25, 2021

One aspect that I think would be nice to have configurable is the resource requests and limits. Especially for selinuxd which is a resource hog, I'm afraid that if someone runs into the limits, they'd just be out of luck..

@saschagrunert
Copy link
Member

saschagrunert commented Mar 1, 2021

Yeah I think the general configuration approach seems reasonable to me. I assume that the configuration would live in a separate config map? Or the same config map we're using right now?

---
apiVersion: v1
kind: ConfigMap
metadata:
name: config
namespace: security-profiles-operator
labels:
security-profiles-operator/config: ""
data:
EnableSelinux: "false"
EnableLogEnricher: "false"

@JAORMX
Copy link
Contributor

JAORMX commented Mar 1, 2021

@saschagrunert I was more thinking along the lines of moving the ConfigMap to be a CRD, so the configuration would live there, and the same CRD would report the status.

@saschagrunert
Copy link
Member

@saschagrunert I was more thinking along the lines of moving the ConfigMap to be a CRD, so the configuration would live there, and the same CRD would report the status.

Sounds good to me!

@JAORMX
Copy link
Contributor

JAORMX commented Mar 3, 2021

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants