Skip to content

Commit

Permalink
Configure whether to set workload pod seccompProfile
Browse files Browse the repository at this point in the history 
PR #3156 changes the
definition of statefulset/job by setting the seccomp profile on the
statefulset/job template spec. This causes all workload pods to get
automatically restarted when upgrading from previous Korifi releases.

We introduce new `statefulsetRunnerTemporarySetPodSeccompProfile` and
`jobTaskRunnerTemporarySetPodSeccompProfile` helm values whose default
value would prevent altering existing statefulsets and jobs.

Those will be probably removed in future releases.

Co-authored-by: Danail Branekov <danailster@gmail.com>
  • Loading branch information
@georgethebeatle @danail-branekov
georgethebeatle and danail-branekov committed Mar 12, 2024
1 parent da2d312 commit 26525c5
Show file tree
Hide file tree
Showing 12 changed files with 125 additions and 49 deletions.
6 changes: 5 additions & 1 deletion controllers/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ type ControllerConfig struct {
SpaceFinalizerAppDeletionTimeout *int64 `yaml:"spaceFinalizerAppDeletionTimeout"`

// job-task-runner
JobTTL string `yaml:"jobTTL"`
JobTTL string `yaml:"jobTTL"`
JobTaskRunnerTemporarySetPodSeccompProfile bool `yaml:"jobTaskRunnerTemporarySetPodSeccompProfile"`

// statefulset-runner
StatefulsetRunnerTemporarySetPodSeccompProfile bool `yaml:"statefulsetRunnerTemporarySetPodSeccompProfile"`

// kpack-image-builder
ClusterBuilderName string `yaml:"clusterBuilderName"`
Expand Down
6 changes: 5 additions & 1 deletion controllers/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ func main() {
mgr.GetScheme(),
jobtaskrunnercontrollers.NewStatusGetter(logger, mgr.GetClient()),
jobTTL,
controllerConfig.JobTaskRunnerTemporarySetPodSeccompProfile,
)
if err = taskWorkloadReconciler.SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "TaskWorkload")
Expand All @@ -361,7 +362,10 @@ func main() {
if err = statefulsetcontrollers.NewAppWorkloadReconciler(
mgr.GetClient(),
mgr.GetScheme(),
statefulsetcontrollers.NewAppWorkloadToStatefulsetConverter(mgr.GetScheme()),
statefulsetcontrollers.NewAppWorkloadToStatefulsetConverter(
mgr.GetScheme(),
controllerConfig.StatefulsetRunnerTemporarySetPodSeccompProfile,
),
statefulsetcontrollers.NewPDBUpdater(mgr.GetClient()),
logger,
).SetupWithManager(mgr); err != nil {
Expand Down
4 changes: 4 additions & 0 deletions helm/korifi/controllers/configmap.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ data:
{{- end }}
{{- if .Values.jobTaskRunner.include }}
jobTTL: {{ required "jobTTL is required" .Values.jobTaskRunner.jobTTL }}
jobTaskRunnerTemporarySetPodSeccompProfile: {{ .Values.jobTaskRunner.temporarySetPodSeccompProfile }}
{{- end }}
{{- if .Values.statefulsetRunner.include }}
statefulsetRunnerTemporarySetPodSeccompProfile: {{ .Values.statefulsetRunner.temporarySetPodSeccompProfile }}
{{- end }}
networking:
gatewayNamespace: {{ .Release.Namespace }}-gateway
Expand Down
8 changes: 8 additions & 0 deletions helm/korifi/values.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,10 @@
"description": "Number of replicas.",
"type": "integer"
},
"temporarySetPodSeccompProfile": {
"description": "Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.",
"type": "boolean"
},
"resources": {
"description": "[`ResourceRequirements`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#resourcerequirements-v1-core) for the API.",
"type": "object",
Expand Down Expand Up @@ -440,6 +444,10 @@
"description": "Number of replicas.",
"type": "integer"
},
"temporarySetPodSeccompProfile": {
"description": "Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.",
"type": "boolean"
},
"resources": {
"description": "[`ResourceRequirements`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#resourcerequirements-v1-core) for the API.",
"type": "object",
Expand Down
2 changes: 2 additions & 0 deletions helm/korifi/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ kpackImageBuilder:
statefulsetRunner:
include: true
replicas: 1
temporarySetPodSeccompProfile: false
resources:
limits:
cpu: 500m
Expand All @@ -109,6 +110,7 @@ statefulsetRunner:
jobTaskRunner:
include: true
replicas: 1
temporarySetPodSeccompProfile: false
resources:
limits:
cpu: 500m
Expand Down
1 change: 1 addition & 0 deletions job-task-runner/controllers/integration/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ var _ = BeforeSuite(func() {
k8sManager.GetScheme(),
controllers.NewStatusGetter(logger, k8sManager.GetClient()),
time.Minute,
false,
)
err = taskWorkloadReconciler.SetupWithManager(k8sManager)
Expect(err).NotTo(HaveOccurred())
Expand Down
3 changes: 0 additions & 3 deletions job-task-runner/controllers/integration/taskworkload_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ var _ = Describe("Job TaskWorkload Controller Integration Test", func() {
Drop: []corev1.Capability{"ALL"},
},
AllowPrivilegeEscalation: tools.PtrTo(false),
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}))
Expect(podSpec.ServiceAccountName).To(Equal("korifi-task"))
})
Expand Down
38 changes: 21 additions & 17 deletions job-task-runner/controllers/taskworkload_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ type TaskStatusGetter interface {

// TaskWorkloadReconciler reconciles a TaskWorkload object
type TaskWorkloadReconciler struct {
k8sClient client.Client
logger logr.Logger
scheme *runtime.Scheme
statusGetter TaskStatusGetter
jobTTL time.Duration
k8sClient client.Client
logger logr.Logger
scheme *runtime.Scheme
statusGetter TaskStatusGetter
jobTTL time.Duration
jobTaskRunnerTemporarySetPodSeccompProfile bool
}

func NewTaskWorkloadReconciler(
Expand All @@ -64,13 +65,15 @@ func NewTaskWorkloadReconciler(
scheme *runtime.Scheme,
statusGetter TaskStatusGetter,
jobTTL time.Duration,
jobTaskRunnerTemporarySetPodSeccompProfile bool,
) *k8s.PatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload] {
taskReconciler := TaskWorkloadReconciler{
k8sClient: k8sClient,
logger: logger,
scheme: scheme,
statusGetter: statusGetter,
jobTTL: jobTTL,
jobTaskRunnerTemporarySetPodSeccompProfile: jobTaskRunnerTemporarySetPodSeccompProfile,
}

return k8s.NewPatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload](logger, k8sClient, &taskReconciler)
Expand Down Expand Up @@ -132,9 +135,9 @@ func (r TaskWorkloadReconciler) getOrCreateJob(ctx context.Context, logger logr.
}

func (r TaskWorkloadReconciler) createJob(ctx context.Context, logger logr.Logger, taskWorkload *korifiv1alpha1.TaskWorkload) (*batchv1.Job, error) {
job, err := r.workloadToJob(taskWorkload)
job := WorkloadToJob(taskWorkload, int32(r.jobTTL.Seconds()), r.jobTaskRunnerTemporarySetPodSeccompProfile)
err := controllerutil.SetControllerReference(taskWorkload, job, r.scheme)
if err != nil {
logger.Info("failed to convert task workload to job", "reason", err)
return nil, err
}

Expand All @@ -151,7 +154,11 @@ func (r TaskWorkloadReconciler) createJob(ctx context.Context, logger logr.Logge
return job, nil
}

func (r *TaskWorkloadReconciler) workloadToJob(taskWorkload *korifiv1alpha1.TaskWorkload) (*batchv1.Job, error) {
func WorkloadToJob(
taskWorkload *korifiv1alpha1.TaskWorkload,
jobTTL int32,
jobTaskRunnerTemporarySetPodSeccompProfile bool,
) *batchv1.Job {
job := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Name: taskWorkload.Name,
Expand All @@ -161,15 +168,12 @@ func (r *TaskWorkloadReconciler) workloadToJob(taskWorkload *korifiv1alpha1.Task
BackoffLimit: tools.PtrTo(int32(0)),
Parallelism: tools.PtrTo(int32(1)),
Completions: tools.PtrTo(int32(1)),
TTLSecondsAfterFinished: tools.PtrTo(int32(r.jobTTL.Seconds())),
TTLSecondsAfterFinished: tools.PtrTo(jobTTL),
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: tools.PtrTo(true),
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
},
AutomountServiceAccountToken: tools.PtrTo(false),
ImagePullSecrets: taskWorkload.Spec.ImagePullSecrets,
Expand All @@ -195,12 +199,12 @@ func (r *TaskWorkloadReconciler) workloadToJob(taskWorkload *korifiv1alpha1.Task
},
}

err := controllerutil.SetControllerReference(taskWorkload, job, r.scheme)
if err != nil {
return nil, err
if jobTaskRunnerTemporarySetPodSeccompProfile {
job.Spec.Template.Spec.SecurityContext.SeccompProfile = &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
}
}

return job, nil
return job
}

func (r *TaskWorkloadReconciler) updateTaskWorkloadStatus(ctx context.Context, taskWorkload *korifiv1alpha1.TaskWorkload, job *batchv1.Job) error {
Expand Down
54 changes: 42 additions & 12 deletions job-task-runner/controllers/taskworkload_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (
korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
"code.cloudfoundry.org/korifi/job-task-runner/controllers"
"code.cloudfoundry.org/korifi/job-task-runner/controllers/fake"
"code.cloudfoundry.org/korifi/tools/k8s"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -27,16 +27,16 @@ var _ = Describe("TaskworkloadController", func() {
var (
statusGetter *fake.TaskStatusGetter

reconciler *k8s.PatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload]
reconcileResult ctrl.Result
reconcileErr error
req ctrl.Request
taskWorkload *korifiv1alpha1.TaskWorkload
getTaskWorkloadError error
createdJob *batchv1.Job
existingJob *batchv1.Job
getExistingJobError error
createJobError error
reconcileResult ctrl.Result
reconcileErr error
req ctrl.Request
taskWorkload *korifiv1alpha1.TaskWorkload
getTaskWorkloadError error
createdJob *batchv1.Job
existingJob *batchv1.Job
getExistingJobError error
createJobError error
jobTaskRunnerTemporarySetPodSeccompProfile bool
)

BeforeEach(func() {
Expand Down Expand Up @@ -98,12 +98,13 @@ var _ = Describe("TaskworkloadController", func() {
Reason: "something",
}}, nil)

reconciler = controllers.NewTaskWorkloadReconciler(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), fakeClient, scheme.Scheme, statusGetter, time.Hour)
jobTaskRunnerTemporarySetPodSeccompProfile = false

req = ctrl.Request{NamespacedName: client.ObjectKeyFromObject(taskWorkload)}
})

JustBeforeEach(func() {
reconciler := controllers.NewTaskWorkloadReconciler(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), fakeClient, scheme.Scheme, statusGetter, time.Hour, jobTaskRunnerTemporarySetPodSeccompProfile)
reconcileResult, reconcileErr = reconciler.Reconcile(context.Background(), req)
})

Expand Down Expand Up @@ -224,4 +225,33 @@ var _ = Describe("TaskworkloadController", func() {
Expect(reconcileErr).To(MatchError(ContainSubstring("get-conditions-error")))
})
})

Describe("jobTaskRunnerTemporarySetPodSeccompProfile", func() {
var (
job *batchv1.Job
jobTaskRunnerTemporarySetPodSeccompProfile bool
)

BeforeEach(func() {
jobTaskRunnerTemporarySetPodSeccompProfile = false
})

JustBeforeEach(func() {
job = controllers.WorkloadToJob(taskWorkload, 123, jobTaskRunnerTemporarySetPodSeccompProfile)
})

It("does not set spec.securityContext.seccompProfile", func() {
Expect(job.Spec.Template.Spec.SecurityContext.SeccompProfile).To(BeNil())
})

When("jobTaskRunnerTemporarySetPodSeccompProfile is set to true", func() {
BeforeEach(func() {
jobTaskRunnerTemporarySetPodSeccompProfile = true
})

It("sets spec.securityContext.seccompProfile to RuntimeDefault", func() {
Expect(job.Spec.Template.Spec.SecurityContext.SeccompProfile.Type).To(Equal(corev1.SeccompProfileTypeRuntimeDefault))
})
})
})
})
18 changes: 13 additions & 5 deletions statefulset-runner/controllers/appworkload_to_stset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@ import (
)

type AppWorkloadToStatefulsetConverter struct {
scheme *runtime.Scheme
scheme *runtime.Scheme
statefulsetRunnerTemporarySetPodSeccompProfile bool
}

func NewAppWorkloadToStatefulsetConverter(scheme *runtime.Scheme) *AppWorkloadToStatefulsetConverter {
func NewAppWorkloadToStatefulsetConverter(
scheme *runtime.Scheme,
statefulsetRunnerTemporarySetPodSeccompProfile bool,
) *AppWorkloadToStatefulsetConverter {
return &AppWorkloadToStatefulsetConverter{
scheme: scheme,
statefulsetRunnerTemporarySetPodSeccompProfile: statefulsetRunnerTemporarySetPodSeccompProfile,
}
}

Expand Down Expand Up @@ -136,16 +141,19 @@ func (r *AppWorkloadToStatefulsetConverter) Convert(appWorkload *korifiv1alpha1.
ImagePullSecrets: appWorkload.Spec.ImagePullSecrets,
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: tools.PtrTo(true),
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
},
ServiceAccountName: ServiceAccountName,
},
},
},
}

if r.statefulsetRunnerTemporarySetPodSeccompProfile {
statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile = &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
}
}

statefulSet.Spec.Template.Spec.AutomountServiceAccountToken = tools.PtrTo(false)
statefulSet.Spec.Selector = statefulSetLabelSelector(appWorkload)

Expand Down
32 changes: 23 additions & 9 deletions statefulset-runner/controllers/appworkload_to_stset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,24 @@ import (

var _ = Describe("AppWorkload to StatefulSet Converter", func() {
var (
statefulSet *appsv1.StatefulSet
appWorkload *korifiv1alpha1.AppWorkload
converter *controllers.AppWorkloadToStatefulsetConverter
statefulSet *appsv1.StatefulSet
appWorkload *korifiv1alpha1.AppWorkload
converter *controllers.AppWorkloadToStatefulsetConverter
statefulsetRunnerTemporarySetPodSeccompProfile bool
)

BeforeEach(func() {
Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
appWorkload = createAppWorkload("some-namespace", "guid_1234")
converter = controllers.NewAppWorkloadToStatefulsetConverter(scheme.Scheme)
statefulsetRunnerTemporarySetPodSeccompProfile = false
})

JustBeforeEach(func() {
var err error
converter = controllers.NewAppWorkloadToStatefulsetConverter(
scheme.Scheme,
statefulsetRunnerTemporarySetPodSeccompProfile,
)
statefulSet, err = converter.Convert(appWorkload)

Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -212,11 +217,6 @@ var _ = Describe("AppWorkload to StatefulSet Converter", func() {
Expect(*statefulSet.Spec.Template.Spec.SecurityContext.RunAsNonRoot).To(BeTrue())
})

It("should set the seccomp profile on the pod", func() {
Expect(statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile).NotTo(BeNil())
Expect(*statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile).To(Equal(corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}))
})

It("should set soft inter-pod anti-affinity", func() {
podAntiAffinity := statefulSet.Spec.Template.Spec.Affinity.PodAntiAffinity
Expect(podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To(BeEmpty())
Expand Down Expand Up @@ -321,4 +321,18 @@ var _ = Describe("AppWorkload to StatefulSet Converter", func() {
})
}
})

It("does not set spec.securityContext.seccompProfile", func() {
Expect(statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile).To(BeNil())
})

When("statefulsetRunnerTemporarySetPodSeccompProfile is set to true", func() {
BeforeEach(func() {
statefulsetRunnerTemporarySetPodSeccompProfile = true
})

It("sets spec.securityContext.seccompProfile to RuntimeDefault", func() {
Expect(statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile.Type).To(Equal(corev1.SeccompProfileTypeRuntimeDefault))
})
})
})
2 changes: 1 addition & 1 deletion statefulset-runner/controllers/integration/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ var _ = BeforeSuite(func() {
appWorkloadReconciler := NewAppWorkloadReconciler(
k8sManager.GetClient(),
k8sManager.GetScheme(),
NewAppWorkloadToStatefulsetConverter(k8sManager.GetScheme()),
NewAppWorkloadToStatefulsetConverter(k8sManager.GetScheme(), false),
NewPDBUpdater(k8sManager.GetClient()),
logger,
)
Expand Down

0 comments on commit 26525c5

Please sign in to comment.