diff --git a/go.mod b/go.mod index fd0a02e9..6786bf68 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 - github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 + github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c k8s.io/api v0.19.0-rc.0 k8s.io/apimachinery v0.19.0-rc.0 diff --git a/go.sum b/go.sum index 1d7c7d9b..3a317f7a 100644 --- a/go.sum +++ b/go.sum @@ -534,6 +534,8 @@ github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 h1:2NQMY github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 h1:xnco3XJMrvlwyQJfKoyVPciATvCJ3Y6SY2D8gI2DT2E= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b h1:7Fuizf0c3ffqyHj7X4AvXnYNFJHbSgHKjuDxDsxeQ8A= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c h1:UQVAfjF71g+v2L0ZT8MedKxrdIpx4KrHqvefnmW1AoU= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c/go.mod h1:J/D+4tWlKpLpTd8Jotb7FnLl1OeT2KrUOP3Wc3ooqis= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2 h1:Xr9gkxfOP0KQWXKNqmwe8vEeSUiUj4Rlee9CMVX2ZUQ= diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 2795dda5..adbeb3cc 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -6,11 +6,15 @@ SPDX-License-Identifier: Apache-2.0 package logindiscovery import ( + "context" "encoding/base64" + "fmt" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog/v2" "github.com/suzerain-io/controller-go" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" @@ -45,13 +49,27 @@ func NewPublisherController(namespace string, kubeClient kubernetes.Interface, p } func (c *publisherController) Sync(ctx controller.Context) error { - configMap, err := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) - if err != nil { - return nil // TODO should this return an error? and should it log? + configMap, err := c.kubeClient. + CoreV1(). + ConfigMaps(clusterInfoNamespace). + Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("failed to get %s configmap: %w", clusterInfoName, err) } + if notFound { + klog.InfoS( + "could not find config map", + "configmap", + klog.KRef(clusterInfoNamespace, clusterInfoName), + ) + return nil + } + kubeConfig, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey] if !kubeConfigPresent { - return nil // TODO should this return an error? and should it log? + klog.InfoS("could not find kubeconfig configmap key") + return nil } config, _ := clientcmd.Load([]byte(kubeConfig)) @@ -74,10 +92,57 @@ func (c *publisherController) Sync(ctx controller.Context) error { CertificateAuthorityData: certificateAuthorityData, }, } - _, _ = c.placeholderClient. - PlaceholderV1alpha1(). - LoginDiscoveryConfigs(c.namespace). - Create(ctx.Context, &discoveryConfig, metav1.CreateOptions{}) + if err := c.createOrUpdateLoginDiscoveryConfig(ctx.Context, &discoveryConfig); err != nil { + return err + } return nil } + +func (c *publisherController) createOrUpdateLoginDiscoveryConfig( + ctx context.Context, + discoveryConfig *placeholderv1alpha1.LoginDiscoveryConfig, +) error { + loginDiscoveryConfigs := c.placeholderClient. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(c.namespace) + + existingDiscoveryConfig, err := loginDiscoveryConfigs.Get( + ctx, + discoveryConfig.Name, + metav1.GetOptions{}, + ) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("could not get logindiscoveryconfig: %w", err) + } + + if notFound { + if _, err := loginDiscoveryConfigs.Create( + ctx, + discoveryConfig, + metav1.CreateOptions{}, + ); err != nil { + return fmt.Errorf("could not create logindiscoveryconfig: %w", err) + } + } else if !equal(existingDiscoveryConfig, discoveryConfig) { + // Update just the fields we care about. + existingDiscoveryConfig.Spec.Server = discoveryConfig.Spec.Server + existingDiscoveryConfig.Spec.CertificateAuthorityData = discoveryConfig.Spec.CertificateAuthorityData + + if _, err := loginDiscoveryConfigs.Update( + ctx, + existingDiscoveryConfig, + metav1.UpdateOptions{}, + ); err != nil { + return fmt.Errorf("could not update logindiscoveryconfig: %w", err) + } + } + + return nil +} + +func equal(a, b *placeholderv1alpha1.LoginDiscoveryConfig) bool { + return a.Spec.Server == b.Spec.Server && + a.Spec.CertificateAuthorityData == b.Spec.CertificateAuthorityData +} diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index 72dffbd7..eff0dacb 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -7,6 +7,7 @@ package logindiscovery import ( "context" + "errors" "strings" "testing" "time" @@ -16,6 +17,7 @@ import ( "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" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" @@ -106,38 +108,144 @@ func TestRun(t *testing.T) { err := controller.TestSync(t, subject, *controllerContext) r.NoError(err) - expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData) + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) expectedActions := []coretesting.Action{ - coretesting.NewCreateAction(expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig), + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + coretesting.NewCreateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), } - - // Expect a LoginDiscoveryConfig to be created with the fields from the cluster-info ConfigMap - actualCreatedObject := placeholderClient.Actions()[0].(coretesting.CreateActionImpl).Object - r.Equal(expectedLoginDiscoveryConfig, actualCreatedObject) r.Equal(expectedActions, placeholderClient.Actions()) }) + + when("creating the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "create", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("create failed") + }, + ) + }) + + it("returns the create error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not create logindiscoveryconfig: create failed") + }) + }) }) when("the LoginDiscoveryConfig already exists", func() { when("the LoginDiscoveryConfig is already up to date according to the data in the ConfigMap", func() { it.Before(func() { - // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data matches the ConfigMap's data + _, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) + r.NoError(err) }) - it.Pend("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes", func() { + it("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes/api calls", func() { err := controller.TestSync(t, subject, *controllerContext) r.NoError(err) - r.Empty(placeholderClient.Actions()) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + } + r.Equal(expectedActions, placeholderClient.Actions()) + }) + + when("getting the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "get", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("get failed") + }, + ) + }) + + it("returns the get error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not get logindiscoveryconfig: get failed") + }) }) }) when("the LoginDiscoveryConfig is stale compared to the data in the ConfigMap", func() { it.Before(func() { - // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data does not match the ConfigMap's data + _, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedLoginDiscoveryConfig.Spec.Server = "https://some-other-server" + err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) + r.NoError(err) }) - it.Pend("updates the existing LoginDiscoveryConfig", func() { - // TODO + it("updates the existing LoginDiscoveryConfig", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + coretesting.NewUpdateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), + } + r.Equal(expectedActions, placeholderClient.Actions()) + }) + + when("updating the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "update", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("update failed") + }, + ) + }) + + it("returns the update error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not update logindiscoveryconfig: update failed") + }) }) }) }) @@ -181,5 +289,22 @@ func TestRun(t *testing.T) { r.Empty(placeholderClient.Actions()) }) }) + + when("getting the cluster-info ConfigMap in the kube-public namespace fails", func() { + it.Before(func() { + kubeClient.PrependReactor( + "get", + "configmaps", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("get failed") + }, + ) + }) + + it("returns an error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "failed to get cluster-info configmap: get failed") + }) + }) }, spec.Parallel(), spec.Report(report.Terminal{})) }