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 the kubelet root directory configurable #1438

Merged
merged 8 commits into from
Jan 31, 2023
4 changes: 2 additions & 2 deletions api/seccompprofile/v1beta1/seccompprofile_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (sp *SeccompProfile) DeepCopyToStatusBaseIf() profilebase.StatusBaseUser {

func (sp *SeccompProfile) SetImplementationStatus() {
profilePath := sp.GetProfilePath()
sp.Status.LocalhostProfile = strings.TrimPrefix(profilePath, config.KubeletSeccompRootPath+"/")
sp.Status.LocalhostProfile = strings.TrimPrefix(profilePath, config.KubeletSeccompRootPath()+"/")
}

func (sp *SeccompProfile) GetProfileFile() string {
Expand All @@ -163,7 +163,7 @@ func (sp *SeccompProfile) GetProfileFile() string {
func (sp *SeccompProfile) GetProfilePath() string {
pfile := sp.GetProfileFile()
return path.Join(
config.ProfilesRootPath,
config.ProfilesRootPath(),
filepath.Base(sp.GetNamespace()),
filepath.Base(pfile),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: gcr.io/k8s-staging-sp-operator/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
2 changes: 2 additions & 0 deletions deploy/kustomize-deployment/manager_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: "/var/lib/kubelet"
tolerations:
- key: "node-role.kubernetes.io/master"
operator: "Exists"
Expand Down
2 changes: 2 additions & 0 deletions deploy/namespace-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2983,6 +2983,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: gcr.io/k8s-staging-sp-operator/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
2 changes: 2 additions & 0 deletions deploy/openshift-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: image-registry.openshift-image-registry.svc:5000/openshift/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
2 changes: 2 additions & 0 deletions deploy/openshift-downstream.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2994,6 +2994,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: gcr.io/k8s-staging-sp-operator/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
2 changes: 2 additions & 0 deletions deploy/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: gcr.io/k8s-staging-sp-operator/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
2 changes: 2 additions & 0 deletions deploy/webhook-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBELET_DIR
value: /var/lib/kubelet
image: gcr.io/k8s-staging-sp-operator/security-profiles-operator:latest
imagePullPolicy: Always
name: security-profiles-operator
Expand Down
7 changes: 7 additions & 0 deletions installation-usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- [Installation using OLM using upstream catalog and bundle](#installation-using-olm-using-upstream-catalog-and-bundle)
- [Installation using helm](#installation-using-helm)
- [Installation on AKS](#installation-on-aks)
- [Configure a custom kubelet root directory](#configure-a-custom-kubelet-root-directory)
- [Set logging verbosity](#set-logging-verbosity)
- [Pull images from private registry](#pull-images-from-private-registry)
- [Configure the SELinux type](#configure-the-selinux-type)
Expand Down Expand Up @@ -189,6 +190,12 @@ NAME STATE
spod RUNNING
```

## Configure a custom kubelet root directory

You can configure a custom kubelet root directory in case your cluster is not using the default `/var/lib/kubelet` path.
You can achieve this by setting the environment variable `KUBELET_DIR` in the operator deployment. This environment variable will
be then set in the manager container as well as it will be propagated into the containers part of spod daemonset.
Comment on lines +193 to +197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better / possible to have this as part of the SPOD instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The challenge is that the kubelet path is required in the non-root enabler sidecar which currently doesn't read any SPOD CRD before copying the profiles into the kubelet directory.

It seems to be used inside of nodestatus controller as well.

The idea is to have this environment variable propagated everywhere that all components used the same custom kubelet directory including both manager and daemon instances.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, make sense. Any further thoughts on that @kubernetes-sigs/security-profiles-operator-maintainers ?

Copy link
Member

@pjbgf pjbgf Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relying on env-vars would make it a lot easier to enable support for non symmetrical configuration across node pools. +1 from me on the approach.


## Set logging verbosity

The operator supports the default logging verbosity of `0` and an enhanced `1`.
Expand Down
35 changes: 28 additions & 7 deletions internal/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package config
import (
"errors"
"os"
"path"
"path/filepath"

"sigs.k8s.io/release-utils/env"
Expand All @@ -44,13 +45,8 @@ const (
// UserRootless is the user which runs the operator.
UserRootless = 65535

// KubeletSeccompRootPath specifies the path where all kubelet seccomp
// profiles are stored.
KubeletSeccompRootPath = "/var/lib/kubelet/seccomp"

// ProfilesRootPath specifies the path where the operator stores seccomp
// profiles.
ProfilesRootPath = KubeletSeccompRootPath + "/operator"
// DefaultKubeletPath specifies the default kubelet path.
DefaultKubeletPath = "/var/lib/kubelet"

// NodeNameEnvKey is the default environment variable key for retrieving
// the name of the current node.
Expand Down Expand Up @@ -84,6 +80,9 @@ const (
// profiling port.
ProfilingPortEnvKey = "SPO_PROFILING_PORT"

// KubeletDirEnvKey is the environment variable key for custom kubelet directory.
KubeletDirEnvKey = "KUBELET_DIR"

// DefaultProfilingPort is the start port where the profiling endpoint runs.
DefaultProfilingPort = 6060

Expand Down Expand Up @@ -155,3 +154,25 @@ func TryToGetOperatorNamespace() (string, error) {
}
return operatorNS, nil
}

// KubeletDir returns the kubelet directory either form an environment variable
// when is set or the default Kubernetes path.
func KubeletDir() string {
kubeletDir := env.Default(KubeletDirEnvKey, "")
if kubeletDir == "" {
return DefaultKubeletPath
}
return kubeletDir
}

// KubeletSeccompRootPath specifies the path where all kubelet seccomp
// profiles are stored.
func KubeletSeccompRootPath() string {
return path.Join(KubeletDir(), "seccomp")
}

// ProfilesRootPath specifies the path where the operator stores seccomp
// profiles.
func ProfilesRootPath() string {
return path.Join(KubeletSeccompRootPath(), "operator")
}
10 changes: 5 additions & 5 deletions internal/pkg/daemon/seccompprofile/seccompprofile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestGetProfilePath(t *testing.T) {
}{
{
name: "AppendNamespaceAndProfile",
want: path.Join(config.ProfilesRootPath, "config-namespace", "file.json"),
want: path.Join(config.ProfilesRootPath(), "config-namespace", "file.json"),
sp: &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: "file.json",
Expand All @@ -220,7 +220,7 @@ func TestGetProfilePath(t *testing.T) {
},
{
name: "BlockTraversalAtProfileName",
want: path.Join(config.ProfilesRootPath, "ns", "file.json"),
want: path.Join(config.ProfilesRootPath(), "ns", "file.json"),
sp: &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: "../../../../../file.json",
Expand All @@ -230,7 +230,7 @@ func TestGetProfilePath(t *testing.T) {
},
{
name: "BlockTraversalAtTargetName",
want: path.Join(config.ProfilesRootPath, "ns", "file.json"),
want: path.Join(config.ProfilesRootPath(), "ns", "file.json"),
sp: &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: "file.json",
Expand All @@ -240,7 +240,7 @@ func TestGetProfilePath(t *testing.T) {
},
{
name: "BlockTraversalAtSPNamespace",
want: path.Join(config.ProfilesRootPath, "ns", "file.json"),
want: path.Join(config.ProfilesRootPath(), "ns", "file.json"),
sp: &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: "file.json",
Expand All @@ -250,7 +250,7 @@ func TestGetProfilePath(t *testing.T) {
},
{
name: "AppendExtension",
want: path.Join(config.ProfilesRootPath, "config-namespace", "file.json"),
want: path.Join(config.ProfilesRootPath(), "config-namespace", "file.json"),
sp: &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: "file",
Expand Down
32 changes: 31 additions & 1 deletion internal/pkg/manager/spod/bindata/spod.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ var Manifest = &appsv1.DaemonSet{
corev1.ResourceEphemeralStorage: resource.MustParse("50Mi"),
},
},
Env: []corev1.EnvVar{
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
},
{
Name: SelinuxPoliciesCopierContainerName,
Expand Down Expand Up @@ -266,6 +272,12 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
corev1.ResourceEphemeralStorage: resource.MustParse("50Mi"),
},
},
Env: []corev1.EnvVar{
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
},
},
Containers: []corev1.Container{
Expand All @@ -276,7 +288,7 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
VolumeMounts: []corev1.VolumeMount{
{
Name: "host-operator-volume",
MountPath: config.ProfilesRootPath,
MountPath: config.ProfilesRootPath(),
},
{
Name: "selinux-drop-dir",
Expand Down Expand Up @@ -345,6 +357,10 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
Name: config.SPOdNameEnvKey,
Value: config.SPOdName,
},
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
Ports: []corev1.ContainerPort{
{
Expand Down Expand Up @@ -432,6 +448,12 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
corev1.ResourceEphemeralStorage: resource.MustParse("400Mi"),
},
},
Env: []corev1.EnvVar{
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
},
{
Name: LogEnricherContainerName,
Expand Down Expand Up @@ -483,6 +505,10 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
},
},
},
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
},
{
Expand Down Expand Up @@ -538,6 +564,10 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil
},
},
},
{
Name: config.KubeletDirEnvKey,
Value: config.KubeletDir(),
},
},
},
{
Expand Down
16 changes: 8 additions & 8 deletions internal/pkg/nonrootenabler/nonrootenabler.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ func (n *NonRootEnabler) Run(logger logr.Logger, runtime string) error {

logger.Info("Container runtime:" + runtime)

logger.Info("Ensuring seccomp root path: " + config.KubeletSeccompRootPath)
logger.Info("Ensuring seccomp root path: " + config.KubeletSeccompRootPath())
if err := n.impl.MkdirAll(
config.KubeletSeccompRootPath, dirPermissions,
config.KubeletSeccompRootPath(), dirPermissions,
); err != nil {
return fmt.Errorf(
"create seccomp root path %s: %w",
config.KubeletSeccompRootPath, err,
config.KubeletSeccompRootPath(), err,
)
}

Expand All @@ -64,7 +64,7 @@ func (n *NonRootEnabler) Run(logger logr.Logger, runtime string) error {
); err != nil {
return fmt.Errorf(
"create operator root path %s: %w",
config.KubeletSeccompRootPath, err,
config.KubeletSeccompRootPath(), err,
)
}

Expand All @@ -74,10 +74,10 @@ func (n *NonRootEnabler) Run(logger logr.Logger, runtime string) error {
return fmt.Errorf("change operator root path permissions: %w", err)
}

if _, err := n.impl.Stat(config.ProfilesRootPath); os.IsNotExist(err) {
if _, err := n.impl.Stat(config.ProfilesRootPath()); os.IsNotExist(err) {
logger.Info("Linking profiles root path")
if err := n.impl.Symlink(
config.OperatorRoot, config.ProfilesRootPath,
config.OperatorRoot, config.ProfilesRootPath(),
); err != nil {
return fmt.Errorf("link profiles root path: %w", err)
}
Expand All @@ -91,9 +91,9 @@ func (n *NonRootEnabler) Run(logger logr.Logger, runtime string) error {
}

kubeletSeccompRootPath := config.KubeletSeccompRootPath
logger.Info("Copying profiles into root path: " + kubeletSeccompRootPath)
logger.Info("Copying profiles into root path: " + kubeletSeccompRootPath())
if err := n.impl.CopyDirContentsLocal(
"/opt/spo-profiles", kubeletSeccompRootPath,
"/opt/spo-profiles", kubeletSeccompRootPath(),
); err != nil {
return fmt.Errorf("copy local security profiles: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion test/tc_allowed_syscalls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (e *e2e) testCaseAllowedSyscallsValidation(nodes []string) {
// the dev container where the tests are executed. This check needs to be skipped.
if e.nodeRootfsPrefix == "" {
statOutput := e.execNode(
node, "stat", "-L", "-c", `%a,%u,%g`, config.ProfilesRootPath,
node, "stat", "-L", "-c", `%a,%u,%g`, config.ProfilesRootPath(),
)
e.Contains(statOutput, "744,65535,65535")

Expand Down
2 changes: 1 addition & 1 deletion test/tc_default_profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (e *e2e) testCaseDefaultAndExampleProfiles(nodes []string) {
// the dev container where the tests are executed. This check needs to be skipped.
if e.nodeRootfsPrefix == "" {
statOutput := e.execNode(
node, "stat", "-L", "-c", `%a,%u,%g`, config.ProfilesRootPath,
node, "stat", "-L", "-c", `%a,%u,%g`, config.ProfilesRootPath(),
)
e.Contains(statOutput, "744,65535,65535")

Expand Down