More WIP for the publisher controller

deploy/rbac.yaml

@@ -120,7 +120,7 @@ metadata:
 rules:
   - apiGroups: [""]
     resources: [configmaps]
-    verbs: [list, watch] #! TODO: do we neeed a get here for the controller?
+    verbs: [list, watch]
 ---
 kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1

internal/apiserver/apiserver.go

@@ -126,7 +126,7 @@ func (c completedConfig) New() (*PlaceHolderServer, error) {
 
 	s.GenericAPIServer.AddPostStartHookOrDie("start-controllers",
 		func(postStartContext genericapiserver.PostStartHookContext) error {
-			klog.InfoS("post start hook")
+			klog.InfoS("start-controllers post start hook starting")
 
 			ctx, cancel := context.WithCancel(context.Background())
 			go func() {

internal/server/server.go

@@ -43,10 +43,9 @@ import (
 	"github.com/suzerain-io/placeholder-name/pkg/config"
 )
 
-// TODO(akeesler): what should these controller settings be?
 const (
-	defaultWorkers = 3
+	singletonWorker       = 1
-	defaultResync  = 20 * time.Minute
+	defaultResyncInterval = 3 * time.Minute
 )
 
 // App is an object that represents the placeholder-name-server application.
@@ -93,13 +92,13 @@ authenticating to the Kubernetes API.`,
 			protoKubeConfig := createProtoKubeConfig(kubeConfig)
 
 			// Connect to the core Kubernetes API.
-			k8s, err := kubernetes.NewForConfig(protoKubeConfig)
+			k8sClient, err := kubernetes.NewForConfig(protoKubeConfig)
 			if err != nil {
 				return fmt.Errorf("could not initialize Kubernetes client: %w", err)
 			}
 
 			// Connect to the Kubernetes aggregation API.
-			aggregation, err := aggregationv1client.NewForConfig(protoKubeConfig)
+			aggregationClient, err := aggregationv1client.NewForConfig(protoKubeConfig)
 			if err != nil {
 				return fmt.Errorf("could not initialize Kubernetes client: %w", err)
 			}
@@ -107,12 +106,12 @@ authenticating to the Kubernetes API.`,
 			// Connect to the placeholder API.
 			// I think we can't use protobuf encoding here because we are using CRDs
 			// (for which protobuf encoding is not supported).
-			placeholder, err := placeholderclientset.NewForConfig(kubeConfig)
+			placeholderClient, err := placeholderclientset.NewForConfig(kubeConfig)
 			if err != nil {
 				return fmt.Errorf("could not initialize placeholder client: %w", err)
 			}
 
-			return a.run(ctx, k8s, aggregation, placeholder)
+			return a.run(ctx, k8sClient, aggregationClient, placeholderClient)
 		},
 		Args: cobra.NoArgs,
 	}
@@ -161,9 +160,9 @@ func (a *App) Run() error {
 
 func (a *App) run(
 	ctx context.Context,
-	k8s kubernetes.Interface,
+	k8sClient kubernetes.Interface,
-	aggregation aggregationv1client.Interface,
+	aggregationClient aggregationv1client.Interface,
-	placeholder placeholderclientset.Interface,
+	placeholderClient placeholderclientset.Interface,
 ) error {
 	cfg, err := config.FromPath(a.configPath)
 	if err != nil {
@@ -185,6 +184,7 @@ func (a *App) run(
 	if err != nil {
 		return fmt.Errorf("could not read pod metadata: %w", err)
 	}
+	serverInstallationNamespace := podinfo.Namespace
 
 	// TODO use the postStart hook to generate certs?
 
@@ -196,7 +196,7 @@ func (a *App) run(
 	const serviceName = "placeholder-name-api"
 
 	cert, err := apiCA.Issue(
-		pkix.Name{CommonName: serviceName + "." + podinfo.Namespace + ".svc"},
+		pkix.Name{CommonName: serviceName + "." + serverInstallationNamespace + ".svc"},
 		[]string{},
 		24*365*time.Hour,
 	)
@@ -232,16 +232,16 @@ func (a *App) run(
 		},
 	}
 	if err := autoregistration.Setup(ctx, autoregistration.SetupOptions{
-		CoreV1:             k8s.CoreV1(),
+		CoreV1:             k8sClient.CoreV1(),
-		AggregationV1:      aggregation,
+		AggregationV1:      aggregationClient,
-		Namespace:          podinfo.Namespace,
+		Namespace:          serverInstallationNamespace,
 		ServiceTemplate:    service,
 		APIServiceTemplate: apiService,
 	}); err != nil {
 		return fmt.Errorf("could not register API service: %w", err)
 	}
 
-	cmrf := wireControllerManagerRunFunc(podinfo, k8s, placeholder)
+	cmrf := wireControllerManagerRunFunc(serverInstallationNamespace, k8sClient, placeholderClient)
 	apiServerConfig, err := a.configServer(
 		cert,
 		webhookTokenAuthenticator,
@@ -323,35 +323,33 @@ func createStaticCertKeyProvider(cert *tls.Certificate) (dynamiccertificates.Cer
 }
 
 func wireControllerManagerRunFunc(
-	podinfo *downward.PodInfo,
+	serverInstallationNamespace string,
 	k8s kubernetes.Interface,
 	placeholder placeholderclientset.Interface,
 ) func(ctx context.Context) {
 	k8sInformers := k8sinformers.NewSharedInformerFactoryWithOptions(
 		k8s,
-		defaultResync,
+		defaultResyncInterval,
 		k8sinformers.WithNamespace(
 			logindiscovery.ClusterInfoNamespace,
 		),
 	)
 	placeholderInformers := placeholderinformers.NewSharedInformerFactoryWithOptions(
 		placeholder,
-		defaultResync,
+		defaultResyncInterval,
-		placeholderinformers.WithNamespace(
+		placeholderinformers.WithNamespace(serverInstallationNamespace),
-			"integration", // TODO(akeesler): unhardcode this.
-		),
 	)
 	cm := controller.
 		NewManager().
 		WithController(
 			logindiscovery.NewPublisherController(
-				podinfo.Namespace,
+				serverInstallationNamespace,
 				placeholder,
 				k8sInformers.Core().V1().ConfigMaps(),
 				placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(),
 				controller.WithInformer,
 			),
-			defaultWorkers,
+			singletonWorker,
 		)
 	return func(ctx context.Context) {
 		k8sInformers.Start(ctx.Done())

test/integration/logindiscoveryconfig_test.go

@@ -14,8 +14,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/rest"
 
 	placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1"
@@ -31,23 +29,18 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	// TODO(akeesler): is there a race here between this test running and the
-	// placeholder-name-server creating the CR?
-
 	config := library.NewClientConfig(t)
-	expectedLDC := getExpectedLDC(namespaceName, config)
+	expectedLDCSpec := expectedLDCSpec(config)
 	configList, err := client.
 		PlaceholderV1alpha1().
 		LoginDiscoveryConfigs(namespaceName).
 		List(ctx, metav1.ListOptions{})
 	require.NoError(t, err)
 	require.Len(t, configList.Items, 1)
-	require.Equal(t, expectedLDC, configList.Items[0])
+	require.Equal(t, expectedLDCSpec, &configList.Items[0].Spec)
 }
 
 func TestReconcilingLoginDiscoveryConfig(t *testing.T) {
-	t.Skip()
-
 	namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE")
 	require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests")
 
@@ -56,61 +49,32 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	// TODO(akeesler): is there a race here between this test running and the
+	err := client.
-	// placeholder-name-server creating the CR?
-
-	w, err := client.
-		PlaceholderV1alpha1().
-		LoginDiscoveryConfigs(namespaceName).
-		Watch(ctx, metav1.ListOptions{})
-	require.NoError(t, err)
-
-	err = client.
 		PlaceholderV1alpha1().
 		LoginDiscoveryConfigs(namespaceName).
 		Delete(ctx, "placeholder-name-config", metav1.DeleteOptions{})
 	require.NoError(t, err)
 
 	config := library.NewClientConfig(t)
-	expectedLDC := getExpectedLDC(namespaceName, config)
+	expectedLDCSpec := expectedLDCSpec(config)
-	received := func(et watch.EventType, o runtime.Object) func() bool {
+
-		return func() bool {
+	var actualLDC *placeholderv1alpha1.LoginDiscoveryConfig
-			select {
+	for i := 0; i < 10; i++ {
-			case e := <-w.ResultChan():
+		actualLDC, err = client.PlaceholderV1alpha1().
-				require.Equal(t, et, e.Type)
+			LoginDiscoveryConfigs(namespaceName).
-				require.Equal(t, o, e.Object)
+			Get(ctx, "placeholder-name-config", metav1.GetOptions{})
-				return true
+		if err == nil {
-			default:
+			break
-				return false
-			}
 		}
+		time.Sleep(time.Millisecond * 750)
 	}
-	require.Eventually(
+	require.NoError(t, err)
-		t,
+	require.Equal(t, expectedLDCSpec, &actualLDC.Spec)
-		received(watch.Deleted, expectedLDC),
-		time.Second,
-		3*time.Second,
-	)
-	require.Eventually(
-		t,
-		received(watch.Added, expectedLDC),
-		time.Second,
-		3*time.Second,
-	)
 }
 
-func getExpectedLDC(
+func expectedLDCSpec(config *rest.Config) *placeholderv1alpha1.LoginDiscoveryConfigSpec {
-	namespaceName string,
+	return &placeholderv1alpha1.LoginDiscoveryConfigSpec{
-	config *rest.Config,
+		Server:                   "https://kind-control-plane:6443", //config.Host, // TODO FIX THIS
-) *placeholderv1alpha1.LoginDiscoveryConfig {
+		CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData),
-	return &placeholderv1alpha1.LoginDiscoveryConfig{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "placeholder-name-config",
-			Namespace: namespaceName,
-		},
-		Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{
-			Server:                   config.Host,
-			CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData),
-		},
 	}
 }

test/integration/loginrequest_test.go

@@ -181,32 +181,41 @@ func TestGetAPIResourceList(t *testing.T) {
 	actualResources := findResources(resourceGroupVersion, resources)
 	require.NotNil(t, actualResources)
 
-	expectedResources := &metav1.APIResourceList{
+	expectedLoginRequestAPIResource := metav1.APIResource{
-		TypeMeta: metav1.TypeMeta{
+		Name:         "loginrequests",
-			Kind:       "APIResourceList",
+		Kind:         "LoginRequest",
-			APIVersion: "v1",
+		SingularName: "", // TODO(akeesler): what should this be?
-		},
+		Verbs: metav1.Verbs([]string{
-		GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1",
+			"create",
-		APIResources: []metav1.APIResource{
+		}),
-			{
+		Namespaced: false,
-				Name:         "loginrequests",
-				Kind:         "LoginRequest",
-				SingularName: "", // TODO(akeesler): what should this be?
-				Verbs: metav1.Verbs([]string{
-					"create",
-				}),
-			},
-		},
 	}
-	require.Equal(t, expectedResources, actualResources)
-}
 
-func TestGetAPIVersion(t *testing.T) {
+	expectedLDCAPIResource := metav1.APIResource{
-	client := library.NewPlaceholderNameClientset(t)
+		Name:         "logindiscoveryconfigs",
+		SingularName: "logindiscoveryconfig",
+		Namespaced:   true,
+		Kind:         "LoginDiscoveryConfig",
+		Verbs: metav1.Verbs([]string{
+			"delete", "deletecollection", "get", "list", "patch", "create", "update", "watch",
+		}),
+		ShortNames:         []string{"ldc"},
+		StorageVersionHash: "unknown: to be filled in automatically below",
+	}
 
-	version, err := client.Discovery().ServerVersion()
+	expectedResourcesMap := map[string]metav1.APIResource{
-	require.NoError(t, err)
+		expectedLoginRequestAPIResource.Name: expectedLoginRequestAPIResource,
-	require.NotNil(t, version) // TODO(akeesler): what can we assert here?
+		expectedLDCAPIResource.Name:          expectedLDCAPIResource,
+	}
+
+	require.Len(t, actualResources.APIResources, 2)
+	for _, actualAPIResource := range actualResources.APIResources {
+		if actualAPIResource.Name == expectedLDCAPIResource.Name {
+			// hard to predict the storage version hash (e.g. "t/+v41y+3e4=") so just don't worry about comparing them
+			expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash
+		}
+		require.Equal(t, expectedResourcesMap[actualAPIResource.Name], actualAPIResource)
+	}
 }
 
 func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup {