diff --git a/internal/controller/kubecertagent/creater.go b/internal/controller/kubecertagent/creater.go index 5a213cd2..7f546a45 100644 --- a/internal/controller/kubecertagent/creater.go +++ b/internal/controller/kubecertagent/creater.go @@ -116,7 +116,7 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { return err } if agentPod == nil { - agentPod = newAgentPod(controllerManagerPod, c.agentPodConfig.PodTemplate()) + agentPod = c.agentPodConfig.newAgentPod(controllerManagerPod) klog.InfoS( "creating agent pod", diff --git a/internal/controller/kubecertagent/deleter.go b/internal/controller/kubecertagent/deleter.go index 992cd998..9aa120f1 100644 --- a/internal/controller/kubecertagent/deleter.go +++ b/internal/controller/kubecertagent/deleter.go @@ -70,7 +70,7 @@ func (c *deleterController) Sync(ctx controllerlib.Context) error { return err } if controllerManagerPod == nil || - !isAgentPodUpToDate(agentPod, newAgentPod(controllerManagerPod, c.agentPodConfig.PodTemplate())) { + !isAgentPodUpToDate(agentPod, c.agentPodConfig.newAgentPod(controllerManagerPod)) { klog.InfoS("deleting agent pod", "pod", klog.KObj(agentPod)) err := c.k8sClient. CoreV1(). diff --git a/internal/controller/kubecertagent/deleter_test.go b/internal/controller/kubecertagent/deleter_test.go index d8d528ca..6f5f2a28 100644 --- a/internal/controller/kubecertagent/deleter_test.go +++ b/internal/controller/kubecertagent/deleter_test.go @@ -264,7 +264,8 @@ func TestDeleterControllerSync(t *testing.T) { when("the agent pod is out of sync via automount service account token", func() { it.Before(func() { updatedAgentPod := agentPod.DeepCopy() - updatedAgentPod.Spec.AutomountServiceAccountToken = boolPtr(true) + t := true + updatedAgentPod.Spec.AutomountServiceAccountToken = &t r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) }) @@ -312,6 +313,59 @@ func TestDeleterControllerSync(t *testing.T) { }) }) + when("the agent pod is out of sync with the template via runAsUser", func() { + it.Before(func() { + updatedAgentPod := agentPod.DeepCopy() + notRoot := int64(1234) + updatedAgentPod.Spec.SecurityContext.RunAsUser = ¬Root + r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + }) + + it("deletes the agent pod", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + + r.NoError(err) + requireAgentPodWasDeleted() + }) + }) + + when("the agent pod is out of sync with the template via runAsGroup", func() { + it.Before(func() { + updatedAgentPod := agentPod.DeepCopy() + notRoot := int64(1234) + updatedAgentPod.Spec.SecurityContext.RunAsGroup = ¬Root + r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + }) + + it("deletes the agent pod", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + + r.NoError(err) + requireAgentPodWasDeleted() + }) + }) + + when("the agent pod is out of sync with the template via having a nil SecurityContext", func() { + it.Before(func() { + updatedAgentPod := agentPod.DeepCopy() + updatedAgentPod.Spec.SecurityContext = nil + r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) + }) + + it("deletes the agent pod", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + + r.NoError(err) + requireAgentPodWasDeleted() + }) + }) + when("the agent pod is out of sync with the template via labels", func() { when("an additional label's value was changed", func() { it.Before(func() { diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index e08d1dea..3d58b964 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -95,8 +95,11 @@ func (c *AgentPodConfig) AgentSelector() labels.Selector { return labels.SelectorFromSet(map[string]string{agentPodLabelKey: agentPodLabelValue}) } -func (c *AgentPodConfig) PodTemplate() *corev1.Pod { +func (c *AgentPodConfig) newAgentPod(controllerManagerPod *corev1.Pod) *corev1.Pod { terminateImmediately := int64(0) + rootID := int64(0) + f := false + falsePtr := &f imagePullSecrets := []corev1.LocalObjectReference{} for _, imagePullSecret := range c.ContainerImagePullSecrets { @@ -108,11 +111,15 @@ func (c *AgentPodConfig) PodTemplate() *corev1.Pod { ) } - pod := &corev1.Pod{ + return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: c.PodNamePrefix, + Name: fmt.Sprintf("%s%s", c.PodNamePrefix, hash(controllerManagerPod)), Namespace: c.Namespace, Labels: c.Labels(), + Annotations: map[string]string{ + controllerManagerNameAnnotationKey: controllerManagerPod.Name, + controllerManagerUIDAnnotationKey: string(controllerManagerPod.UID), + }, }, Spec: corev1.PodSpec{ TerminationGracePeriodSeconds: &terminateImmediately, @@ -123,6 +130,7 @@ func (c *AgentPodConfig) PodTemplate() *corev1.Pod { Image: c.ContainerImage, ImagePullPolicy: corev1.PullIfNotPresent, Command: []string{"/bin/sleep", "infinity"}, + VolumeMounts: controllerManagerPod.Spec.Containers[0].VolumeMounts, Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("16Mi"), @@ -135,44 +143,20 @@ func (c *AgentPodConfig) PodTemplate() *corev1.Pod { }, }, }, + Volumes: controllerManagerPod.Spec.Volumes, + RestartPolicy: corev1.RestartPolicyNever, + NodeSelector: controllerManagerPod.Spec.NodeSelector, + AutomountServiceAccountToken: falsePtr, + NodeName: controllerManagerPod.Spec.NodeName, + Tolerations: controllerManagerPod.Spec.Tolerations, + // We need to run the agent pod as root since the file permissions + // on the cluster keypair usually restricts access to only root. + SecurityContext: &corev1.PodSecurityContext{ + RunAsUser: &rootID, + RunAsGroup: &rootID, + }, }, } - return pod -} - -func newAgentPod( - controllerManagerPod *corev1.Pod, - template *corev1.Pod, -) *corev1.Pod { - agentPod := template.DeepCopy() - - agentPod.Name = fmt.Sprintf("%s%s", agentPod.Name, hash(controllerManagerPod)) - - // It would be nice to use the OwnerReferences field here, but the agent pod is most likely in a - // different namespace than the kube-controller-manager pod, and therefore that breaks the - // OwnerReferences contract (see metav1.OwnerReference doc). - if agentPod.Annotations == nil { - agentPod.Annotations = make(map[string]string) - } - agentPod.Annotations[controllerManagerNameAnnotationKey] = controllerManagerPod.Name - agentPod.Annotations[controllerManagerUIDAnnotationKey] = string(controllerManagerPod.UID) - - // We need to run the agent pod as root since the file permissions on the cluster keypair usually - // restricts access to only root. - rootID := int64(0) - agentPod.Spec.Containers[0].VolumeMounts = controllerManagerPod.Spec.Containers[0].VolumeMounts - agentPod.Spec.Volumes = controllerManagerPod.Spec.Volumes - agentPod.Spec.RestartPolicy = corev1.RestartPolicyNever - agentPod.Spec.NodeSelector = controllerManagerPod.Spec.NodeSelector - agentPod.Spec.AutomountServiceAccountToken = boolPtr(false) - agentPod.Spec.NodeName = controllerManagerPod.Spec.NodeName - agentPod.Spec.Tolerations = controllerManagerPod.Spec.Tolerations - agentPod.Spec.SecurityContext = &corev1.PodSecurityContext{ - RunAsUser: &rootID, - RunAsGroup: &rootID, - } - - return agentPod } func isAgentPodUpToDate(actualAgentPod, expectedAgentPod *corev1.Pod) bool { @@ -346,8 +330,6 @@ func strategyError(clock clock.Clock, err error) configv1alpha1.CredentialIssuer } } -func boolPtr(b bool) *bool { return &b } - func hash(controllerManagerPod *corev1.Pod) string { // FNV should be faster than SHA, and we don't care about hash-reversibility here, and Kubernetes // uses FNV for their pod templates, so should be good enough for us? diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 26011dde..a0dc704e 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -70,6 +70,7 @@ func exampleControllerManagerAndAgentPods( } zero := int64(0) + f := false // fnv 32a hash of controller-manager uid controllerManagerPodHash := "fbb0addd" @@ -114,10 +115,11 @@ func exampleControllerManagerAndAgentPods( }, }, RestartPolicy: corev1.RestartPolicyNever, - AutomountServiceAccountToken: boolPtr(false), + AutomountServiceAccountToken: &f, NodeName: controllerManagerPod.Spec.NodeName, NodeSelector: controllerManagerPod.Spec.NodeSelector, Tolerations: controllerManagerPod.Spec.Tolerations, + SecurityContext: &corev1.PodSecurityContext{RunAsUser: &zero, RunAsGroup: &zero}, }, }