diff --git a/internal/controller/issuerconfig/kube_config_info_publisher.go b/internal/controller/issuerconfig/kube_config_info_publisher.go deleted file mode 100644 index 5a940811..00000000 --- a/internal/controller/issuerconfig/kube_config_info_publisher.go +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package issuerconfig - -import ( - "encoding/base64" - "fmt" - - k8serrors "k8s.io/apimachinery/pkg/api/errors" - corev1informers "k8s.io/client-go/informers/core/v1" - "k8s.io/client-go/tools/clientcmd" - "k8s.io/klog/v2" - - configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" - pinnipedclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" - pinnipedcontroller "go.pinniped.dev/internal/controller" - "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/plog" -) - -const ( - ClusterInfoNamespace = "kube-public" - clusterInfoName = "cluster-info" - clusterInfoConfigMapKey = "kubeconfig" -) - -type kubeConigInfoPublisherController struct { - credentialIssuerResourceName string - credentialIssuerLabels map[string]string - serverOverride *string - pinnipedClient pinnipedclientset.Interface - configMapInformer corev1informers.ConfigMapInformer -} - -// NewKubeConfigInfoPublisherController returns a controller that syncs the -// configv1alpha1.CredentialIssuer.Status.KubeConfigInfo field with the cluster-info ConfigMap -// in the kube-public namespace. -func NewKubeConfigInfoPublisherController( - credentialIssuerResourceName string, - credentialIssuerLabels map[string]string, - serverOverride *string, - pinnipedClient pinnipedclientset.Interface, - configMapInformer corev1informers.ConfigMapInformer, - withInformer pinnipedcontroller.WithInformerOptionFunc, -) controllerlib.Controller { - return controllerlib.New( - controllerlib.Config{ - Name: "publisher-controller", - Syncer: &kubeConigInfoPublisherController{ - credentialIssuerResourceName: credentialIssuerResourceName, - credentialIssuerLabels: credentialIssuerLabels, - serverOverride: serverOverride, - pinnipedClient: pinnipedClient, - configMapInformer: configMapInformer, - }, - }, - withInformer( - configMapInformer, - pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(clusterInfoName, ClusterInfoNamespace), - controllerlib.InformerOption{}, - ), - ) -} - -func (c *kubeConigInfoPublisherController) Sync(ctx controllerlib.Context) error { - configMap, err := c.configMapInformer. - Lister(). - ConfigMaps(ClusterInfoNamespace). - Get(clusterInfoName) - notFound := k8serrors.IsNotFound(err) - if err != nil && !notFound { - return fmt.Errorf("failed to get %s configmap: %w", clusterInfoName, err) - } - if notFound { - plog.Debug( - "could not find config map", - "configmap", - klog.KRef(ClusterInfoNamespace, clusterInfoName), - ) - return nil - } - - kubeConfig, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey] - if !kubeConfigPresent { - plog.Debug("could not find kubeconfig configmap key") - return nil - } - - config, err := clientcmd.Load([]byte(kubeConfig)) - if err != nil { - plog.Debug("could not load kubeconfig configmap key") - return nil - } - - var certificateAuthorityData, server string - for _, v := range config.Clusters { - certificateAuthorityData = base64.StdEncoding.EncodeToString(v.CertificateAuthorityData) - server = v.Server - break - } - - if c.serverOverride != nil { - server = *c.serverOverride - } - - updateServerAndCAFunc := func(c *configv1alpha1.CredentialIssuerStatus) { - c.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: server, - CertificateAuthorityData: certificateAuthorityData, - } - } - - return CreateOrUpdateCredentialIssuerStatus( - ctx.Context, - c.credentialIssuerResourceName, - c.credentialIssuerLabels, - c.pinnipedClient, - updateServerAndCAFunc, - ) -} diff --git a/internal/controller/issuerconfig/kube_config_info_publisher_test.go b/internal/controller/issuerconfig/kube_config_info_publisher_test.go deleted file mode 100644 index c11c8758..00000000 --- a/internal/controller/issuerconfig/kube_config_info_publisher_test.go +++ /dev/null @@ -1,444 +0,0 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package issuerconfig - -import ( - "context" - "errors" - "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" - "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/latest/apis/concierge/config/v1alpha1" - pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" - "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/here" - "go.pinniped.dev/internal/testutil" -) - -func TestInformerFilters(t *testing.T) { - spec.Run(t, "informer filters", func(t *testing.T, when spec.G, it spec.S) { - const credentialIssuerResourceName = "some-resource-name" - - var r *require.Assertions - var observableWithInformerOption *testutil.ObservableWithInformerOption - var configMapInformerFilter controllerlib.Filter - - it.Before(func() { - r = require.New(t) - observableWithInformerOption = testutil.NewObservableWithInformerOption() - configMapInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().ConfigMaps() - _ = NewKubeConfigInfoPublisherController( - credentialIssuerResourceName, - map[string]string{}, - nil, - nil, - configMapInformer, - observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters - ) - configMapInformerFilter = observableWithInformerOption.GetFilterForInformer(configMapInformer) - }) - - when("watching ConfigMap objects", func() { - var subject controllerlib.Filter - var target, wrongNamespace, wrongName, unrelated *corev1.ConfigMap - - it.Before(func() { - subject = configMapInformerFilter - target = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}} - wrongNamespace = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "wrong-namespace"}} - wrongName = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "kube-public"}} - unrelated = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}} - }) - - when("the target ConfigMap changes", func() { - it("returns true to trigger the sync method", func() { - r.True(subject.Add(target)) - r.True(subject.Update(target, unrelated)) - r.True(subject.Update(unrelated, target)) - r.True(subject.Delete(target)) - }) - }) - - when("a ConfigMap from another namespace changes", func() { - it("returns false to avoid triggering the sync method", func() { - r.False(subject.Add(wrongNamespace)) - r.False(subject.Update(wrongNamespace, unrelated)) - r.False(subject.Update(unrelated, wrongNamespace)) - r.False(subject.Delete(wrongNamespace)) - }) - }) - - when("a ConfigMap with a different name changes", func() { - it("returns false to avoid triggering the sync method", func() { - r.False(subject.Add(wrongName)) - r.False(subject.Update(wrongName, unrelated)) - r.False(subject.Update(unrelated, wrongName)) - r.False(subject.Delete(wrongName)) - }) - }) - - when("a ConfigMap with a different name and a different namespace changes", func() { - it("returns false to avoid triggering the sync method", func() { - r.False(subject.Add(unrelated)) - r.False(subject.Update(unrelated, unrelated)) - r.False(subject.Delete(unrelated)) - }) - }) - }) - }, spec.Parallel(), spec.Report(report.Terminal{})) -} - -func TestSync(t *testing.T) { - spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { - const credentialIssuerResourceName = "some-resource-name" - - var r *require.Assertions - - var subject controllerlib.Controller - var serverOverride *string - var kubeInformerClient *kubernetesfake.Clientset - var kubeInformers kubeinformers.SharedInformerFactory - var pinnipedAPIClient *pinnipedfake.Clientset - var timeoutContext context.Context - var timeoutContextCancel context.CancelFunc - var syncContext *controllerlib.Context - - var expectedCredentialIssuer = func(expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *configv1alpha1.CredentialIssuer, *configv1alpha1.CredentialIssuer) { - expectedCredentialIssuerGVR := schema.GroupVersionResource{ - Group: configv1alpha1.GroupName, - Version: "v1alpha1", - Resource: "credentialissuers", - } - - expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ - ObjectMeta: metav1.ObjectMeta{ - Name: credentialIssuerResourceName, - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - }, - } - - expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ - ObjectMeta: metav1.ObjectMeta{ - Name: credentialIssuerResourceName, - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - }, - Status: configv1alpha1.CredentialIssuerStatus{ - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: expectedServerURL, - CertificateAuthorityData: expectedCAData, - }, - }, - } - return expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer - } - - // 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 = NewKubeConfigInfoPublisherController( - credentialIssuerResourceName, - map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - serverOverride, - pinnipedAPIClient, - kubeInformers.Core().V1().ConfigMaps(), - 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: "kube-public", - Name: "cluster-info", - }, - } - - // Must start informers before calling TestRunSynchronously() - kubeInformers.Start(timeoutContext.Done()) - controllerlib.TestRunSynchronously(t, subject) - } - - it.Before(func() { - r = require.New(t) - - timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) - - kubeInformerClient = kubernetesfake.NewSimpleClientset() - kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0) - pinnipedAPIClient = pinnipedfake.NewSimpleClientset() - }) - - it.After(func() { - timeoutContextCancel() - }) - - when("there is a cluster-info ConfigMap in the kube-public namespace", func() { - const caData = "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=" // "some-certificate-authority-data" base64 encoded - const kubeServerURL = "https://some-server" - - when("the ConfigMap has the expected `kubeconfig` top-level data key", func() { - it.Before(func() { - clusterInfoConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, - // Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below. - Data: map[string]string{ - "kubeconfig": here.Docf(` - kind: Config - apiVersion: v1 - clusters: - - name: "" - cluster: - certificate-authority-data: "%s" - server: "%s"`, - caData, kubeServerURL), - "uninteresting-key": "uninteresting-value", - }, - } - err := kubeInformerClient.Tracker().Add(clusterInfoConfigMap) - r.NoError(err) - }) - - when("the CredentialIssuer does not already exist", func() { - it("creates a CredentialIssuer", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - - expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer := expectedCredentialIssuer( - kubeServerURL, - caData, - ) - - r.Equal( - []coretesting.Action{ - coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCreateCredentialIssuer.Name), - coretesting.NewRootCreateAction( - expectedCredentialIssuerGVR, - expectedCreateCredentialIssuer, - ), - coretesting.NewRootUpdateSubresourceAction( - expectedCredentialIssuerGVR, - "status", - expectedCredentialIssuer, - ), - }, - pinnipedAPIClient.Actions(), - ) - }) - - when("creating the CredentialIssuer fails", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor( - "create", - "credentialissuers", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("create failed") - }, - ) - }) - - it("returns the create error", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.EqualError(err, "could not create or update credentialissuer: create failed: create failed") - }) - }) - - when("a server override is passed to the controller", func() { - it("uses the server override field", func() { - serverOverride = new(string) - *serverOverride = "https://some-server-override" - - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - - expectedCredentialIssuerGVR, expectedCreateCredentialIssuer, expectedCredentialIssuer := expectedCredentialIssuer( - kubeServerURL, - caData, - ) - expectedCredentialIssuer.Status.KubeConfigInfo.Server = "https://some-server-override" - - r.Equal( - []coretesting.Action{ - coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCreateCredentialIssuer.Name), - coretesting.NewRootCreateAction( - expectedCredentialIssuerGVR, - expectedCreateCredentialIssuer, - ), - coretesting.NewRootUpdateSubresourceAction( - expectedCredentialIssuerGVR, - "status", - expectedCredentialIssuer, - ), - }, - pinnipedAPIClient.Actions(), - ) - }) - }) - }) - - when("the CredentialIssuer already exists", func() { - when("the CredentialIssuer is already up to date according to the data in the ConfigMap", func() { - var credentialIssuerGVR schema.GroupVersionResource - var credentialIssuer *configv1alpha1.CredentialIssuer - - it.Before(func() { - credentialIssuerGVR, _, credentialIssuer = expectedCredentialIssuer( - kubeServerURL, - caData, - ) - err := pinnipedAPIClient.Tracker().Add(credentialIssuer) - r.NoError(err) - }) - - it("does not update the CredentialIssuer to avoid unnecessary etcd writes/api calls", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - - r.Equal( - []coretesting.Action{ - coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuer.Name), - }, - pinnipedAPIClient.Actions(), - ) - }) - }) - - when("the CredentialIssuer is stale compared to the data in the ConfigMap", func() { - it.Before(func() { - _, _, expectedCredentialIssuer := expectedCredentialIssuer( - kubeServerURL, - caData, - ) - expectedCredentialIssuer.Status.KubeConfigInfo.Server = "https://some-other-server" - r.NoError(pinnipedAPIClient.Tracker().Add(expectedCredentialIssuer)) - }) - - it("updates the existing CredentialIssuer", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - - expectedCredentialIssuerGVR, _, expectedCredentialIssuer := expectedCredentialIssuer( - kubeServerURL, - caData, - ) - expectedActions := []coretesting.Action{ - coretesting.NewRootGetAction(expectedCredentialIssuerGVR, expectedCredentialIssuer.Name), - coretesting.NewRootUpdateSubresourceAction( - expectedCredentialIssuerGVR, - "status", - expectedCredentialIssuer, - ), - } - r.Equal(expectedActions, pinnipedAPIClient.Actions()) - }) - - when("updating the CredentialIssuer fails", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor( - "update", - "credentialissuers", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("update failed") - }, - ) - }) - - it("returns the update error", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.EqualError(err, "could not create or update credentialissuer: update failed") - }) - }) - }) - }) - }) - - when("the ConfigMap is missing the expected `kubeconfig` top-level data key", func() { - it.Before(func() { - clusterInfoConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, - Data: map[string]string{ - "these are not the droids you're looking for": "uninteresting-value", - }, - } - err := kubeInformerClient.Tracker().Add(clusterInfoConfigMap) - r.NoError(err) - }) - - it("keeps waiting for it to exist", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - r.Empty(pinnipedAPIClient.Actions()) - }) - }) - - when("the ConfigMap does not have a valid kubeconfig", func() { - it.Before(func() { - clusterInfoConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, - Data: map[string]string{ - "kubeconfig": "this is an invalid kubeconfig", - }, - } - err := kubeInformerClient.Tracker().Add(clusterInfoConfigMap) - r.NoError(err) - }) - - it("keeps waiting for it to be properly formatted", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - r.Empty(pinnipedAPIClient.Actions()) - }) - }) - }) - - when("there is not a cluster-info ConfigMap in the kube-public namespace", func() { - it.Before(func() { - unrelatedConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oops this is not the cluster-info ConfigMap", - Namespace: "kube-public", - }, - } - err := kubeInformerClient.Tracker().Add(unrelatedConfigMap) - r.NoError(err) - }) - - it("keeps waiting for one", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) - r.Empty(pinnipedAPIClient.Actions()) - }) - }) - }, spec.Parallel(), spec.Report(report.Terminal{})) -} diff --git a/internal/controller/issuerconfig/update_strategy.go b/internal/controller/issuerconfig/update_strategy.go index 33a5ecd8..669516ff 100644 --- a/internal/controller/issuerconfig/update_strategy.go +++ b/internal/controller/issuerconfig/update_strategy.go @@ -42,6 +42,14 @@ func mergeStrategy(configToUpdate *v1alpha1.CredentialIssuerStatus, strategy v1a configToUpdate.Strategies = append(configToUpdate.Strategies, strategy) } sort.Stable(sortableStrategies(configToUpdate.Strategies)) + + // Special case: the "TokenCredentialRequestAPI" data is mirrored into the deprecated status.kubeConfigInfo field. + if strategy.Frontend != nil && strategy.Frontend.Type == v1alpha1.TokenCredentialRequestAPIFrontendType { + configToUpdate.KubeConfigInfo = &v1alpha1.CredentialIssuerKubeConfigInfo{ + Server: strategy.Frontend.TokenCredentialRequestAPIInfo.Server, + CertificateAuthorityData: strategy.Frontend.TokenCredentialRequestAPIInfo.CertificateAuthorityData, + } + } } // TODO: sort strategies by server preference rather than alphanumerically by type. diff --git a/internal/controller/issuerconfig/update_strategy_test.go b/internal/controller/issuerconfig/update_strategy_test.go index f261c872..b1b90429 100644 --- a/internal/controller/issuerconfig/update_strategy_test.go +++ b/internal/controller/issuerconfig/update_strategy_test.go @@ -47,6 +47,48 @@ func TestMergeStrategy(t *testing.T) { }, }, }, + { + name: "new entry updating deprecated kubeConfigInfo", + configToUpdate: v1alpha1.CredentialIssuerStatus{ + Strategies: nil, + }, + strategy: v1alpha1.CredentialIssuerStrategy{ + Type: "Type1", + Status: v1alpha1.SuccessStrategyStatus, + Reason: "some reason", + Message: "some message", + LastUpdateTime: t1, + Frontend: &v1alpha1.CredentialIssuerFrontend{ + Type: "TokenCredentialRequestAPI", + TokenCredentialRequestAPIInfo: &v1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://test-server", + CertificateAuthorityData: "test-ca-bundle", + }, + }, + }, + expected: v1alpha1.CredentialIssuerStatus{ + Strategies: []v1alpha1.CredentialIssuerStrategy{ + { + Type: "Type1", + Status: v1alpha1.SuccessStrategyStatus, + Reason: "some reason", + Message: "some message", + LastUpdateTime: t1, + Frontend: &v1alpha1.CredentialIssuerFrontend{ + Type: "TokenCredentialRequestAPI", + TokenCredentialRequestAPIInfo: &v1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://test-server", + CertificateAuthorityData: "test-ca-bundle", + }, + }, + }, + }, + KubeConfigInfo: &v1alpha1.CredentialIssuerKubeConfigInfo{ + Server: "https://test-server", + CertificateAuthorityData: "test-ca-bundle", + }, + }, + }, { name: "existing entry to update", configToUpdate: v1alpha1.CredentialIssuerStatus{ diff --git a/internal/controller/kubecertagent/annotater_test.go b/internal/controller/kubecertagent/annotater_test.go index 164cb7b0..b60cfad8 100644 --- a/internal/controller/kubecertagent/annotater_test.go +++ b/internal/controller/kubecertagent/annotater_test.go @@ -238,10 +238,6 @@ func TestAnnotaterControllerSync(t *testing.T) { }, Status: configv1alpha1.CredentialIssuerStatus{ Strategies: []configv1alpha1.CredentialIssuerStrategy{}, - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "some-server", - CertificateAuthorityData: "some-ca-value", - }, }, } r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuer)) diff --git a/internal/controller/kubecertagent/creater_test.go b/internal/controller/kubecertagent/creater_test.go index 18df3e1f..eab57997 100644 --- a/internal/controller/kubecertagent/creater_test.go +++ b/internal/controller/kubecertagent/creater_test.go @@ -309,10 +309,6 @@ func TestCreaterControllerSync(t *testing.T) { }, Status: configv1alpha1.CredentialIssuerStatus{ Strategies: []configv1alpha1.CredentialIssuerStrategy{}, - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "some-server", - CertificateAuthorityData: "some-ca-value", - }, }, } r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuer)) @@ -449,10 +445,6 @@ func TestCreaterControllerSync(t *testing.T) { }, Status: configv1alpha1.CredentialIssuerStatus{ Strategies: []configv1alpha1.CredentialIssuerStrategy{}, - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "some-server", - CertificateAuthorityData: "some-ca-value", - }, }, } r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuer)) diff --git a/internal/controller/kubecertagent/execer.go b/internal/controller/kubecertagent/execer.go index 2322103e..0c1e7638 100644 --- a/internal/controller/kubecertagent/execer.go +++ b/internal/controller/kubecertagent/execer.go @@ -4,14 +4,18 @@ package kubecertagent import ( + "encoding/base64" "fmt" v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/clock" corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" + configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controller/issuerconfig" @@ -19,13 +23,21 @@ import ( "go.pinniped.dev/internal/dynamiccert" ) +const ( + ClusterInfoNamespace = "kube-public" + clusterInfoName = "cluster-info" + clusterInfoConfigMapKey = "kubeconfig" +) + type execerController struct { credentialIssuerLocationConfig *CredentialIssuerLocationConfig + discoveryURLOverride *string dynamicCertProvider dynamiccert.Provider podCommandExecutor PodCommandExecutor clock clock.Clock pinnipedAPIClient pinnipedclientset.Interface agentPodInformer corev1informers.PodInformer + configMapInformer corev1informers.ConfigMapInformer } // NewExecerController returns a controllerlib.Controller that listens for agent pods with proper @@ -36,11 +48,13 @@ type execerController struct { // credentialIssuerLocationConfig, with any errors that it encounters. func NewExecerController( credentialIssuerLocationConfig *CredentialIssuerLocationConfig, + discoveryURLOverride *string, dynamicCertProvider dynamiccert.Provider, podCommandExecutor PodCommandExecutor, pinnipedAPIClient pinnipedclientset.Interface, clock clock.Clock, agentPodInformer corev1informers.PodInformer, + configMapInformer corev1informers.ConfigMapInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( @@ -48,11 +62,13 @@ func NewExecerController( Name: "kube-cert-agent-execer-controller", Syncer: &execerController{ credentialIssuerLocationConfig: credentialIssuerLocationConfig, + discoveryURLOverride: discoveryURLOverride, dynamicCertProvider: dynamicCertProvider, podCommandExecutor: podCommandExecutor, pinnipedAPIClient: pinnipedAPIClient, clock: clock, agentPodInformer: agentPodInformer, + configMapInformer: configMapInformer, }, }, withInformer( @@ -60,6 +76,11 @@ func NewExecerController( pinnipedcontroller.SimpleFilter(isAgentPod, nil), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), + withInformer( + configMapInformer, + pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(clusterInfoName, ClusterInfoNamespace), + controllerlib.InformerOption{}, + ), ) } @@ -114,18 +135,74 @@ func (c *execerController) Sync(ctx controllerlib.Context) error { c.dynamicCertProvider.Set([]byte(certPEM), []byte(keyPEM)) - err = issuerconfig.UpdateStrategy( + apiInfo, err := c.getTokenCredentialRequestAPIInfo() + if err != nil { + strategyResultUpdateErr := issuerconfig.UpdateStrategy( + ctx.Context, + c.credentialIssuerLocationConfig.Name, + nil, + c.pinnipedAPIClient, + configv1alpha1.CredentialIssuerStrategy{ + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.ErrorStrategyStatus, + Reason: configv1alpha1.CouldNotGetClusterInfoStrategyReason, + Message: err.Error(), + LastUpdateTime: metav1.NewTime(c.clock.Now()), + }, + ) + klog.ErrorS(strategyResultUpdateErr, "could not create or update CredentialIssuer with strategy success") + return err + } + + return issuerconfig.UpdateStrategy( ctx.Context, c.credentialIssuerLocationConfig.Name, nil, c.pinnipedAPIClient, - strategySuccess(c.clock), + configv1alpha1.CredentialIssuerStrategy{ + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: metav1.NewTime(c.clock.Now()), + Frontend: &configv1alpha1.CredentialIssuerFrontend{ + Type: configv1alpha1.TokenCredentialRequestAPIFrontendType, + TokenCredentialRequestAPIInfo: apiInfo, + }, + }, ) +} + +func (c *execerController) getTokenCredentialRequestAPIInfo() (*configv1alpha1.TokenCredentialRequestAPIInfo, error) { + configMap, err := c.configMapInformer. + Lister(). + ConfigMaps(ClusterInfoNamespace). + Get(clusterInfoName) if err != nil { - return err + return nil, fmt.Errorf("failed to get %s configmap: %w", clusterInfoName, err) } - return nil + kubeConfigYAML, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey] + if !kubeConfigPresent { + return nil, fmt.Errorf("failed to get %s key from %s configmap", clusterInfoConfigMapKey, clusterInfoName) + } + + kubeconfig, err := clientcmd.Load([]byte(kubeConfigYAML)) + if err != nil { + return nil, fmt.Errorf("failed to load data from %s key in %s configmap", clusterInfoConfigMapKey, clusterInfoName) + } + + for _, v := range kubeconfig.Clusters { + result := &configv1alpha1.TokenCredentialRequestAPIInfo{ + Server: v.Server, + CertificateAuthorityData: base64.StdEncoding.EncodeToString(v.CertificateAuthorityData), + } + if c.discoveryURLOverride != nil { + result.Server = *c.discoveryURLOverride + } + return result, nil + } + return nil, fmt.Errorf("kubeconfig in %s key in %s configmap did not contain any clusters", clusterInfoConfigMapKey, clusterInfoName) } func (c *execerController) getKeypairFilePaths(pod *v1.Pod) (string, string) { diff --git a/internal/controller/kubecertagent/execer_test.go b/internal/controller/kubecertagent/execer_test.go index 774e0c92..485e9c6f 100644 --- a/internal/controller/kubecertagent/execer_test.go +++ b/internal/controller/kubecertagent/execer_test.go @@ -27,6 +27,7 @@ import ( pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/dynamiccert" + "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/testutil" ) @@ -41,16 +42,20 @@ func TestExecerControllerOptions(t *testing.T) { it.Before(func() { r = require.New(t) observableWithInformerOption = testutil.NewObservableWithInformerOption() - agentPodsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Pods() + informerFactory := kubeinformers.NewSharedInformerFactory(nil, 0) + agentPodsInformer := informerFactory.Core().V1().Pods() + configMapsInformer := informerFactory.Core().V1().ConfigMaps() _ = NewExecerController( &CredentialIssuerLocationConfig{ Name: "ignored by this test", }, + nil, // discoveryURLOverride, not needed for this test nil, // dynamicCertProvider, not needed for this test nil, // podCommandExecutor, not needed for this test nil, // pinnipedAPIClient, not needed for this test nil, // clock, not needed for this test agentPodsInformer, + configMapsInformer, observableWithInformerOption.WithInformer, ) agentPodInformerFilter = observableWithInformerOption.GetFilterForInformer(agentPodsInformer) @@ -144,9 +149,10 @@ func TestManagerControllerSync(t *testing.T) { var timeoutContextCancel context.CancelFunc var syncContext *controllerlib.Context var pinnipedAPIClient *pinnipedfake.Clientset - var agentPodInformer kubeinformers.SharedInformerFactory - var agentPodInformerClient *kubernetesfake.Clientset + var kubeInformerFactory kubeinformers.SharedInformerFactory + var kubeClientset *kubernetesfake.Clientset var fakeExecutor *fakePodExecutor + var discoveryURLOverride *string var dynamicCertProvider dynamiccert.Provider var fakeCertPEM, fakeKeyPEM string var credentialIssuerGVR schema.GroupVersionResource @@ -160,11 +166,13 @@ func TestManagerControllerSync(t *testing.T) { &CredentialIssuerLocationConfig{ Name: credentialIssuerResourceName, }, + discoveryURLOverride, dynamicCertProvider, fakeExecutor, pinnipedAPIClient, clock.NewFakeClock(frozenNow), - agentPodInformer.Core().V1().Pods(), + kubeInformerFactory.Core().V1().Pods(), + kubeInformerFactory.Core().V1().ConfigMaps(), controllerlib.WithInformer, ) @@ -179,7 +187,7 @@ func TestManagerControllerSync(t *testing.T) { } // Must start informers before calling TestRunSynchronously() - agentPodInformer.Start(timeoutContext.Done()) + kubeInformerFactory.Start(timeoutContext.Done()) controllerlib.TestRunSynchronously(t, subject) } @@ -219,8 +227,8 @@ func TestManagerControllerSync(t *testing.T) { timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) pinnipedAPIClient = pinnipedfake.NewSimpleClientset() - agentPodInformerClient = kubernetesfake.NewSimpleClientset() - agentPodInformer = kubeinformers.NewSharedInformerFactory(agentPodInformerClient, 0) + kubeClientset = kubernetesfake.NewSimpleClientset() + kubeInformerFactory = kubeinformers.NewSharedInformerFactory(kubeClientset, 0) fakeExecutor = &fakePodExecutor{r: r} frozenNow = time.Date(2020, time.September, 23, 7, 42, 0, 0, time.Local) dynamicCertProvider = dynamiccert.New() @@ -253,7 +261,7 @@ func TestManagerControllerSync(t *testing.T) { Namespace: agentPodNamespace, }, } - r.NoError(agentPodInformerClient.Tracker().Add(unrelatedPod)) + r.NoError(kubeClientset.Tracker().Add(unrelatedPod)) startInformersAndController() }) @@ -266,7 +274,7 @@ func TestManagerControllerSync(t *testing.T) { 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)) + r.NoError(kubeClientset.Tracker().Add(agentPod)) startInformersAndController() }) @@ -280,7 +288,7 @@ func TestManagerControllerSync(t *testing.T) { it.Before(func() { agentPod := newAgentPod(agentPodName, true) agentPod.Status.Phase = corev1.PodPending // not Running - r.NoError(agentPodInformerClient.Tracker().Add(agentPod)) + r.NoError(kubeClientset.Tracker().Add(agentPod)) startInformersAndController() }) @@ -295,8 +303,8 @@ func TestManagerControllerSync(t *testing.T) { 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)) + r.NoError(kubeClientset.Tracker().Add(targetAgentPod)) + r.NoError(kubeClientset.Tracker().Add(anotherAgentPod)) }) when("the resulting pod execs will succeed", func() { @@ -304,90 +312,10 @@ func TestManagerControllerSync(t *testing.T) { 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 CredentialIssuer", func() { - var initialCredentialIssuer *configv1alpha1.CredentialIssuer - - it.Before(func() { - initialCredentialIssuer = &configv1alpha1.CredentialIssuer{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: credentialIssuerResourceName, - }, - Status: configv1alpha1.CredentialIssuerStatus{ - Strategies: []configv1alpha1.CredentialIssuerStrategy{}, - KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: "some-server", - CertificateAuthorityData: "some-ca-value", - }, - }, - } - r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuer)) - }) - - it("also updates the the existing CredentialIssuer status field", func() { + when("the cluster-info ConfigMap is not found", func() { + it("returns an error and updates the strategy with an error", func() { startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - - expectedCredentialIssuer := initialCredentialIssuer.DeepCopy() - expectedCredentialIssuer.Status.Strategies = []configv1alpha1.CredentialIssuerStrategy{ - { - Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, - Status: configv1alpha1.SuccessStrategyStatus, - Reason: configv1alpha1.FetchedKeyStrategyReason, - Message: "Key was fetched successfully", - LastUpdateTime: metav1.NewTime(frozenNow), - }, - } - expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) - expectedCreateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) - r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) - }) - - when("updating the CredentialIssuer fails", func() { - it.Before(func() { - pinnipedAPIClient.PrependReactor( - "update", - "credentialissuers", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("some update error") - }, - ) - }) - - it("returns an error", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.EqualError(err, "could not create or update credentialissuer: some update error") - }) - }) - }) - - when("there is not already a CredentialIssuer", func() { - it.Before(func() { - startInformersAndController() - }) - - it("also creates the the CredentialIssuer with the appropriate status field", func() { - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), `failed to get cluster-info configmap: configmap "cluster-info" not found`) expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ TypeMeta: metav1.TypeMeta{}, @@ -405,9 +333,9 @@ func TestManagerControllerSync(t *testing.T) { Strategies: []configv1alpha1.CredentialIssuerStrategy{ { Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, - Status: configv1alpha1.SuccessStrategyStatus, - Reason: configv1alpha1.FetchedKeyStrategyReason, - Message: "Key was fetched successfully", + Status: configv1alpha1.ErrorStrategyStatus, + Reason: configv1alpha1.CouldNotGetClusterInfoStrategyReason, + Message: `failed to get cluster-info configmap: configmap "cluster-info" not found`, LastUpdateTime: metav1.NewTime(frozenNow), }, }, @@ -419,6 +347,223 @@ func TestManagerControllerSync(t *testing.T) { r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) }) }) + + when("the cluster-info ConfigMap is missing a key", func() { + it.Before(func() { + r.NoError(kubeClientset.Tracker().Add(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ClusterInfoNamespace, + Name: clusterInfoName, + }, + Data: map[string]string{"uninteresting-key": "uninteresting-value"}, + })) + }) + it("returns an error", func() { + startInformersAndController() + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), `failed to get kubeconfig key from cluster-info configmap`) + }) + }) + + when("the cluster-info ConfigMap is contains invalid YAML", func() { + it.Before(func() { + r.NoError(kubeClientset.Tracker().Add(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ClusterInfoNamespace, + Name: clusterInfoName, + }, + Data: map[string]string{"kubeconfig": "invalid-yaml"}, + })) + }) + it("returns an error", func() { + startInformersAndController() + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), `failed to load data from kubeconfig key in cluster-info configmap`) + }) + }) + + when("the cluster-info ConfigMap is contains an empty list of clusters", func() { + it.Before(func() { + r.NoError(kubeClientset.Tracker().Add(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ClusterInfoNamespace, + Name: clusterInfoName, + }, + Data: map[string]string{ + "kubeconfig": here.Doc(` + kind: Config + apiVersion: v1 + clusters: [] + `), + "uninteresting-key": "uninteresting-value", + }, + })) + }) + it("returns an error", func() { + startInformersAndController() + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), `kubeconfig in kubeconfig key in cluster-info configmap did not contain any clusters`) + }) + }) + + when("the cluster-info ConfigMap is valid", func() { + it.Before(func() { + const caData = "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=" // "some-certificate-authority-data" base64 encoded + const kubeServerURL = "https://some-server" + r.NoError(kubeClientset.Tracker().Add(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ClusterInfoNamespace, + Name: clusterInfoName, + }, + Data: map[string]string{ + "kubeconfig": here.Docf(` + kind: Config + apiVersion: v1 + clusters: + - name: "" + cluster: + certificate-authority-data: "%s" + server: "%s"`, + caData, kubeServerURL), + "uninteresting-key": "uninteresting-value", + }, + })) + }) + + 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 CredentialIssuer", func() { + var initialCredentialIssuer *configv1alpha1.CredentialIssuer + + it.Before(func() { + initialCredentialIssuer = &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + Status: configv1alpha1.CredentialIssuerStatus{ + Strategies: []configv1alpha1.CredentialIssuerStrategy{}, + }, + } + r.NoError(pinnipedAPIClient.Tracker().Add(initialCredentialIssuer)) + }) + + it("also updates the the existing CredentialIssuer status field", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + // The first update to the CredentialIssuer will set the strategy entry + expectedCredentialIssuer := initialCredentialIssuer.DeepCopy() + expectedCredentialIssuer.Status.Strategies = []configv1alpha1.CredentialIssuerStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: metav1.NewTime(frozenNow), + Frontend: &configv1alpha1.CredentialIssuerFrontend{ + Type: configv1alpha1.TokenCredentialRequestAPIFrontendType, + TokenCredentialRequestAPIInfo: &configv1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://some-server", + CertificateAuthorityData: "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=", + }, + }, + }, + } + expectedCredentialIssuer.Status.KubeConfigInfo = &configv1alpha1.CredentialIssuerKubeConfigInfo{ + Server: "https://some-server", + CertificateAuthorityData: "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=", + } + expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) + expectedCreateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction}, pinnipedAPIClient.Actions()) + }) + + when("updating the CredentialIssuer fails", func() { + it.Before(func() { + pinnipedAPIClient.PrependReactor( + "update", + "credentialissuers", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some update error") + }, + ) + }) + + it("returns an error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "could not create or update credentialissuer: some update error") + }) + }) + }) + + when("there is not already a CredentialIssuer", func() { + it.Before(func() { + server := "https://overridden-server-url.example.com" + discoveryURLOverride = &server + startInformersAndController() + }) + + it("also creates the the CredentialIssuer with the appropriate status field", func() { + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + expectedCreateCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + } + + expectedCredentialIssuer := &configv1alpha1.CredentialIssuer{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: credentialIssuerResourceName, + }, + Status: configv1alpha1.CredentialIssuerStatus{ + Strategies: []configv1alpha1.CredentialIssuerStrategy{ + { + Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: configv1alpha1.SuccessStrategyStatus, + Reason: configv1alpha1.FetchedKeyStrategyReason, + Message: "Key was fetched successfully", + LastUpdateTime: metav1.NewTime(frozenNow), + Frontend: &configv1alpha1.CredentialIssuerFrontend{ + Type: configv1alpha1.TokenCredentialRequestAPIFrontendType, + TokenCredentialRequestAPIInfo: &configv1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://overridden-server-url.example.com", + CertificateAuthorityData: "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=", + }, + }, + }, + }, + KubeConfigInfo: &configv1alpha1.CredentialIssuerKubeConfigInfo{ + Server: "https://overridden-server-url.example.com", + CertificateAuthorityData: "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=", + }, + }, + } + expectedGetAction := coretesting.NewRootGetAction(credentialIssuerGVR, credentialIssuerResourceName) + expectedCreateAction := coretesting.NewRootCreateAction(credentialIssuerGVR, expectedCreateCredentialIssuer) + expectedUpdateAction := coretesting.NewRootUpdateSubresourceAction(credentialIssuerGVR, "status", expectedCredentialIssuer) + r.Equal([]coretesting.Action{expectedGetAction, expectedCreateAction, expectedUpdateAction}, pinnipedAPIClient.Actions()) + }) + }) + }) }) when("the first resulting pod exec will fail", func() { diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index dff42f63..553765fc 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -277,16 +277,6 @@ func findControllerManagerPodForSpecificAgentPod( return maybeControllerManagerPod, nil } -func strategySuccess(clock clock.Clock) configv1alpha1.CredentialIssuerStrategy { - return configv1alpha1.CredentialIssuerStrategy{ - Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, - Status: configv1alpha1.SuccessStrategyStatus, - Reason: configv1alpha1.FetchedKeyStrategyReason, - Message: "Key was fetched successfully", - LastUpdateTime: metav1.NewTime(clock.Now()), - } -} - func strategyError(clock clock.Clock, err error) configv1alpha1.CredentialIssuerStrategy { return configv1alpha1.CredentialIssuerStrategy{ Type: configv1alpha1.KubeClusterSigningCertificateStrategyType, diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 902ed43b..cd00604b 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -24,7 +24,6 @@ import ( "go.pinniped.dev/internal/controller/authenticator/cachecleaner" "go.pinniped.dev/internal/controller/authenticator/jwtcachefiller" "go.pinniped.dev/internal/controller/authenticator/webhookcachefiller" - "go.pinniped.dev/internal/controller/issuerconfig" "go.pinniped.dev/internal/controller/kubecertagent" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/deploymentref" @@ -124,20 +123,6 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { controllerManager := controllerlib. NewManager(). - // KubeConfig info publishing controller is responsible for writing the KubeConfig information to the - // CredentialIssuer resource and keeping that information up to date. - WithController( - issuerconfig.NewKubeConfigInfoPublisherController( - c.NamesConfig.CredentialIssuer, - c.Labels, - c.DiscoveryURLOverride, - client.PinnipedConcierge, - informers.kubePublicNamespaceK8s.Core().V1().ConfigMaps(), - controllerlib.WithInformer, - ), - singletonWorker, - ). - // API certs controllers are responsible for managing the TLS certificates used to serve Pinniped's API. WithController( apicerts.NewCertsManagerController( @@ -231,11 +216,13 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { WithController( kubecertagent.NewExecerController( credentialIssuerLocationConfig, + c.DiscoveryURLOverride, c.DynamicSigningCertProvider, kubecertagent.NewPodCommandExecutor(client.JSONConfig, client.Kubernetes), client.PinnipedConcierge, clock.RealClock{}, informers.installationNamespaceK8s.Core().V1().Pods(), + informers.kubePublicNamespaceK8s.Core().V1().ConfigMaps(), controllerlib.WithInformer, ), singletonWorker, @@ -303,7 +290,7 @@ func createInformers( kubePublicNamespaceK8s: k8sinformers.NewSharedInformerFactoryWithOptions( k8sClient, defaultResyncInterval, - k8sinformers.WithNamespace(issuerconfig.ClusterInfoNamespace), + k8sinformers.WithNamespace(kubecertagent.ClusterInfoNamespace), ), kubeSystemNamespaceK8s: k8sinformers.NewSharedInformerFactoryWithOptions( k8sClient, diff --git a/test/integration/concierge_credentialissuerconfig_test.go b/test/integration/concierge_credentialissuer_test.go similarity index 86% rename from test/integration/concierge_credentialissuerconfig_test.go rename to test/integration/concierge_credentialissuer_test.go index 45ce038d..c7ed6e66 100644 --- a/test/integration/concierge_credentialissuerconfig_test.go +++ b/test/integration/concierge_credentialissuer_test.go @@ -72,12 +72,20 @@ func TestCredentialIssuer(t *testing.T) { require.Equal(t, configv1alpha1.SuccessStrategyStatus, actualStatusStrategy.Status) require.Equal(t, configv1alpha1.FetchedKeyStrategyReason, actualStatusStrategy.Reason) require.Equal(t, "Key was fetched successfully", actualStatusStrategy.Message) + require.NotNil(t, actualStatusStrategy.Frontend) + require.Equal(t, configv1alpha1.TokenCredentialRequestAPIFrontendType, actualStatusStrategy.Frontend.Type) + expectedTokenRequestAPIInfo := configv1alpha1.TokenCredentialRequestAPIInfo{ + Server: config.Host, + CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), + } + require.Equal(t, &expectedTokenRequestAPIInfo, actualStatusStrategy.Frontend.TokenCredentialRequestAPIInfo) + // Verify the published kube config info. require.Equal( t, &configv1alpha1.CredentialIssuerKubeConfigInfo{ - Server: config.Host, - CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), + Server: expectedTokenRequestAPIInfo.Server, + CertificateAuthorityData: expectedTokenRequestAPIInfo.CertificateAuthorityData, }, actualStatusKubeConfigInfo, )