diff --git a/internal/controller/kubecertagent/annotater.go b/internal/controller/kubecertagent/annotater.go index 42713f00..db9670ac 100644 --- a/internal/controller/kubecertagent/annotater.go +++ b/internal/controller/kubecertagent/annotater.go @@ -28,7 +28,7 @@ const ( ) type annotaterController struct { - agentInfo *Info + agentPodConfig *AgentPodConfig k8sClient kubernetes.Interface kubeSystemPodInformer corev1informers.PodInformer agentPodInformer corev1informers.PodInformer @@ -41,7 +41,7 @@ type annotaterController struct { // agentInfo.CertPathAnnotation and agentInfo.KeyPathAnnotation annotation keys, with the best-guess // paths to the kube API's certificate and key. func NewAnnotaterController( - agentInfo *Info, + agentPodConfig *AgentPodConfig, k8sClient kubernetes.Interface, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, @@ -51,7 +51,7 @@ func NewAnnotaterController( controllerlib.Config{ Name: "kube-cert-agent-annotater-controller", Syncer: &annotaterController{ - agentInfo: agentInfo, + agentPodConfig: agentPodConfig, k8sClient: k8sClient, kubeSystemPodInformer: kubeSystemPodInformer, agentPodInformer: agentPodInformer, @@ -64,9 +64,7 @@ func NewAnnotaterController( ), withInformer( agentPodInformer, - pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { - return isAgentPod(obj, agentInfo.Template.Labels) - }), + pinnipedcontroller.SimpleFilter(isAgentPod), controllerlib.InformerOption{}, ), ) @@ -74,10 +72,10 @@ func NewAnnotaterController( // Sync implements controllerlib.Syncer. func (c *annotaterController) Sync(ctx controllerlib.Context) error { - agentSelector := labels.SelectorFromSet(c.agentInfo.Template.Labels) + agentSelector := labels.SelectorFromSet(c.agentPodConfig.Labels()) agentPods, err := c.agentPodInformer. Lister(). - Pods(c.agentInfo.Template.Namespace). + Pods(c.agentPodConfig.Namespace). List(agentSelector) if err != nil { return fmt.Errorf("informer cannot list agent pods: %w", err) @@ -131,8 +129,8 @@ func (c *annotaterController) maybeUpdateAgentPod( return err } - if agentPod.Annotations[c.agentInfo.CertPathAnnotation] != certPath || - agentPod.Annotations[c.agentInfo.KeyPathAnnotation] != keyPath { + if agentPod.Annotations[agentPodCertPathAnnotationKey] != certPath || + agentPod.Annotations[agentPodKeyPathAnnotationKey] != keyPath { if err := c.reallyUpdateAgentPod( ctx, agentPod, @@ -158,8 +156,8 @@ func (c *annotaterController) reallyUpdateAgentPod( if updatedAgentPod.Annotations == nil { updatedAgentPod.Annotations = make(map[string]string) } - updatedAgentPod.Annotations[c.agentInfo.CertPathAnnotation] = certPath - updatedAgentPod.Annotations[c.agentInfo.KeyPathAnnotation] = keyPath + updatedAgentPod.Annotations[agentPodCertPathAnnotationKey] = certPath + updatedAgentPod.Annotations[agentPodKeyPathAnnotationKey] = keyPath klog.InfoS( "updating agent pod annotations", diff --git a/internal/controller/kubecertagent/annotater_test.go b/internal/controller/kubecertagent/annotater_test.go index 8b1365ab..793c5aa0 100644 --- a/internal/controller/kubecertagent/annotater_test.go +++ b/internal/controller/kubecertagent/annotater_test.go @@ -12,9 +12,7 @@ import ( "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" kubeinformers "k8s.io/client-go/informers" corev1informers "k8s.io/client-go/informers/core/v1" kubernetesfake "k8s.io/client-go/kubernetes/fake" @@ -29,15 +27,14 @@ func TestAnnotaterControllerFilter(t *testing.T) { t, "AnnotaterControllerFilter", func( - agentPodTemplate *corev1.Pod, + agentPodConfig *AgentPodConfig, + _ *CredentialIssuerConfigLocationConfig, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, observableWithInformerOption *testutil.ObservableWithInformerOption, ) { _ = NewAnnotaterController( - &Info{ - Template: agentPodTemplate, - }, + agentPodConfig, nil, // k8sClient, shouldn't matter kubeSystemPodInformer, agentPodInformer, @@ -56,10 +53,10 @@ func TestAnnotaterControllerSync(t *testing.T) { const ( certPath = "some-cert-path" - certPathAnnotation = "some-cert-path-annotation" + certPathAnnotation = "kube-cert-agent.pinniped.dev/cert-path" keyPath = "some-key-path" - keyPathAnnotation = "some-key-path-annotation" + keyPathAnnotation = "kube-cert-agent.pinniped.dev/key-path" ) var r *require.Assertions @@ -73,99 +70,18 @@ func TestAnnotaterControllerSync(t *testing.T) { var timeoutContext context.Context var timeoutContextCancel context.CancelFunc var syncContext *controllerlib.Context - - agentPodTemplate := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-agent-name-", - Namespace: agentPodNamespace, - Labels: map[string]string{ - "some-label-key": "some-label-value", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-agent-image", - }, - }, - }, - } - - controllerManagerPod := &corev1.Pod{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Pod", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: kubeSystemNamespace, - Name: "some-controller-manager-name", - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - UID: types.UID("some-controller-manager-uid"), - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-controller-manager-image", - Command: []string{ - "kube-controller-manager", - "--cluster-signing-cert-file=" + certPath, - "--cluster-signing-key-file=" + keyPath, - }, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "some-volume-mount-name", - }, - }, - }, - }, - NodeName: "some-node-name", - NodeSelector: map[string]string{ - "some-node-selector-key": "some-node-selector-value", - }, - Tolerations: []corev1.Toleration{ - { - Key: "some-toleration", - }, - }, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - }, - } - - // fnv 32a hash of controller-manager uid - controllerManagerPodHash := "fbb0addd" - agentPod := agentPodTemplate.DeepCopy() - agentPod.Namespace = agentPodNamespace - agentPod.Name += controllerManagerPodHash - agentPod.Annotations = map[string]string{ - "kube-cert-agent.pinniped.dev/controller-manager-name": controllerManagerPod.Name, - "kube-cert-agent.pinniped.dev/controller-manager-uid": string(controllerManagerPod.UID), - } - agentPod.Spec.Containers[0].VolumeMounts = controllerManagerPod.Spec.Containers[0].VolumeMounts - agentPod.Spec.RestartPolicy = corev1.RestartPolicyNever - agentPod.Spec.AutomountServiceAccountToken = boolPtr(false) - agentPod.Spec.NodeName = controllerManagerPod.Spec.NodeName - agentPod.Spec.NodeSelector = controllerManagerPod.Spec.NodeSelector - agentPod.Spec.Tolerations = controllerManagerPod.Spec.Tolerations - - podsGVR := schema.GroupVersionResource{ - Group: corev1.SchemeGroupVersion.Group, - Version: corev1.SchemeGroupVersion.Version, - Resource: "pods", - } + var controllerManagerPod, agentPod *corev1.Pod + var podsGVR schema.GroupVersionResource // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewAnnotaterController( - &Info{ - Template: agentPodTemplate, - CertPathAnnotation: certPathAnnotation, - KeyPathAnnotation: keyPathAnnotation, + &AgentPodConfig{ + Namespace: agentPodNamespace, + ContainerImage: "some-agent-image", + PodNamePrefix: "some-agent-name-", }, kubeAPIClient, kubeSystemInformers.Core().V1().Pods(), @@ -202,6 +118,16 @@ func TestAnnotaterControllerSync(t *testing.T) { timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) + controllerManagerPod, agentPod = exampleControllerManagerAndAgentPods( + kubeSystemNamespace, agentPodNamespace, certPath, keyPath, + ) + + podsGVR = schema.GroupVersionResource{ + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: "pods", + } + // Add a pod into the test that doesn't matter to make sure we don't accidentally trigger any // logic on this thing. ignorablePod := corev1.Pod{} diff --git a/internal/controller/kubecertagent/creater.go b/internal/controller/kubecertagent/creater.go index e79be5af..9bdf5333 100644 --- a/internal/controller/kubecertagent/creater.go +++ b/internal/controller/kubecertagent/creater.go @@ -18,10 +18,11 @@ import ( ) type createrController struct { - agentInfo *Info - k8sClient kubernetes.Interface - kubeSystemPodInformer corev1informers.PodInformer - agentPodInformer corev1informers.PodInformer + agentPodConfig *AgentPodConfig + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig + k8sClient kubernetes.Interface + kubeSystemPodInformer corev1informers.PodInformer + agentPodInformer corev1informers.PodInformer } // NewCreaterController returns a controller that creates new kube-cert-agent pods for every known @@ -29,7 +30,8 @@ type createrController struct { // // This controller only uses the Template field of the provided agentInfo. func NewCreaterController( - agentInfo *Info, + agentPodConfig *AgentPodConfig, + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig, k8sClient kubernetes.Interface, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, @@ -40,10 +42,11 @@ func NewCreaterController( //nolint: misspell Name: "kube-cert-agent-creater-controller", Syncer: &createrController{ - agentInfo: agentInfo, - k8sClient: k8sClient, - kubeSystemPodInformer: kubeSystemPodInformer, - agentPodInformer: agentPodInformer, + agentPodConfig: agentPodConfig, + credentialIssuerConfigLocationConfig: credentialIssuerConfigLocationConfig, + k8sClient: k8sClient, + kubeSystemPodInformer: kubeSystemPodInformer, + agentPodInformer: agentPodInformer, }, }, withInformer( @@ -53,9 +56,7 @@ func NewCreaterController( ), withInformer( agentPodInformer, - pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { - return isAgentPod(obj, agentInfo.Template.Labels) - }), + pinnipedcontroller.SimpleFilter(isAgentPod), controllerlib.InformerOption{}, ), ) @@ -80,13 +81,13 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { controllerManagerPod, c.kubeSystemPodInformer, c.agentPodInformer, - c.agentInfo.Template.Labels, + c.agentPodConfig.Labels(), ) if err != nil { return err } if agentPod == nil { - agentPod = newAgentPod(controllerManagerPod, c.agentInfo.Template) + agentPod = newAgentPod(controllerManagerPod, c.agentPodConfig.PodTemplate()) klog.InfoS( "creating agent pod", @@ -96,7 +97,7 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { klog.KObj(controllerManagerPod), ) _, err := c.k8sClient.CoreV1(). - Pods(c.agentInfo.Template.Namespace). + Pods(c.agentPodConfig.Namespace). Create(ctx.Context, agentPod, metav1.CreateOptions{}) if err != nil { // TODO if agent pods fail to create then update the CIC status with an error saying that they couldn't create diff --git a/internal/controller/kubecertagent/creater_test.go b/internal/controller/kubecertagent/creater_test.go index 084d7011..535363b7 100644 --- a/internal/controller/kubecertagent/creater_test.go +++ b/internal/controller/kubecertagent/creater_test.go @@ -12,9 +12,7 @@ import ( "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" kubeinformers "k8s.io/client-go/informers" corev1informers "k8s.io/client-go/informers/core/v1" kubernetesfake "k8s.io/client-go/kubernetes/fake" @@ -29,15 +27,15 @@ func TestCreaterControllerFilter(t *testing.T) { t, "CreaterControllerFilter", func( - agentPodTemplate *corev1.Pod, + agentPodConfig *AgentPodConfig, + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, observableWithInformerOption *testutil.ObservableWithInformerOption, ) { _ = NewCreaterController( - &Info{ - Template: agentPodTemplate, - }, + agentPodConfig, + credentialIssuerConfigLocationConfig, nil, // k8sClient, shouldn't matter kubeSystemPodInformer, agentPodInformer, @@ -63,92 +61,22 @@ func TestCreaterControllerSync(t *testing.T) { var timeoutContext context.Context var timeoutContextCancel context.CancelFunc var syncContext *controllerlib.Context - - agentPodTemplate := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: agentPodNamespace, - Name: "some-agent-name-", - Labels: map[string]string{ - "some-label-key": "some-label-value", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-agent-image", - }, - }, - }, - } - - controllerManagerPod := &corev1.Pod{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Pod", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: kubeSystemNamespace, - Name: "some-controller-manager-name", - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - UID: types.UID("some-controller-manager-uid"), - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-controller-manager-image", - VolumeMounts: []corev1.VolumeMount{ - { - Name: "some-volume-mount-name", - }, - }, - }, - }, - NodeName: "some-node-name", - NodeSelector: map[string]string{ - "some-node-selector-key": "some-node-selector-value", - }, - Tolerations: []corev1.Toleration{ - { - Key: "some-toleration", - }, - }, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - }, - } - - // fnv 32a hash of controller-manager uid - controllerManagerPodHash := "fbb0addd" - agentPod := agentPodTemplate.DeepCopy() - agentPod.Name += controllerManagerPodHash - agentPod.Namespace = agentPodNamespace - agentPod.Annotations = map[string]string{ - "kube-cert-agent.pinniped.dev/controller-manager-name": controllerManagerPod.Name, - "kube-cert-agent.pinniped.dev/controller-manager-uid": string(controllerManagerPod.UID), - } - agentPod.Spec.Containers[0].VolumeMounts = controllerManagerPod.Spec.Containers[0].VolumeMounts - agentPod.Spec.RestartPolicy = corev1.RestartPolicyNever - agentPod.Spec.AutomountServiceAccountToken = boolPtr(false) - agentPod.Spec.NodeName = controllerManagerPod.Spec.NodeName - agentPod.Spec.NodeSelector = controllerManagerPod.Spec.NodeSelector - agentPod.Spec.Tolerations = controllerManagerPod.Spec.Tolerations - - podsGVR := schema.GroupVersionResource{ - Group: corev1.SchemeGroupVersion.Group, - Version: corev1.SchemeGroupVersion.Version, - Resource: "pods", - } + var controllerManagerPod, agentPod *corev1.Pod + var podsGVR schema.GroupVersionResource // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewCreaterController( - &Info{ - Template: agentPodTemplate, + &AgentPodConfig{ + Namespace: agentPodNamespace, + ContainerImage: "some-agent-image", + PodNamePrefix: "some-agent-name-", + }, + &CredentialIssuerConfigLocationConfig{ + Namespace: "not used yet", + Name: "not used yet", }, kubeAPIClient, kubeSystemInformers.Core().V1().Pods(), @@ -185,6 +113,16 @@ func TestCreaterControllerSync(t *testing.T) { timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) + controllerManagerPod, agentPod = exampleControllerManagerAndAgentPods( + kubeSystemNamespace, agentPodNamespace, "ignored for this test", "ignored for this test", + ) + + podsGVR = schema.GroupVersionResource{ + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: "pods", + } + // Add a pod into the test that doesn't matter to make sure we don't accidentally trigger any // logic on this thing. ignorablePod := corev1.Pod{} diff --git a/internal/controller/kubecertagent/deleter.go b/internal/controller/kubecertagent/deleter.go index 2412b8fe..61976352 100644 --- a/internal/controller/kubecertagent/deleter.go +++ b/internal/controller/kubecertagent/deleter.go @@ -17,7 +17,7 @@ import ( ) type deleterController struct { - agentInfo *Info + agentPodConfig *AgentPodConfig k8sClient kubernetes.Interface kubeSystemPodInformer corev1informers.PodInformer agentPodInformer corev1informers.PodInformer @@ -28,7 +28,7 @@ type deleterController struct { // // This controller only uses the Template field of the provided agentInfo. func NewDeleterController( - agentInfo *Info, + agentPodConfig *AgentPodConfig, k8sClient kubernetes.Interface, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, @@ -38,7 +38,7 @@ func NewDeleterController( controllerlib.Config{ Name: "kube-cert-agent-deleter-controller", Syncer: &deleterController{ - agentInfo: agentInfo, + agentPodConfig: agentPodConfig, k8sClient: k8sClient, kubeSystemPodInformer: kubeSystemPodInformer, agentPodInformer: agentPodInformer, @@ -51,9 +51,7 @@ func NewDeleterController( ), withInformer( agentPodInformer, - pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { - return isAgentPod(obj, agentInfo.Template.Labels) - }), + pinnipedcontroller.SimpleFilter(isAgentPod), controllerlib.InformerOption{}, ), ) @@ -61,10 +59,10 @@ func NewDeleterController( // Sync implements controllerlib.Syncer. func (c *deleterController) Sync(ctx controllerlib.Context) error { - agentSelector := labels.SelectorFromSet(c.agentInfo.Template.Labels) + agentSelector := labels.SelectorFromSet(c.agentPodConfig.Labels()) agentPods, err := c.agentPodInformer. Lister(). - Pods(c.agentInfo.Template.Namespace). + Pods(c.agentPodConfig.Namespace). List(agentSelector) if err != nil { return fmt.Errorf("informer cannot list agent pods: %w", err) @@ -76,7 +74,7 @@ func (c *deleterController) Sync(ctx controllerlib.Context) error { return err } if controllerManagerPod == nil || - !isAgentPodUpToDate(agentPod, newAgentPod(controllerManagerPod, c.agentInfo.Template)) { + !isAgentPodUpToDate(agentPod, newAgentPod(controllerManagerPod, c.agentPodConfig.PodTemplate())) { 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 bbf2277d..065370fc 100644 --- a/internal/controller/kubecertagent/deleter_test.go +++ b/internal/controller/kubecertagent/deleter_test.go @@ -12,9 +12,7 @@ import ( "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" kubeinformers "k8s.io/client-go/informers" corev1informers "k8s.io/client-go/informers/core/v1" kubernetesfake "k8s.io/client-go/kubernetes/fake" @@ -29,15 +27,14 @@ func TestDeleterControllerFilter(t *testing.T) { t, "DeleterControllerFilter", func( - agentPodTemplate *corev1.Pod, + agentPodConfig *AgentPodConfig, + _ *CredentialIssuerConfigLocationConfig, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, observableWithInformerOption *testutil.ObservableWithInformerOption, ) { _ = NewDeleterController( - &Info{ - Template: agentPodTemplate, - }, + agentPodConfig, nil, // k8sClient, shouldn't matter kubeSystemPodInformer, agentPodInformer, @@ -63,92 +60,18 @@ func TestDeleterControllerSync(t *testing.T) { var timeoutContext context.Context var timeoutContextCancel context.CancelFunc var syncContext *controllerlib.Context - - agentPodTemplate := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-agent-name-", - Namespace: agentPodNamespace, - Labels: map[string]string{ - "some-label-key": "some-label-value", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-agent-image", - }, - }, - }, - } - - controllerManagerPod := &corev1.Pod{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Pod", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: kubeSystemNamespace, - Name: "some-controller-manager-name", - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - UID: types.UID("some-controller-manager-uid"), - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-controller-manager-image", - VolumeMounts: []corev1.VolumeMount{ - { - Name: "some-volume-mount-name", - }, - }, - }, - }, - NodeName: "some-node-name", - NodeSelector: map[string]string{ - "some-node-selector-key": "some-node-selector-value", - }, - Tolerations: []corev1.Toleration{ - { - Key: "some-toleration", - }, - }, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - }, - } - - podsGVR := schema.GroupVersionResource{ - Group: corev1.SchemeGroupVersion.Group, - Version: corev1.SchemeGroupVersion.Version, - Resource: "pods", - } - - // fnv 32a hash of "some-controller-manager-uid" - controllerManagerPodHash := "fbb0addd" - agentPod := agentPodTemplate.DeepCopy() - agentPod.Namespace = agentPodNamespace - agentPod.Name += controllerManagerPodHash - agentPod.Annotations = map[string]string{ - "kube-cert-agent.pinniped.dev/controller-manager-name": controllerManagerPod.Name, - "kube-cert-agent.pinniped.dev/controller-manager-uid": string(controllerManagerPod.UID), - } - agentPod.Spec.Containers[0].VolumeMounts = controllerManagerPod.Spec.Containers[0].VolumeMounts - agentPod.Spec.RestartPolicy = corev1.RestartPolicyNever - agentPod.Spec.AutomountServiceAccountToken = boolPtr(false) - agentPod.Spec.NodeName = controllerManagerPod.Spec.NodeName - agentPod.Spec.NodeSelector = controllerManagerPod.Spec.NodeSelector - agentPod.Spec.Tolerations = controllerManagerPod.Spec.Tolerations + var controllerManagerPod, agentPod *corev1.Pod + var podsGVR schema.GroupVersionResource // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewDeleterController( - &Info{ - Template: agentPodTemplate, + &AgentPodConfig{ + Namespace: agentPodNamespace, + ContainerImage: "some-agent-image", + PodNamePrefix: "some-agent-name-", }, kubeAPIClient, kubeSystemInformers.Core().V1().Pods(), @@ -185,6 +108,16 @@ func TestDeleterControllerSync(t *testing.T) { agentInformerClient = kubernetesfake.NewSimpleClientset() agentInformers = kubeinformers.NewSharedInformerFactory(agentInformerClient, 0) + controllerManagerPod, agentPod = exampleControllerManagerAndAgentPods( + kubeSystemNamespace, agentPodNamespace, "ignored for this test", "ignored for this test", + ) + + podsGVR = schema.GroupVersionResource{ + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: "pods", + } + // Add an pod into the test that doesn't matter to make sure we don't accidentally // trigger any logic on this thing. ignorablePod := corev1.Pod{} @@ -228,8 +161,8 @@ func TestDeleterControllerSync(t *testing.T) { Name: "some-other-volume-mount", }, } - r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod)) - r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod)) + r.NoError(kubeSystemInformerClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) }) it("deletes the agent pod", func() { @@ -257,8 +190,8 @@ func TestDeleterControllerSync(t *testing.T) { Name: "some-other-volume", }, } - r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod)) - r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod)) + r.NoError(kubeSystemInformerClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) }) it("deletes the agent pod", func() { @@ -284,8 +217,8 @@ func TestDeleterControllerSync(t *testing.T) { controllerManagerPod.Spec.NodeSelector = map[string]string{ "some-other-node-selector-key": "some-other-node-selector-value", } - r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod)) - r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod)) + r.NoError(kubeSystemInformerClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) }) it("deletes the agent pod", func() { @@ -309,8 +242,8 @@ func TestDeleterControllerSync(t *testing.T) { when("the agent pod is out of sync with the controller manager via node name", func() { it.Before(func() { controllerManagerPod.Spec.NodeName = "some-other-node-name" - r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod)) - r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod)) + r.NoError(kubeSystemInformerClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) }) it("deletes the agent pod", func() { @@ -338,8 +271,8 @@ func TestDeleterControllerSync(t *testing.T) { Key: "some-other-toleration-key", }, } - r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod)) - r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod)) + r.NoError(kubeSystemInformerClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) + r.NoError(kubeAPIClient.Tracker().Update(podsGVR, controllerManagerPod, controllerManagerPod.Namespace)) }) it("deletes the agent pod", func() { @@ -368,7 +301,7 @@ func TestDeleterControllerSync(t *testing.T) { r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) }) - it.Focus("deletes the agent pod", func() { + it("deletes the agent pod", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) @@ -394,7 +327,7 @@ func TestDeleterControllerSync(t *testing.T) { r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace)) }) - it.Focus("deletes the agent pod", func() { + it("deletes the agent pod", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) diff --git a/internal/controller/kubecertagent/execer.go b/internal/controller/kubecertagent/execer.go index fc1d0f30..45f9aee3 100644 --- a/internal/controller/kubecertagent/execer.go +++ b/internal/controller/kubecertagent/execer.go @@ -22,23 +22,19 @@ import ( ) type execerController struct { - agentInfo *Info - credentialIssuerConfigNamespaceName string - credentialIssuerConfigResourceName string - dynamicCertProvider dynamiccert.Provider - podCommandExecutor PodCommandExecutor - clock clock.Clock - pinnipedAPIClient pinnipedclientset.Interface - agentPodInformer corev1informers.PodInformer + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig + dynamicCertProvider dynamiccert.Provider + podCommandExecutor PodCommandExecutor + clock clock.Clock + pinnipedAPIClient pinnipedclientset.Interface + agentPodInformer corev1informers.PodInformer } // NewExecerController returns a controllerlib.Controller that listens for agent pods with proper // cert/key path annotations and execs into them to get the cert/key material. It sets the retrieved // key material in a provided dynamicCertProvider. func NewExecerController( - agentInfo *Info, - credentialIssuerConfigNamespaceName string, - credentialIssuerConfigResourceName string, + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig, dynamicCertProvider dynamiccert.Provider, podCommandExecutor PodCommandExecutor, pinnipedAPIClient pinnipedclientset.Interface, @@ -50,21 +46,17 @@ func NewExecerController( controllerlib.Config{ Name: "kube-cert-agent-execer-controller", Syncer: &execerController{ - agentInfo: agentInfo, - credentialIssuerConfigNamespaceName: credentialIssuerConfigNamespaceName, - credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, - dynamicCertProvider: dynamicCertProvider, - podCommandExecutor: podCommandExecutor, - pinnipedAPIClient: pinnipedAPIClient, - clock: clock, - agentPodInformer: agentPodInformer, + credentialIssuerConfigLocationConfig: credentialIssuerConfigLocationConfig, + dynamicCertProvider: dynamicCertProvider, + podCommandExecutor: podCommandExecutor, + pinnipedAPIClient: pinnipedAPIClient, + clock: clock, + agentPodInformer: agentPodInformer, }, }, withInformer( agentPodInformer, - pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { - return isAgentPod(obj, agentInfo.Template.Labels) - }), + pinnipedcontroller.SimpleFilter(isAgentPod), controllerlib.InformerOption{}, ), ) @@ -120,8 +112,8 @@ func (c *execerController) Sync(ctx controllerlib.Context) error { func (c *execerController) createOrUpdateCredentialIssuerConfig(ctx controllerlib.Context, strategyResult configv1alpha1.CredentialIssuerConfigStrategy) error { return issuerconfig.CreateOrUpdateCredentialIssuerConfig( ctx.Context, - c.credentialIssuerConfigNamespaceName, - c.credentialIssuerConfigResourceName, + c.credentialIssuerConfigLocationConfig.Namespace, + c.credentialIssuerConfigLocationConfig.Name, c.pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.Strategies = []configv1alpha1.CredentialIssuerConfigStrategy{strategyResult} @@ -155,8 +147,8 @@ func (c *execerController) getKeypairFilePaths(pod *v1.Pod) (string, string) { annotations = make(map[string]string) } - certPath := annotations[c.agentInfo.CertPathAnnotation] - keyPath := annotations[c.agentInfo.KeyPathAnnotation] + certPath := annotations[agentPodCertPathAnnotationKey] + keyPath := annotations[agentPodKeyPathAnnotationKey] return certPath, keyPath } diff --git a/internal/controller/kubecertagent/execer_test.go b/internal/controller/kubecertagent/execer_test.go index ef3633ff..ebbd0ca0 100644 --- a/internal/controller/kubecertagent/execer_test.go +++ b/internal/controller/kubecertagent/execer_test.go @@ -38,27 +38,15 @@ func TestExecerControllerOptions(t *testing.T) { whateverPod := &corev1.Pod{} - agentPodTemplate := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-agent-name-ignored", - Namespace: "some-namespace-ignored", - Labels: map[string]string{ - "some-label-key": "some-label-value", - }, - }, - Spec: corev1.PodSpec{}, - } - it.Before(func() { r = require.New(t) observableWithInformerOption = testutil.NewObservableWithInformerOption() agentPodsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Pods() _ = NewExecerController( - &Info{ - Template: agentPodTemplate, + &CredentialIssuerConfigLocationConfig{ + Namespace: "ignored by this test", + Name: "ignored by this test", }, - "credentialIssuerConfigNamespaceName", - "credentialIssuerConfigResourceName", nil, // dynamicCertProvider, not needed for this test nil, // podCommandExecutor, not needed for this test nil, // pinnipedAPIClient, not needed for this test @@ -70,13 +58,12 @@ func TestExecerControllerOptions(t *testing.T) { }) when("the change is happening in the agent's namespace", func() { - when("a pod with all the agent labels is added/updated/deleted", func() { + when("a pod with all agent labels is added/updated/deleted", func() { it("returns true", func() { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "some-label-key": "some-label-value", - "some-other-label-key": "some-other-label-value", + "kube-cert-agent.pinniped.dev": "true", }, }, } @@ -88,7 +75,7 @@ func TestExecerControllerOptions(t *testing.T) { }) }) - when("a pod missing any of the agent labels is added/updated/deleted", func() { + when("a pod missing the agent label is added/updated/deleted", func() { it("returns false", func() { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -143,8 +130,8 @@ func TestManagerControllerSync(t *testing.T) { spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { const agentPodNamespace = "some-namespace" const agentPodName = "some-agent-pod-name-123" - const certPathAnnotationName = "cert-path-annotation-name" - const keyPathAnnotationName = "key-path-annotation-name" + const certPathAnnotationName = "kube-cert-agent.pinniped.dev/cert-path" + const keyPathAnnotationName = "kube-cert-agent.pinniped.dev/key-path" const fakeCertPath = "/some/cert/path" const fakeKeyPath = "/some/key/path" const defaultDynamicCertProviderCert = "initial-cert" @@ -162,7 +149,6 @@ func TestManagerControllerSync(t *testing.T) { var agentPodInformer kubeinformers.SharedInformerFactory var agentPodInformerClient *kubernetesfake.Clientset var fakeExecutor *fakePodExecutor - var agentPodTemplate *corev1.Pod var dynamicCertProvider dynamiccert.Provider var fakeCertPEM, fakeKeyPEM string var credentialIssuerConfigGVR schema.GroupVersionResource @@ -173,13 +159,10 @@ func TestManagerControllerSync(t *testing.T) { var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewExecerController( - &Info{ - Template: agentPodTemplate, - CertPathAnnotation: certPathAnnotationName, - KeyPathAnnotation: keyPathAnnotationName, + &CredentialIssuerConfigLocationConfig{ + Namespace: credentialIssuerConfigNamespaceName, + Name: credentialIssuerConfigResourceName, }, - credentialIssuerConfigNamespaceName, - credentialIssuerConfigResourceName, dynamicCertProvider, fakeExecutor, pinnipedAPIClient, @@ -254,23 +237,6 @@ func TestManagerControllerSync(t *testing.T) { fakeCertPEM = loadFile("./testdata/test.crt") fakeKeyPEM = loadFile("./testdata/test.key") - agentPodTemplate = &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-agent-pod-name-", - Namespace: agentPodNamespace, - Labels: map[string]string{ - "some-label-key": "some-label-value", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "some-agent-image", - }, - }, - }, - } - credentialIssuerConfigGVR = schema.GroupVersionResource{ Group: configv1alpha1.GroupName, Version: configv1alpha1.SchemeGroupVersion.Version, diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index f4e0be91..9ef72b6e 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -14,6 +14,8 @@ import ( "fmt" "hash/fnv" + "k8s.io/apimachinery/pkg/api/resource" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -28,62 +30,77 @@ const ( controllerManagerNameAnnotationKey = "kube-cert-agent.pinniped.dev/controller-manager-name" controllerManagerUIDAnnotationKey = "kube-cert-agent.pinniped.dev/controller-manager-uid" + + // agentPodLabelKey is used to identify which pods are created by the kube-cert-agent + // controllers. + agentPodLabelKey = "kube-cert-agent.pinniped.dev" + agentPodLabelValue = "true" + + // agentPodCertPathAnnotationKey is the annotation that the kube-cert-agent pod will use + // to communicate the in-pod path to the kube API's certificate. + agentPodCertPathAnnotationKey = "kube-cert-agent.pinniped.dev/cert-path" + + // agentPodKeyPathAnnotationKey is the annotation that the kube-cert-agent pod will use + // to communicate the in-pod path to the kube API's key. + agentPodKeyPathAnnotationKey = "kube-cert-agent.pinniped.dev/key-path" ) -// Info holds necessary information about the agent pod. It was pulled out into a struct to have a -// common parameter for each controller. -type Info struct { - // Template is an injection point for pod fields. The required pod fields are as follows. - // .Namespace: serves as the namespace of the agent pods - // .Name: serves as the name prefix for each of the agent pods - // .Labels: serves as a way to filter for agent pods - // .Spec.Containers[0].Name: serves as the container name the agent pods - // .Spec.Containers[0].Image: serves as the container image for the agent pods - // .Spec.Containers[0].Command: serves as the container command for the agent pods - Template *corev1.Pod +type AgentPodConfig struct { + // The namespace in which agent pods will be created. + Namespace string - // CertPathAnnotation is the name of the annotation key that will be used when setting the - // best-guess path to the kube API's certificate in the agent pod. - CertPathAnnotation string + // The container image used for the agent pods. + ContainerImage string - // KeyPathAnnotation is the name of the annotation key that will be used when setting the - // best-guess path to the kube API's private key in the agent pod. - KeyPathAnnotation string + // The name prefix for each of the agent pods. + PodNamePrefix string } -func isControllerManagerPod(obj metav1.Object) bool { - pod, ok := obj.(*corev1.Pod) - if !ok { - return false - } +type CredentialIssuerConfigLocationConfig struct { + // The namespace in which the CredentialIssuerConfig should be created/updated. + Namespace string - if pod.Labels == nil { - return false - } - - component, ok := pod.Labels["component"] - if !ok || component != "kube-controller-manager" { - return false - } - - if pod.Status.Phase != corev1.PodRunning { - return false - } - - return true + // The resource name for the CredentialIssuerConfig to be created/updated. + Name string } -func isAgentPod(obj metav1.Object, agentLabels map[string]string) bool { - for agentLabelKey, agentLabelValue := range agentLabels { - v, ok := obj.GetLabels()[agentLabelKey] - if !ok { - return false - } - if v != agentLabelValue { - return false - } +func (c *AgentPodConfig) Labels() map[string]string { + return map[string]string{ + agentPodLabelKey: agentPodLabelValue, } - return true +} + +func (c *AgentPodConfig) PodTemplate() *corev1.Pod { + terminateImmediately := int64(0) + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.PodNamePrefix, + Namespace: c.Namespace, + Labels: c.Labels(), + }, + Spec: corev1.PodSpec{ + TerminationGracePeriodSeconds: &terminateImmediately, + Containers: []corev1.Container{ + { + Name: "sleeper", + Image: c.ContainerImage, + ImagePullPolicy: corev1.PullIfNotPresent, + Command: []string{"/bin/sleep", "infinity"}, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("16Mi"), + corev1.ResourceCPU: resource.MustParse("10m"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("16Mi"), + corev1.ResourceCPU: resource.MustParse("10m"), + }, + }, + }, + }, + }, + } + return pod } func newAgentPod( @@ -157,6 +174,33 @@ func isAgentPodUpToDate(actualAgentPod, expectedAgentPod *corev1.Pod) bool { ) } +func isControllerManagerPod(obj metav1.Object) bool { + pod, ok := obj.(*corev1.Pod) + if !ok { + return false + } + + if pod.Labels == nil { + return false + } + + component, ok := pod.Labels["component"] + if !ok || component != "kube-controller-manager" { + return false + } + + if pod.Status.Phase != corev1.PodRunning { + return false + } + + return true +} + +func isAgentPod(obj metav1.Object) bool { + value, foundLabel := obj.GetLabels()[agentPodLabelKey] + return foundLabel && value == agentPodLabelValue +} + func findControllerManagerPodForSpecificAgentPod( agentPod *corev1.Pod, kubeSystemPodInformer corev1informers.PodInformer, diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 892af6ce..e265fdc7 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -9,7 +9,9 @@ import ( "github.com/sclevine/spec" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" kubeinformers "k8s.io/client-go/informers" corev1informers "k8s.io/client-go/informers/core/v1" @@ -17,10 +19,114 @@ import ( "go.pinniped.dev/internal/testutil" ) +func exampleControllerManagerAndAgentPods( + kubeSystemNamespace, + agentPodNamespace, + certPath, + keyPath string, +) (*corev1.Pod, *corev1.Pod) { + controllerManagerPod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Pod", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: kubeSystemNamespace, + Name: "some-controller-manager-name", + Labels: map[string]string{ + "component": "kube-controller-manager", + }, + UID: types.UID("some-controller-manager-uid"), + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "some-controller-manager-image", + Command: []string{ + "kube-controller-manager", + "--cluster-signing-cert-file=" + certPath, + "--cluster-signing-key-file=" + keyPath, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "some-volume-mount-name", + }, + }, + }, + }, + NodeName: "some-node-name", + NodeSelector: map[string]string{ + "some-node-selector-key": "some-node-selector-value", + }, + Tolerations: []corev1.Toleration{ + { + Key: "some-toleration", + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + }, + } + + zero := int64(0) + + // fnv 32a hash of controller-manager uid + controllerManagerPodHash := "fbb0addd" + agentPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-agent-name-" + controllerManagerPodHash, + Namespace: agentPodNamespace, + Labels: map[string]string{ + "kube-cert-agent.pinniped.dev": "true", + }, + Annotations: map[string]string{ + "kube-cert-agent.pinniped.dev/controller-manager-name": controllerManagerPod.Name, + "kube-cert-agent.pinniped.dev/controller-manager-uid": string(controllerManagerPod.UID), + }, + }, + Spec: corev1.PodSpec{ + TerminationGracePeriodSeconds: &zero, + Containers: []corev1.Container{ + { + Name: "sleeper", + Image: "some-agent-image", + ImagePullPolicy: corev1.PullIfNotPresent, + VolumeMounts: controllerManagerPod.Spec.Containers[0].VolumeMounts, + Command: []string{"/bin/sleep", "infinity"}, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("16Mi"), + corev1.ResourceCPU: resource.MustParse("10m"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("16Mi"), + corev1.ResourceCPU: resource.MustParse("10m"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyNever, + AutomountServiceAccountToken: boolPtr(false), + NodeName: controllerManagerPod.Spec.NodeName, + NodeSelector: controllerManagerPod.Spec.NodeSelector, + Tolerations: controllerManagerPod.Spec.Tolerations, + }, + } + + return controllerManagerPod, agentPod +} + func defineSharedKubecertagentFilterSpecs( t *testing.T, name string, - newFunc func(agentPodTemplate *corev1.Pod, kubeSystemPodInformer corev1informers.PodInformer, agentPodInformer corev1informers.PodInformer, observableWithInformerOption *testutil.ObservableWithInformerOption), + newFunc func( + agentPodConfig *AgentPodConfig, + credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig, + kubeSystemPodInformer corev1informers.PodInformer, + agentPodInformer corev1informers.PodInformer, + observableWithInformerOption *testutil.ObservableWithInformerOption, + ), ) { spec.Run(t, name, func(t *testing.T, when spec.G, it spec.S) { var r *require.Assertions @@ -31,15 +137,10 @@ func defineSharedKubecertagentFilterSpecs( it.Before(func() { r = require.New(t) - agentPodTemplate := &corev1.Pod{} - agentPodTemplate.Labels = map[string]string{ - "some-label-key": "some-label-value", - "some-other-label-key": "some-other-label-value", - } kubeSystemPodInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Pods() agentPodInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Pods() observableWithInformerOption := testutil.NewObservableWithInformerOption() - newFunc(agentPodTemplate, kubeSystemPodInformer, agentPodInformer, observableWithInformerOption) + newFunc(&AgentPodConfig{}, &CredentialIssuerConfigLocationConfig{}, kubeSystemPodInformer, agentPodInformer, observableWithInformerOption) kubeSystemPodInformerFilter = observableWithInformerOption.GetFilterForInformer(kubeSystemPodInformer) agentPodInformerFilter = observableWithInformerOption.GetFilterForInformer(agentPodInformer) @@ -100,14 +201,13 @@ func defineSharedKubecertagentFilterSpecs( }) }) - when("the change is happening in the agent's namespace", func() { - when("a pod with all the agent labels is added/updated/deleted", func() { + when("the change is happening in the agent's informer", func() { + when("a pod with the agent label is added/updated/deleted", func() { it("returns true", func() { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "some-label-key": "some-label-value", - "some-other-label-key": "some-other-label-value", + "kube-cert-agent.pinniped.dev": "true", }, }, } @@ -119,7 +219,7 @@ func defineSharedKubecertagentFilterSpecs( }) }) - when("a pod missing any of the agent labels is added/updated/deleted", func() { + when("a pod missing the agent label is added/updated/deleted", func() { it("returns false", func() { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index ea30d758..dfb1d934 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -10,7 +10,6 @@ import ( "fmt" "time" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/clock" k8sinformers "k8s.io/client-go/informers" @@ -50,6 +49,10 @@ type Config struct { // objects should be named. NamesConfig *api.NamesConfigSpec + // KubeCertAgentConfig comes from the Pinniped config API (see api.Config). It configures how + // the kubecertagent package's controllers should manage the agent pods. + KubeCertAgentConfig *api.KubeCertAgentSpec + // DiscoveryURLOverride allows a caller to inject a hardcoded discovery URL into Pinniped // discovery document. DiscoveryURLOverride *string @@ -69,17 +72,6 @@ type Config struct { // IDPCache is a cache of authenticators shared amongst various IDP-related controllers. IDPCache *idpcache.Cache - - // KubeCertAgentTemplate is the template from which the kube-cert-agent controllers will create a - // kube-cert-agent pod. See kubecertagent.Info for more details. - KubeCertAgentTemplate *corev1.Pod - // KubeCertAgentCertPathAnnotation is the name of the annotation key that will be used when - // setting the best-guess path to the kube API's certificate. See kubecertagent.Info for more - // details. - KubeCertAgentCertPathAnnotation string - // KubeCertAgentKeyPathAnnotation is the name of the annotation key that will be used when setting - // the best-guess path to the kube API's key. See kubecertagent.Info for more details. - KubeCertAgentKeyPathAnnotation string } // Prepare the controllers and their informers and return a function that will start them when called. @@ -98,9 +90,23 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { // Create informers. Don't forget to make sure they get started in the function returned below. informers := createInformers(c.ServerInstallationNamespace, k8sClient, pinnipedClient) + // Configuration for the kubecertagent controllers created below. + agentPodConfig := &kubecertagent.AgentPodConfig{ + Namespace: c.ServerInstallationNamespace, + ContainerImage: *c.KubeCertAgentConfig.Image, + PodNamePrefix: *c.KubeCertAgentConfig.NamePrefix, + } + credentialIssuerConfigLocationConfig := &kubecertagent.CredentialIssuerConfigLocationConfig{ + Namespace: c.ServerInstallationNamespace, + Name: c.NamesConfig.CredentialIssuerConfig, + } + // Create controller manager. controllerManager := controllerlib. NewManager(). + + // KubeConfig info publishing controller is responsible for writing the KubeConfig information to the + // CredentialIssuerConfig resource and keeping that information up to date. WithController( issuerconfig.NewKubeConfigInfoPublisherController( c.ServerInstallationNamespace, @@ -113,6 +119,8 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { ), singletonWorker, ). + + // API certs controllers are responsible for managing the TLS certificates used to serve Pinniped's API. WithController( apicerts.NewCertsManagerController( c.ServerInstallationNamespace, @@ -159,6 +167,55 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { ), singletonWorker, ). + + // Kube cert agent controllers are responsible for finding the cluster's signing keys and keeping them + // up to date in memory, as well as reporting status on this cluster integration strategy. + WithController( + kubecertagent.NewCreaterController( + agentPodConfig, + credentialIssuerConfigLocationConfig, + k8sClient, + informers.kubeSystemNamespaceK8s.Core().V1().Pods(), + informers.installationNamespaceK8s.Core().V1().Pods(), + controllerlib.WithInformer, + ), + singletonWorker, + ). + WithController( + kubecertagent.NewAnnotaterController( + agentPodConfig, + k8sClient, + informers.kubeSystemNamespaceK8s.Core().V1().Pods(), + informers.installationNamespaceK8s.Core().V1().Pods(), + controllerlib.WithInformer, + ), + singletonWorker, + ). + WithController( + kubecertagent.NewExecerController( + credentialIssuerConfigLocationConfig, + c.DynamicSigningCertProvider, + kubecertagent.NewPodCommandExecutor(kubeConfig, k8sClient), + pinnipedClient, + clock.RealClock{}, + informers.installationNamespaceK8s.Core().V1().Pods(), + controllerlib.WithInformer, + ), + singletonWorker, + ). + WithController( + kubecertagent.NewDeleterController( + agentPodConfig, + k8sClient, + informers.kubeSystemNamespaceK8s.Core().V1().Pods(), + informers.installationNamespaceK8s.Core().V1().Pods(), + controllerlib.WithInformer, + ), + singletonWorker, + ). + + // The cache filler controllers are responsible for keep an in-memory representation of active + // IDPs up to date. WithController( webhookcachefiller.New( c.IDPCache, @@ -174,62 +231,6 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { klogr.New(), ), singletonWorker, - ). - WithController( - kubecertagent.NewCreaterController( - &kubecertagent.Info{ - Template: c.KubeCertAgentTemplate, - }, - k8sClient, - informers.kubeSystemNamespaceK8s.Core().V1().Pods(), - informers.installationNamespaceK8s.Core().V1().Pods(), - controllerlib.WithInformer, - ), - singletonWorker, - ). - WithController( - kubecertagent.NewDeleterController( - &kubecertagent.Info{ - Template: c.KubeCertAgentTemplate, - }, - k8sClient, - informers.kubeSystemNamespaceK8s.Core().V1().Pods(), - informers.installationNamespaceK8s.Core().V1().Pods(), - controllerlib.WithInformer, - ), - singletonWorker, - ). - WithController( - kubecertagent.NewAnnotaterController( - &kubecertagent.Info{ - Template: c.KubeCertAgentTemplate, - CertPathAnnotation: c.KubeCertAgentCertPathAnnotation, - KeyPathAnnotation: c.KubeCertAgentKeyPathAnnotation, - }, - k8sClient, - informers.kubeSystemNamespaceK8s.Core().V1().Pods(), - informers.installationNamespaceK8s.Core().V1().Pods(), - controllerlib.WithInformer, - ), - singletonWorker, - ). - WithController( - kubecertagent.NewExecerController( - &kubecertagent.Info{ - Template: c.KubeCertAgentTemplate, - CertPathAnnotation: c.KubeCertAgentCertPathAnnotation, - KeyPathAnnotation: c.KubeCertAgentKeyPathAnnotation, - }, - c.ServerInstallationNamespace, - c.NamesConfig.CredentialIssuerConfig, - c.DynamicSigningCertProvider, - kubecertagent.NewPodCommandExecutor(kubeConfig, k8sClient), - pinnipedClient, - clock.RealClock{}, - informers.installationNamespaceK8s.Core().V1().Pods(), - controllerlib.WithInformer, - ), - singletonWorker, ) // Return a function which starts the informers and controllers. diff --git a/internal/server/server.go b/internal/server/server.go index 2839b3ab..63456a79 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -11,9 +11,6 @@ import ( "time" "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" genericapiserver "k8s.io/apiserver/pkg/server" genericoptions "k8s.io/apiserver/pkg/server/options" @@ -27,22 +24,6 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/registry/credentialrequest" "go.pinniped.dev/pkg/config" - configapi "go.pinniped.dev/pkg/config/api" -) - -// These constants are various label/annotation keys used in Pinniped. They are namespaced by -// a "pinniped.dev" child domain so they don't collide with other keys. -const ( - // kubeCertAgentLabelKey is used to identify which pods are created by the kube-cert-agent - // controllers. - kubeCertAgentLabelKey = "kube-cert-agent.pinniped.dev" - - // kubeCertAgentCertPathAnnotationKey is the annotation that the kube-cert-agent pod will use - // to communicate the in-pod path to the kube API's certificate. - kubeCertAgentCertPathAnnotationKey = "kube-cert-agent.pinniped.dev/cert-path" - // kubeCertAgentKeyPathAnnotationKey is the annotation that the kube-cert-agent pod will use - // to communicate the in-pod path to the kube API's key. - kubeCertAgentKeyPathAnnotationKey = "kube-cert-agent.pinniped.dev/key-path" ) // App is an object that represents the pinniped-server application. @@ -123,12 +104,6 @@ func (a *App) runServer(ctx context.Context) error { } serverInstallationNamespace := podInfo.Namespace - // Load the Kubernetes cluster signing CA. - kubeCertAgentTemplate := createKubeCertAgentTemplate( - &cfg.KubeCertAgentConfig, - serverInstallationNamespace, - ) - // Initialize the cache of active identity providers. idpCache := idpcache.New() @@ -147,17 +122,15 @@ func (a *App) runServer(ctx context.Context) error { // post start hook of the aggregated API server. startControllersFunc, err := controllermanager.PrepareControllers( &controllermanager.Config{ - ServerInstallationNamespace: serverInstallationNamespace, - NamesConfig: &cfg.NamesConfig, - DiscoveryURLOverride: cfg.DiscoveryInfo.URL, - DynamicServingCertProvider: dynamicServingCertProvider, - DynamicSigningCertProvider: dynamicSigningCertProvider, - ServingCertDuration: time.Duration(*cfg.APIConfig.ServingCertificateConfig.DurationSeconds) * time.Second, - ServingCertRenewBefore: time.Duration(*cfg.APIConfig.ServingCertificateConfig.RenewBeforeSeconds) * time.Second, - IDPCache: idpCache, - KubeCertAgentTemplate: kubeCertAgentTemplate, - KubeCertAgentCertPathAnnotation: kubeCertAgentCertPathAnnotationKey, - KubeCertAgentKeyPathAnnotation: kubeCertAgentKeyPathAnnotationKey, + ServerInstallationNamespace: serverInstallationNamespace, + NamesConfig: &cfg.NamesConfig, + KubeCertAgentConfig: &cfg.KubeCertAgentConfig, + DiscoveryURLOverride: cfg.DiscoveryInfo.URL, + DynamicServingCertProvider: dynamicServingCertProvider, + DynamicSigningCertProvider: dynamicSigningCertProvider, + ServingCertDuration: time.Duration(*cfg.APIConfig.ServingCertificateConfig.DurationSeconds) * time.Second, + ServingCertRenewBefore: time.Duration(*cfg.APIConfig.ServingCertificateConfig.RenewBeforeSeconds) * time.Second, + IDPCache: idpCache, }, ) if err != nil { @@ -224,38 +197,3 @@ func getAggregatedAPIServerConfig( } return apiServerConfig, nil } - -func createKubeCertAgentTemplate(cfg *configapi.KubeCertAgentSpec, serverInstallationNamespace string) *corev1.Pod { - terminateImmediately := int64(0) - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: *cfg.NamePrefix, - Namespace: serverInstallationNamespace, // create the agent pods in the same namespace where Pinniped is installed - Labels: map[string]string{ - kubeCertAgentLabelKey: "", - }, - }, - Spec: corev1.PodSpec{ - TerminationGracePeriodSeconds: &terminateImmediately, - Containers: []corev1.Container{ - { - Name: "sleeper", - Image: *cfg.Image, - ImagePullPolicy: corev1.PullIfNotPresent, - Command: []string{"/bin/sleep", "infinity"}, - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("16Mi"), - corev1.ResourceCPU: resource.MustParse("10m"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("16Mi"), - corev1.ResourceCPU: resource.MustParse("10m"), - }, - }, - }, - }, - }, - } - return pod -} diff --git a/test/integration/kubecertagent_test.go b/test/integration/kubecertagent_test.go index 2418fd5c..f266cf99 100644 --- a/test/integration/kubecertagent_test.go +++ b/test/integration/kubecertagent_test.go @@ -21,7 +21,7 @@ import ( ) const ( - kubeCertAgentLabelSelector = "kube-cert-agent.pinniped.dev=" + kubeCertAgentLabelSelector = "kube-cert-agent.pinniped.dev=true" ) func TestKubeCertAgent(t *testing.T) {