diff --git a/internal/controller/issuerconfig/publisher.go b/internal/controller/issuerconfig/publisher.go index a8e087f6..b4e6c56b 100644 --- a/internal/controller/issuerconfig/publisher.go +++ b/internal/controller/issuerconfig/publisher.go @@ -26,32 +26,34 @@ const ( ) type publisherController struct { - namespace string - credentialIssuerConfigResourceName string - serverOverride *string - pinnipedClient pinnipedclientset.Interface - configMapInformer corev1informers.ConfigMapInformer - credentialIssuerConfigInformer configv1alpha1informers.CredentialIssuerConfigInformer + credentialIssuerConfigNamespaceName string + credentialIssuerConfigResourceName string + serverOverride *string + pinnipedClient pinnipedclientset.Interface + configMapInformer corev1informers.ConfigMapInformer + credentialIssuerConfigInformer configv1alpha1informers.CredentialIssuerConfigInformer } -func NewPublisherController(namespace string, +// TODO rename this NewKubeConfigInfoPublisherController, along with the private type and the source/test files. +func NewPublisherController( + credentialIssuerConfigNamespaceName string, credentialIssuerConfigResourceName string, serverOverride *string, pinnipedClient pinnipedclientset.Interface, configMapInformer corev1informers.ConfigMapInformer, - credentialIssuerConfigInformer configv1alpha1informers.CredentialIssuerConfigInformer, + credentialIssuerConfigInformer configv1alpha1informers.CredentialIssuerConfigInformer, // TODO don't have this informer here withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: "publisher-controller", Syncer: &publisherController{ - credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, - namespace: namespace, - serverOverride: serverOverride, - pinnipedClient: pinnipedClient, - configMapInformer: configMapInformer, - credentialIssuerConfigInformer: credentialIssuerConfigInformer, + credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, + credentialIssuerConfigNamespaceName: credentialIssuerConfigNamespaceName, + serverOverride: serverOverride, + pinnipedClient: pinnipedClient, + configMapInformer: configMapInformer, + credentialIssuerConfigInformer: credentialIssuerConfigInformer, }, }, withInformer( @@ -61,7 +63,7 @@ func NewPublisherController(namespace string, ), withInformer( credentialIssuerConfigInformer, - pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(credentialIssuerConfigResourceName, namespace), + pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(credentialIssuerConfigResourceName, credentialIssuerConfigNamespaceName), controllerlib.InformerOption{}, ), ) @@ -110,7 +112,7 @@ func (c *publisherController) Sync(ctx controllerlib.Context) error { existingCredentialIssuerConfigFromInformerCache, err := c.credentialIssuerConfigInformer. Lister(). - CredentialIssuerConfigs(c.namespace). + CredentialIssuerConfigs(c.credentialIssuerConfigNamespaceName). Get(c.credentialIssuerConfigResourceName) notFound = k8serrors.IsNotFound(err) if err != nil && !notFound { @@ -129,7 +131,7 @@ func (c *publisherController) Sync(ctx controllerlib.Context) error { existingCredentialIssuerConfigFromInformerCache, notFound, c.credentialIssuerConfigResourceName, - c.namespace, + c.credentialIssuerConfigNamespaceName, c.pinnipedClient, updateServerAndCAFunc) diff --git a/internal/controller/kubecertagent/annotater.go b/internal/controller/kubecertagent/annotater.go index 2b8aa7d4..a87137f4 100644 --- a/internal/controller/kubecertagent/annotater.go +++ b/internal/controller/kubecertagent/annotater.go @@ -86,6 +86,7 @@ func (c *annotaterController) Sync(ctx controllerlib.Context) error { continue } + // TODO if the paths cannot be found, then still add the annotations anyway using the defaults k8sAPIServerCAKeyPEMDefaultPath and k8sAPIServerCACertPEMDefaultPath certPath, certPathOK := getContainerArgByName(controllerManagerPod, "cluster-signing-cert-file") keyPath, keyPathOK := getContainerArgByName(controllerManagerPod, "cluster-signing-key-file") if err := c.maybeUpdateAgentPod( @@ -97,6 +98,7 @@ func (c *annotaterController) Sync(ctx controllerlib.Context) error { keyPath, keyPathOK, ); err != nil { + // TODO Failed, so update the CIC status? return fmt.Errorf("cannot update agent pod: %w", err) } } diff --git a/internal/controller/kubecertagent/creater.go b/internal/controller/kubecertagent/creater.go index a4643207..e79be5af 100644 --- a/internal/controller/kubecertagent/creater.go +++ b/internal/controller/kubecertagent/creater.go @@ -73,6 +73,8 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { return fmt.Errorf("informer cannot list controller manager pods: %w", err) } + // TODO if controllerManagerPods is empty then update the CIC status with an error message saying that they couldn't be found + for _, controllerManagerPod := range controllerManagerPods { agentPod, err := findAgentPodForSpecificControllerManagerPod( controllerManagerPod, @@ -97,6 +99,7 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { Pods(c.agentInfo.Template.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 return fmt.Errorf("cannot create agent pod: %w", err) } } diff --git a/internal/controller/kubecertagent/execer.go b/internal/controller/kubecertagent/execer.go new file mode 100644 index 00000000..2435c474 --- /dev/null +++ b/internal/controller/kubecertagent/execer.go @@ -0,0 +1,172 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package kubecertagent + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/klog/v2" + + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" + "go.pinniped.dev/internal/certauthority/kubecertauthority" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/issuerconfig" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/provider" +) + +type CurrentTimeProvider interface { + Now() metav1.Time +} + +type currentTimeProvider struct{} + +// TODO use this as the currentTimeProvider argument when calling NewExecerController() from prepare_controllers.go. +func NewCurrentTimeProvider() CurrentTimeProvider { + return ¤tTimeProvider{} +} + +func (f *currentTimeProvider) Now() metav1.Time { + return metav1.Now() +} + +type execerController struct { + agentInfo *Info + credentialIssuerConfigNamespaceName string + credentialIssuerConfigResourceName string + dynamicCertProvider provider.DynamicTLSServingCertProvider + podCommandExecutor kubecertauthority.PodCommandExecutor + currentTimeProvider CurrentTimeProvider + pinnipedAPIClient pinnipedclientset.Interface + agentPodInformer corev1informers.PodInformer +} + +func NewExecerController( + agentInfo *Info, + credentialIssuerConfigNamespaceName string, + credentialIssuerConfigResourceName string, + dynamicCertProvider provider.DynamicTLSServingCertProvider, + podCommandExecutor kubecertauthority.PodCommandExecutor, + pinnipedAPIClient pinnipedclientset.Interface, + currentTimeProvider CurrentTimeProvider, + agentPodInformer corev1informers.PodInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + return controllerlib.New( + controllerlib.Config{ + Name: "kube-cert-agent-execer-controller", + Syncer: &execerController{ + agentInfo: agentInfo, + credentialIssuerConfigNamespaceName: credentialIssuerConfigNamespaceName, + credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, + dynamicCertProvider: dynamicCertProvider, + podCommandExecutor: podCommandExecutor, + currentTimeProvider: currentTimeProvider, + pinnipedAPIClient: pinnipedAPIClient, + agentPodInformer: agentPodInformer, + }, + }, + withInformer( + agentPodInformer, + pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { + return isAgentPod(obj, agentInfo.Template.Labels) + }), + controllerlib.InformerOption{}, + ), + ) +} + +func (c *execerController) Sync(ctx controllerlib.Context) error { + maybeAgentPod, err := c.agentPodInformer.Lister().Pods(ctx.Key.Namespace).Get(ctx.Key.Name) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("failed to get %s/%s pod: %w", ctx.Key.Namespace, ctx.Key.Name, err) + } + if notFound { + // The pod in question does not exist, so it was probably deleted + return nil + } + + certPath, keyPath := c.getKeypairFilePaths(maybeAgentPod) + if certPath == "" || keyPath == "" { + // The annotator controller has not annotated this agent pod yet, or it is not an agent pod at all + return nil + } + agentPod := maybeAgentPod + + if agentPod.Status.Phase != v1.PodRunning { + // Seems to be an agent pod, but it is not ready yet + return nil + } + + certPEM, err := c.podCommandExecutor.Exec(agentPod.Namespace, agentPod.Name, "cat", certPath) + if err != nil { + strategyResultUpdateErr := c.createOrUpdateCredentialIssuerConfig(ctx, c.strategyError(err)) + klog.ErrorS(strategyResultUpdateErr, "could not create or update CredentialIssuerConfig with strategy success") + return err + } + + keyPEM, err := c.podCommandExecutor.Exec(agentPod.Namespace, agentPod.Name, "cat", keyPath) + if err != nil { + strategyResultUpdateErr := c.createOrUpdateCredentialIssuerConfig(ctx, c.strategyError(err)) + klog.ErrorS(strategyResultUpdateErr, "could not create or update CredentialIssuerConfig with strategy success") + return err + } + + c.dynamicCertProvider.Set([]byte(certPEM), []byte(keyPEM)) + + err = c.createOrUpdateCredentialIssuerConfig(ctx, c.strategySuccess()) + _ = err // TODO return this error? (needs test) + + return nil +} + +func (c *execerController) createOrUpdateCredentialIssuerConfig(ctx controllerlib.Context, strategyResult configv1alpha1.CredentialIssuerConfigStrategy) error { + return issuerconfig.CreateOrUpdateCredentialIssuerConfig( + ctx.Context, + c.credentialIssuerConfigNamespaceName, + c.credentialIssuerConfigResourceName, + c.pinnipedAPIClient, + func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { + configToUpdate.Status.Strategies = []configv1alpha1.CredentialIssuerConfigStrategy{strategyResult} + }, + ) +} + +func (c *execerController) strategySuccess() configv1alpha1.CredentialIssuerConfigStrategy { + return configv1alpha1.CredentialIssuerConfigStrategy{ + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: c.currentTimeProvider.Now(), + } +} + +func (c *execerController) strategyError(err error) configv1alpha1.CredentialIssuerConfigStrategy { + return configv1alpha1.CredentialIssuerConfigStrategy{ + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.ErrorStrategyStatus, + Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, + Message: err.Error(), + LastUpdateTime: c.currentTimeProvider.Now(), + } +} + +func (c *execerController) getKeypairFilePaths(pod *v1.Pod) (string, string) { + annotations := pod.Annotations + if annotations == nil { + annotations = make(map[string]string) + } + + certPath := annotations[c.agentInfo.CertPathAnnotation] + keyPath := annotations[c.agentInfo.KeyPathAnnotation] + + return certPath, keyPath +} diff --git a/internal/controller/kubecertagent/execer_test.go b/internal/controller/kubecertagent/execer_test.go new file mode 100644 index 00000000..c5271c28 --- /dev/null +++ b/internal/controller/kubecertagent/execer_test.go @@ -0,0 +1,532 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package kubecertagent + +import ( + "context" + "fmt" + "io/ioutil" + "testing" + "time" + + "github.com/sclevine/spec" + "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" + kubeinformers "k8s.io/client-go/informers" + kubernetesfake "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" + + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/provider" + "go.pinniped.dev/internal/testutil" +) + +type fakeCurrentTimeProvider struct { + frozenNow *metav1.Time +} + +func (f *fakeCurrentTimeProvider) Now() metav1.Time { + if f.frozenNow == nil { + realNow := metav1.Now() + f.frozenNow = &realNow + } + return *f.frozenNow +} + +func TestExecerControllerOptions(t *testing.T) { + spec.Run(t, "options", func(t *testing.T, when spec.G, it spec.S) { + var r *require.Assertions + var observableWithInformerOption *testutil.ObservableWithInformerOption + var agentPodInformerFilter controllerlib.Filter + + 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, + }, + "credentialIssuerConfigNamespaceName", + "credentialIssuerConfigResourceName", + nil, // not needed for this test + nil, // not needed for this test + nil, // not needed for this test + &fakeCurrentTimeProvider{}, + agentPodsInformer, + observableWithInformerOption.WithInformer, + ) + agentPodInformerFilter = observableWithInformerOption.GetFilterForInformer(agentPodsInformer) + }) + + when("the change is happening in the agent's namespace", func() { + when("a pod with all the 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", + }, + }, + } + + r.True(agentPodInformerFilter.Add(pod)) + r.True(agentPodInformerFilter.Update(whateverPod, pod)) + r.True(agentPodInformerFilter.Update(pod, whateverPod)) + r.True(agentPodInformerFilter.Delete(pod)) + }) + }) + + when("a pod missing any of the agent labels is added/updated/deleted", func() { + it("returns false", func() { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "some-other-label-key": "some-other-label-value", + }, + }, + } + + r.False(agentPodInformerFilter.Add(pod)) + r.False(agentPodInformerFilter.Update(whateverPod, pod)) + r.False(agentPodInformerFilter.Update(pod, whateverPod)) + r.False(agentPodInformerFilter.Delete(pod)) + }) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) +} + +type fakePodExecutor struct { + r *require.Assertions + + resultsToReturn []string + errorsToReturn []error + + calledWithPodName []string + calledWithPodNamespace []string + calledWithCommandAndArgs [][]string + + callCount int +} + +func (s *fakePodExecutor) Exec(podNamespace string, podName string, commandAndArgs ...string) (string, error) { + s.calledWithPodNamespace = append(s.calledWithPodNamespace, podNamespace) + s.calledWithPodName = append(s.calledWithPodName, podName) + s.calledWithCommandAndArgs = append(s.calledWithCommandAndArgs, commandAndArgs) + s.r.Less(s.callCount, len(s.resultsToReturn), "unexpected extra invocation of fakePodExecutor") + result := s.resultsToReturn[s.callCount] + var err error = nil + if s.errorsToReturn != nil { + s.r.Less(s.callCount, len(s.errorsToReturn), "unexpected extra invocation of fakePodExecutor") + err = s.errorsToReturn[s.callCount] + } + s.callCount++ + if err != nil { + return "", err + } + return result, nil +} + +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 fakeCertPath = "/some/cert/path" + const fakeKeyPath = "/some/key/path" + const defaultDynamicCertProviderCert = "initial-cert" + const defaultDynamicCertProviderKey = "initial-key" + const credentialIssuerConfigNamespaceName = "cic-namespace-name" + const credentialIssuerConfigResourceName = "cic-resource-name" + + var r *require.Assertions + + var subject controllerlib.Controller + var timeoutContext context.Context + var timeoutContextCancel context.CancelFunc + var syncContext *controllerlib.Context + var pinnipedAPIClient *pinnipedfake.Clientset + var agentPodInformer kubeinformers.SharedInformerFactory + var agentPodInformerClient *kubernetesfake.Clientset + var fakeExecutor *fakePodExecutor + var agentPodTemplate *corev1.Pod + var dynamicCertProvider provider.DynamicTLSServingCertProvider + var fakeCertPEM, fakeKeyPEM string + var fakeNow *fakeCurrentTimeProvider + var credentialIssuerConfigGVR 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 = NewExecerController( + &Info{ + Template: agentPodTemplate, + CertPathAnnotation: certPathAnnotationName, + KeyPathAnnotation: keyPathAnnotationName, + }, + credentialIssuerConfigNamespaceName, + credentialIssuerConfigResourceName, + dynamicCertProvider, + fakeExecutor, + pinnipedAPIClient, + fakeNow, + agentPodInformer.Core().V1().Pods(), + controllerlib.WithInformer, + ) + + // Set this at the last second to support calling subject.Name(). + syncContext = &controllerlib.Context{ + Context: timeoutContext, + Name: subject.Name(), + Key: controllerlib.Key{ + Namespace: agentPodNamespace, + Name: agentPodName, + }, + } + + // Must start informers before calling TestRunSynchronously() + agentPodInformer.Start(timeoutContext.Done()) + controllerlib.TestRunSynchronously(t, subject) + } + + var newAgentPod = func(agentPodName string, hasCertPathAnnotations bool) *corev1.Pod { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: agentPodName, + Namespace: agentPodNamespace, + Labels: map[string]string{ + "some-label-key": "some-label-value", + }, + }, + } + if hasCertPathAnnotations { + pod.Annotations = map[string]string{ + certPathAnnotationName: fakeCertPath, + keyPathAnnotationName: fakeKeyPath, + } + } + return pod + } + + var requireDynamicCertProviderHasDefaultValues = func() { + actualCertPEM, actualKeyPEM := dynamicCertProvider.CurrentCertKeyContent() + r.Equal(defaultDynamicCertProviderCert, string(actualCertPEM)) + r.Equal(defaultDynamicCertProviderKey, string(actualKeyPEM)) + } + + var requireNoExternalActionsTaken = func() { + r.Empty(pinnipedAPIClient.Actions()) + r.Zero(fakeExecutor.callCount) + requireDynamicCertProviderHasDefaultValues() + } + + it.Before(func() { + r = require.New(t) + + timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) + pinnipedAPIClient = pinnipedfake.NewSimpleClientset() + agentPodInformerClient = kubernetesfake.NewSimpleClientset() + agentPodInformer = kubeinformers.NewSharedInformerFactory(agentPodInformerClient, 0) + fakeExecutor = &fakePodExecutor{r: r} + fakeNow = &fakeCurrentTimeProvider{} + fakeNow.Now() // call once to initialize + dynamicCertProvider = provider.NewDynamicTLSServingCertProvider() + dynamicCertProvider.Set([]byte(defaultDynamicCertProviderCert), []byte(defaultDynamicCertProviderKey)) + + loadFile := func(filename string) string { + bytes, err := ioutil.ReadFile(filename) + r.NoError(err) + return string(bytes) + } + 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, + Resource: "credentialissuerconfigs", + } + }) + + it.After(func() { + timeoutContextCancel() + }) + + when("there is not yet any agent pods or they were deleted", func() { + it.Before(func() { + unrelatedPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some other pod", + Namespace: agentPodNamespace, + }, + } + r.NoError(agentPodInformerClient.Tracker().Add(unrelatedPod)) + startInformersAndController() + }) + + it("does nothing", func() { + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + requireNoExternalActionsTaken() + }) + }) + + when("there is an agent pod, as determined by its labels matching the agent pod template labels, which is not yet annotated by the annotater controller", func() { + it.Before(func() { + agentPod := newAgentPod(agentPodName, false) + r.NoError(agentPodInformerClient.Tracker().Add(agentPod)) + startInformersAndController() + }) + + it("does nothing", func() { + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + requireNoExternalActionsTaken() + }) + }) + + when("there is an agent pod, as determined by its labels matching the agent pod template labels, and it was annotated by the annotater controller, but it is not Running", func() { + it.Before(func() { + agentPod := newAgentPod(agentPodName, true) + agentPod.Status.Phase = corev1.PodPending // not Running + r.NoError(agentPodInformerClient.Tracker().Add(agentPod)) + startInformersAndController() + }) + + it("does nothing", func() { + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + requireNoExternalActionsTaken() + }) + }) + + when("there is an agent pod, as determined by its labels matching the agent pod template labels, which is already annotated by the annotater controller, and it is Running", func() { + it.Before(func() { + targetAgentPod := newAgentPod(agentPodName, true) + targetAgentPod.Status.Phase = corev1.PodRunning + anotherAgentPod := newAgentPod("some-other-agent-pod-which-is-not-the-context-of-this-sync", true) + r.NoError(agentPodInformerClient.Tracker().Add(targetAgentPod)) + r.NoError(agentPodInformerClient.Tracker().Add(anotherAgentPod)) + }) + + when("the resulting pod execs will succeed", func() { + it.Before(func() { + fakeExecutor.resultsToReturn = []string{fakeCertPEM, fakeKeyPEM} + }) + + it("execs to the agent pod to get the keys and updates the dynamic certificates provider with the new certs", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + r.Equal(2, fakeExecutor.callCount) + + r.Equal(agentPodNamespace, fakeExecutor.calledWithPodNamespace[0]) + r.Equal(agentPodName, fakeExecutor.calledWithPodName[0]) + r.Equal([]string{"cat", fakeCertPath}, fakeExecutor.calledWithCommandAndArgs[0]) + + r.Equal(agentPodNamespace, fakeExecutor.calledWithPodNamespace[1]) + r.Equal(agentPodName, fakeExecutor.calledWithPodName[1]) + r.Equal([]string{"cat", fakeKeyPath}, fakeExecutor.calledWithCommandAndArgs[1]) + + actualCertPEM, actualKeyPEM := dynamicCertProvider.CurrentCertKeyContent() + r.Equal(fakeCertPEM, string(actualCertPEM)) + r.Equal(fakeKeyPEM, string(actualKeyPEM)) + }) + + when("there is already a CredentialIssuerConfig", func() { + var initialCredentialIssuerConfig *configv1alpha1.CredentialIssuerConfig + + it.Before(func() { + initialCredentialIssuerConfig = &configv1alpha1.CredentialIssuerConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerConfigResourceName, + Namespace: credentialIssuerConfigNamespaceName, + }, + Status: configv1alpha1.CredentialIssuerConfigStatus{ + Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{}, + KubeConfigInfo: &configv1alpha1.CredentialIssuerConfigKubeConfigInfo{ + Server: "some-server", + CertificateAuthorityData: "some-ca-value", + }, + }, + } + r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuerConfig)) + }) + + it("also updates the the existing CredentialIssuerConfig status field", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + expectedCredentialIssuerConfig := initialCredentialIssuerConfig.DeepCopy() + expectedCredentialIssuerConfig.Status.Strategies = []configv1alpha1.CredentialIssuerConfigStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: fakeNow.Now(), + }, + } + expectedGetAction := coretesting.NewGetAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, credentialIssuerConfigResourceName) + expectedCreateAction := coretesting.NewUpdateAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, expectedCredentialIssuerConfig) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + }) + }) + + when("there is not already a CredentialIssuerConfig", func() { + it.Before(func() { + startInformersAndController() + }) + + it("also creates the the CredentialIssuerConfig with the appropriate status field", func() { + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + expectedCredentialIssuerConfig := &configv1alpha1.CredentialIssuerConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerConfigResourceName, + Namespace: credentialIssuerConfigNamespaceName, + }, + Status: configv1alpha1.CredentialIssuerConfigStatus{ + Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: fakeNow.Now(), + }, + }, + }, + } + expectedGetAction := coretesting.NewGetAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, credentialIssuerConfigResourceName) + expectedCreateAction := coretesting.NewCreateAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, expectedCredentialIssuerConfig) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + }) + }) + }) + + when("the first resulting pod exec will fail", func() { + var podExecErrorMessage string + + it.Before(func() { + podExecErrorMessage = "some pod exec error message" + fakeExecutor.errorsToReturn = []error{fmt.Errorf(podExecErrorMessage), nil} + fakeExecutor.resultsToReturn = []string{"", fakeKeyPEM} + startInformersAndController() + }) + + it("does not update the dynamic certificates provider", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + requireDynamicCertProviderHasDefaultValues() + }) + + it("creates or updates the the CredentialIssuerConfig status field with an error", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + + expectedCredentialIssuerConfig := &configv1alpha1.CredentialIssuerConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerConfigResourceName, + Namespace: credentialIssuerConfigNamespaceName, + }, + Status: configv1alpha1.CredentialIssuerConfigStatus{ + Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.ErrorStrategyStatus, + Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, + Message: podExecErrorMessage, + LastUpdateTime: metav1.Now(), + }, + }, + }, + } + expectedGetAction := coretesting.NewGetAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, credentialIssuerConfigResourceName) + expectedCreateAction := coretesting.NewCreateAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, expectedCredentialIssuerConfig) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + }) + }) + + when("the second resulting pod exec will fail", func() { + var podExecErrorMessage string + + it.Before(func() { + podExecErrorMessage = "some pod exec error message" + fakeExecutor.errorsToReturn = []error{nil, fmt.Errorf(podExecErrorMessage)} + fakeExecutor.resultsToReturn = []string{fakeCertPEM, ""} + startInformersAndController() + }) + + it("does not update the dynamic certificates provider", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + requireDynamicCertProviderHasDefaultValues() + }) + + it("creates or updates the the CredentialIssuerConfig status field with an error", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), podExecErrorMessage) + + expectedCredentialIssuerConfig := &configv1alpha1.CredentialIssuerConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerConfigResourceName, + Namespace: credentialIssuerConfigNamespaceName, + }, + Status: configv1alpha1.CredentialIssuerConfigStatus{ + Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.ErrorStrategyStatus, + Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, + Message: podExecErrorMessage, + LastUpdateTime: metav1.Now(), + }, + }, + }, + } + expectedGetAction := coretesting.NewGetAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, credentialIssuerConfigResourceName) + expectedCreateAction := coretesting.NewCreateAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, expectedCredentialIssuerConfig) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + }) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) +} diff --git a/internal/controller/kubecertagent/testdata/test.crt b/internal/controller/kubecertagent/testdata/test.crt new file mode 100644 index 00000000..796a7690 --- /dev/null +++ b/internal/controller/kubecertagent/testdata/test.crt @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICyDCCAbCgAwIBAgIBADANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwprdWJl +cm5ldGVzMB4XDTIwMDcyNTIxMDQxOFoXDTMwMDcyMzIxMDQxOFowFTETMBEGA1UE +AxMKa3ViZXJuZXRlczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL3K +hYv2gIQ1Dwzh2cWMid+ofAnvLIfV2Xv61vTLGprUI+XUqB4/gtf6X6UNn0Lett2n +d8p4wy7hw73hU/ggdvmWJvqBrSjc3JGfy+kj66fKXX+PTlbL7QbwiRvcSqIXIWlV +lHHxECWrED8jCulw/NVqfook/h5iNUCT9yswSJr/0fImiVnoTlIoEYG2eCNejZ5c +g39uD3ZTqd9ZxWwSLLnI+2kpJnZBPcd1ZQ8AQqzDgZtYRCqacn5gckQUKZWKQlxo +Eft6g1XHJouAWAZw7hEtk0v8rG0/eKF7wamxFi6BFVlbjWBsB4T9rApbdBWTKeCJ +Hv8fv5RMFSzpT3uzTO8CAwEAAaMjMCEwDgYDVR0PAQH/BAQDAgKkMA8GA1UdEwEB +/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBACh5RhbxqJe+Z/gc17cZhKNmdiwu +I2pLp3QBfwvN+Wbmajzw/7rYhY0d8JYVTJzXSCPWi6UAKxAtXOLF8WIIf9i39n6R +uKOBGW14FzzGyRJiD3qaG/JTvEW+SLhwl68Ndr5LHSnbugAqq31abcQy6Zl9v5A8 +JKC97Lj/Sn8rj7opKy4W3oq7NCQsAb0zh4IllRF6UvSnJySfsg7xdXHHpxYDHtOS +XcOu5ySUIZTgFe9RfeUZlGZ5xn0ckMlQ7qW2Wx1q0OVWw5us4NtkGqKrHG4Tn1X7 +uwo/Yytn5sDxrDv1/oii6AZOCsTPre4oD3wz4nmVzCVJcgrqH4Q24hT8WNg= +-----END CERTIFICATE----- diff --git a/internal/controller/kubecertagent/testdata/test.key b/internal/controller/kubecertagent/testdata/test.key new file mode 100644 index 00000000..7ad653ae --- /dev/null +++ b/internal/controller/kubecertagent/testdata/test.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAvcqFi/aAhDUPDOHZxYyJ36h8Ce8sh9XZe/rW9MsamtQj5dSo +Hj+C1/pfpQ2fQt623ad3ynjDLuHDveFT+CB2+ZYm+oGtKNzckZ/L6SPrp8pdf49O +VsvtBvCJG9xKohchaVWUcfEQJasQPyMK6XD81Wp+iiT+HmI1QJP3KzBImv/R8iaJ +WehOUigRgbZ4I16NnlyDf24PdlOp31nFbBIsucj7aSkmdkE9x3VlDwBCrMOBm1hE +KppyfmByRBQplYpCXGgR+3qDVccmi4BYBnDuES2TS/ysbT94oXvBqbEWLoEVWVuN +YGwHhP2sClt0FZMp4Ike/x+/lEwVLOlPe7NM7wIDAQABAoIBAFC1tUEmHNUcM0BJ +M3D9KQzB+63F1mwVlx1QOOV1EeVR3co5Ox1R6PSr9sycFGQ9jgqI0zp5TJe9Tp6L +GkhklfPh1MWnK9o6wlnzWKXWrrp2Jni+mpPyuOPAmq4Maniv2XeP+0bROwqpyojv +AA7yC7M+TH226ZJGNVs3EV9+cwHml0yuzBfIJn/rv/w2g+WRKM/MC0S7k2d8bRlA +NycKVGAGBhKTltjoVYOeh6aHEpSjK8zfaePjo5dYJvoVIli60YCgcJOU/8jXT+Np +1Fm7tRvAtj3pUp0Sqdaf2RUzh9jfJp2VFCHuSJ6TPqArOyQojtMcTHF0TiW7xrHP +xOCRIAECgYEAwGBPU7vdthMJBg+ORUoGQQaItTeJvQwIqJvbKD2osp4jhS1dGZBw +W30GKEc/gd8JNtOq9BBnMicPF7hktuy+bSPv41XPud67rSSO7Tsw20C10gFRq06B +zIJWFAUqK3IkvVc3VDmtSLSDox4QZ/BdqaMlQ5y5JCsC5kThmkZFlO8CgYEA/I9X +YHi6RioMJE1fqOHJL4DDjlezmcuRrD7fE5InKbtJZ2JhGYOX/C0KXnHTOWTCDxxN +FBvpvD6Xv5o3PhB9Z6k2fqvJ4GS8urkG/KU4xcC+bak+9ava8oaiSqG16zD9NH2P +jJ60NrbLl1J0pU9fiwuFVUKJ4hDZOfN9RqYdyAECgYAVwo8WhJiGgM6zfcz073OX +pVqPTPHqjVLpZ3+5pIfRdGvGI6R1QM5EuvaYVb7MPOM47WZX5wcVOC/P2g6iVlMP +21HGIC2384a9BfaYxOo40q/+SiHnw6CQ9mkwKIllkqqvNA9RGpkMMUb2i28For2l +c4vCgxa6DZdtXns6TRqPxwKBgCfY5cxOv/T6BVhk7MbUeM2J31DB/ZAyUhV/Bess +kAlBh19MYk2IOZ6L7KriApV3lDaWHIMjtEkDByYvyq98Io0MYZCywfMpca10K+oI +l2B7/I+IuGpCZxUEsO5dfTpSTGDPvqpND9niFVUWqVi7oTNq6ep9yQtl5SADjqxq +4SABAoGAIm0hUg1wtcS46cGLy6PIkPM5tocTSghtz4vFsuk/i4QA9GBoBO2gH6ty ++kJHmeaXt2dmgySp0QAWit5UlceEumB0NXnAdJZQxeGSFSyYkDWhwXd8wDceKo/1 +LfCU6Dk8IN/SsppVUWXQ2rlORvxlrHeCio8o0kS9Yiu55WMYg4g= +-----END RSA PRIVATE KEY----- diff --git a/internal/provider/dynamic_tls_serving_cert_provider.go b/internal/provider/dynamic_tls_serving_cert_provider.go index 53338848..e5beab27 100644 --- a/internal/provider/dynamic_tls_serving_cert_provider.go +++ b/internal/provider/dynamic_tls_serving_cert_provider.go @@ -20,6 +20,7 @@ type dynamicTLSServingCertProvider struct { mutex sync.RWMutex } +// TODO rename this type to DynamicCertProvider, since we are now going to use it for other types of certs too func NewDynamicTLSServingCertProvider() DynamicTLSServingCertProvider { return &dynamicTLSServingCertProvider{} } diff --git a/internal/server/server.go b/internal/server/server.go index 215d170b..2c84cbaf 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -134,6 +134,7 @@ func (a *App) runServer(ctx context.Context) error { &cfg.KubeCertAgentConfig, serverInstallationNamespace, ) + // TODO replace this with our new controller k8sClusterCA, shutdownCA, err := getClusterCASigner( ctx, serverInstallationNamespace, @@ -159,10 +160,11 @@ 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, - DynamicCertProvider: dynamicCertProvider, + ServerInstallationNamespace: serverInstallationNamespace, + NamesConfig: &cfg.NamesConfig, + DiscoveryURLOverride: cfg.DiscoveryInfo.URL, + DynamicCertProvider: dynamicCertProvider, + //KubeAPISigningCertProvider: nil, // TODO pass this as a NewDynamicTLSServingCertProvider(), so it can be passed into the new controller ServingCertDuration: time.Duration(*cfg.APIConfig.ServingCertificateConfig.DurationSeconds) * time.Second, ServingCertRenewBefore: time.Duration(*cfg.APIConfig.ServingCertificateConfig.RenewBeforeSeconds) * time.Second, IDPCache: idpCache, @@ -179,7 +181,7 @@ func (a *App) runServer(ctx context.Context) error { aggregatedAPIServerConfig, err := getAggregatedAPIServerConfig( dynamicCertProvider, idpCache, - k8sClusterCA, + k8sClusterCA, // TODO pass the same instance of DynamicTLSServingCertProvider as above, but wrapped into a new type that implements credentialrequest.CertIssuer, which should return ErrIncapableOfIssuingCertificates until the certs are available startControllersFunc, ) if err != nil { diff --git a/test/integration/credentialissuerconfig_test.go b/test/integration/credentialissuerconfig_test.go index befec19d..3cde0579 100644 --- a/test/integration/credentialissuerconfig_test.go +++ b/test/integration/credentialissuerconfig_test.go @@ -78,6 +78,7 @@ func TestCredentialIssuerConfig(t *testing.T) { // Mutate the existing object. Don't delete it because that would mess up its `Status.Strategies` array, // since the reconciling controller is not currently responsible for that field. updatedServerValue := "https://junk" + // TODO maybe mutate the kube-info configmap's CA value instead, because that's the object that we care to check that the controller is watching existingConfig.Status.KubeConfigInfo.Server = updatedServerValue updatedConfig, err := client. ConfigV1alpha1().