More tests for the PublisherController

- Also, don't repeat `spec.Parallel()` because, according to the docs
  for the spec package, "options are inherited by subgroups and subspecs"
- Two tests are left pending to be filled in on the next commit

cmd/placeholder-name/main_test.go

@@ -64,7 +64,7 @@ func TestRun(t *testing.T) {
 				err := run(envGetter, tokenExchanger, buffer, 30*time.Second)
 				r.EqualError(err, "failed to login: environment variable not set: PLACEHOLDER_NAME_K8S_API_ENDPOINT")
 			})
-		}, spec.Parallel())
+		})
 
 		when("the token exchange fails", func() {
 			it.Before(func() {
@@ -77,7 +77,7 @@ func TestRun(t *testing.T) {
 				err := run(envGetter, tokenExchanger, buffer, 30*time.Second)
 				r.EqualError(err, "failed to login: some error")
 			})
-		}, spec.Parallel())
+		})
 
 		when("the JSON encoder fails", func() {
 			it.Before(func() {
@@ -92,7 +92,7 @@ func TestRun(t *testing.T) {
 				err := run(envGetter, tokenExchanger, &library.ErrorWriter{ReturnError: fmt.Errorf("some IO error")}, 30*time.Second)
 				r.EqualError(err, "failed to marshal response to stdout: some IO error")
 			})
-		}, spec.Parallel())
+		})
 
 		when("the token exchange times out", func() {
 			it.Before(func() {
@@ -112,7 +112,7 @@ func TestRun(t *testing.T) {
 				err := run(envGetter, tokenExchanger, buffer, 1*time.Millisecond)
 				r.EqualError(err, "failed to login: context deadline exceeded")
 			})
-		}, spec.Parallel())
+		})
 
 		when("the token exchange succeeds", func() {
 			var actualToken, actualCaBundle, actualAPIEndpoint string
@@ -146,6 +146,6 @@ func TestRun(t *testing.T) {
 				}`
 				r.JSONEq(expected, buffer.String())
 			})
-		}, spec.Parallel())
+		})
-	}, spec.Report(report.Terminal{}))
+	}, spec.Parallel(), spec.Report(report.Terminal{}))
 }

internal/controller/logindiscovery/publisher.go

@@ -45,8 +45,14 @@ func NewPublisherController(namespace string, kubeClient kubernetes.Interface, p
 }
 
 func (c *publisherController) Sync(ctx controller.Context) error {
-	configMap, _ := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{})
+	configMap, err := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{})
-	kubeConfig := configMap.Data[clusterInfoConfigMapKey] // TODO also handle when the key is not found
+	if err != nil {
+		return nil // TODO should this return an error? and should it log?
+	}
+	kubeConfig, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey]
+	if !kubeConfigPresent {
+		return nil // TODO should this return an error? and should it log?
+	}
 
 	config, _ := clientcmd.Load([]byte(kubeConfig))
 

internal/controller/logindiscovery/publisher_test.go

@@ -25,12 +25,6 @@ import (
 	placeholderfake "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned/fake"
 )
 
-// TODO: add test for ConfigMap does NOT exist
-// TODO: add test for when LoginDiscoveryConfig already exists
-//  There should be 4 tests of the above cross product
-
-// TODO test when the expected key in the configmap does not exist
-
 func TestRun(t *testing.T) {
 	spec.Run(t, "publisher", func(t *testing.T, when spec.G, it spec.S) {
 		const installedInNamespace = "some-namespace"
@@ -79,19 +73,18 @@ func TestRun(t *testing.T) {
 			}
 		})
 
-		when("when there is a cluster-info ConfigMap in the kube-public namespace", func() {
+		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"
+			var clusterInfoConfigMap *corev1.ConfigMap
 
-			it.Before(func() {
+			when("the ConfigMap has the expected `kubeconfig` top-level data key", func() {
-				clusterInfo := &corev1.ConfigMap{
+				it.Before(func() {
-					ObjectMeta: metav1.ObjectMeta{
+					clusterInfoConfigMap = &corev1.ConfigMap{
-						Name:      "cluster-info",
+						ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"},
-						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{
-					// Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below.
+							"kubeconfig": strings.ReplaceAll(`
-					Data: map[string]string{
-						"kubeconfig": strings.ReplaceAll(`
 							kind: Config
 							apiVersion: v1
 							clusters:
@@ -99,31 +92,94 @@ func TestRun(t *testing.T) {
 							  cluster:
 							    certificate-authority-data: "`+caData+`"
 							    server: "`+kubeServerURL+`"`, "\t", "  "),
-						"uninteresting-key": "uninteresting-value",
+							"uninteresting-key": "uninteresting-value",
+						},
+					}
+					err := kubeClient.Tracker().Add(clusterInfoConfigMap)
+					r.NoError(err)
+				})
+
+				when("the LoginDiscoveryConfig does not already exist", func() {
+					it("creates a LoginDiscoveryConfig", func() {
+						defer timeoutContextCancel()
+
+						err := controller.TestSync(t, subject, *controllerContext)
+						r.NoError(err)
+
+						expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData)
+						expectedActions := []coretesting.Action{
+							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("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
+						})
+
+						it.Pend("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes", func() {
+							err := controller.TestSync(t, subject, *controllerContext)
+							r.NoError(err)
+							r.Empty(placeholderClient.Actions())
+						})
+					})
+
+					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
+						})
+
+						it.Pend("updates the existing LoginDiscoveryConfig", func() {
+							// TODO
+						})
+					})
+				})
+			})
+
+			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 := kubeClient.Tracker().Add(clusterInfoConfigMap)
+					r.NoError(err)
+				})
+
+				it("keeps waiting for it to exist", func() {
+					err := controller.TestSync(t, subject, *controllerContext)
+					r.NoError(err)
+					r.Empty(placeholderClient.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 := kubeClient.Tracker().Add(clusterInfo)
+				err := kubeClient.Tracker().Add(unrelatedConfigMap)
 				r.NoError(err)
 			})
 
-			when("when the LoginDiscoveryConfig does not already exist", func() {
+			it("keeps waiting for one", func() {
-				it("creates a LoginDiscoveryConfig", func() {
+				err := controller.TestSync(t, subject, *controllerContext)
-					defer timeoutContextCancel()
+				r.NoError(err)
-
+				r.Empty(placeholderClient.Actions())
-					err := controller.TestSync(t, subject, *controllerContext) // Call the controller's sync method once
+			})
-					r.NoError(err)
+		})
-
+	}, spec.Parallel(), spec.Report(report.Terminal{}))
-					expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData)
-					expectedActions := []coretesting.Action{
-						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())
-				})
-			}, spec.Parallel())
-		}, spec.Parallel())
-	}, spec.Report(report.Terminal{}))
 }