Supervisor controllers apply custom labels to JWKS secrets

Signed-off-by: Ryan Richard <richardry@vmware.com>

cmd/pinniped-supervisor/main.go

@@ -23,6 +23,7 @@ import (
 
 	pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned"
 	pinnipedinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions"
+	"go.pinniped.dev/internal/config/supervisor"
 	"go.pinniped.dev/internal/controller/supervisorconfig"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/downward"
@@ -63,6 +64,7 @@ func waitForSignal() os.Signal {
 
 func startControllers(
 	ctx context.Context,
+	cfg *supervisor.Config,
 	issuerProvider *manager.Manager,
 	kubeClient kubernetes.Interface,
 	pinnipedClient pinnipedclientset.Interface,
@@ -84,6 +86,7 @@ func startControllers(
 		).
 		WithController(
 			supervisorconfig.NewJWKSController(
+				cfg.Labels,
 				kubeClient,
 				pinnipedClient,
 				kubeInformers.Core().V1().Secrets(),
@@ -120,7 +123,7 @@ func newClients() (kubernetes.Interface, pinnipedclientset.Interface, error) {
 	return kubeClient, pinnipedClient, nil
 }
 
-func run(serverInstallationNamespace string) error {
+func run(serverInstallationNamespace string, cfg *supervisor.Config) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
@@ -142,7 +145,7 @@ func run(serverInstallationNamespace string) error {
 	)
 
 	oidProvidersManager := manager.NewManager(http.NotFoundHandler())
-	startControllers(ctx, oidProvidersManager, kubeClient, pinnipedClient, kubeInformers, pinnipedInformers)
+	startControllers(ctx, cfg, oidProvidersManager, kubeClient, pinnipedClient, kubeInformers, pinnipedInformers)
 
 	//nolint: gosec // Intentionally binding to all network interfaces.
 	l, err := net.Listen("tcp", ":80")
@@ -173,7 +176,13 @@ func main() {
 		klog.Fatal(fmt.Errorf("could not read pod metadata: %w", err))
 	}
 
-	if err := run(podInfo.Namespace); err != nil {
+	// Read the server config file.
+	cfg, err := supervisor.FromPath(os.Args[2])
+	if err != nil {
+		klog.Fatal(fmt.Errorf("could not load config: %w", err))
+	}
+
+	if err := run(podInfo.Namespace, cfg); err != nil {
 		klog.Fatal(err)
 	}
 }

deploy/supervisor/deployment.yaml

@@ -30,8 +30,6 @@ metadata:
 data:
   #@yaml/text-templated-strings
   pinniped.yaml: |
-    names:
-      dynamicConfigMap: (@= defaultResourceNameWithSuffix("dynamic-config") @)
     labels: (@= json.encode(labels()).rstrip() @)
 ---
 #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "":

internal/concierge/server/server.go

@@ -17,13 +17,13 @@ import (
 	loginv1alpha1 "go.pinniped.dev/generated/1.19/apis/login/v1alpha1"
 	"go.pinniped.dev/internal/certauthority/dynamiccertauthority"
 	"go.pinniped.dev/internal/concierge/apiserver"
+	"go.pinniped.dev/internal/config/concierge"
 	"go.pinniped.dev/internal/controller/identityprovider/idpcache"
 	"go.pinniped.dev/internal/controllermanager"
 	"go.pinniped.dev/internal/downward"
 	"go.pinniped.dev/internal/dynamiccert"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/registry/credentialrequest"
-	"go.pinniped.dev/pkg/config"
 )
 
 // App is an object that represents the pinniped-concierge application.
@@ -92,7 +92,7 @@ func addCommandlineFlagsToCommand(cmd *cobra.Command, app *App) {
 // Boot the aggregated API server, which will in turn boot the controllers.
 func (a *App) runServer(ctx context.Context) error {
 	// Read the server config file.
-	cfg, err := config.FromPath(a.configPath)
+	cfg, err := concierge.FromPath(a.configPath)
 	if err != nil {
 		return fmt.Errorf("could not load config: %w", err)
 	}

internal/config/concierge/config.go

@@ -1,9 +1,9 @@
 // Copyright 2020 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-// Package config contains functionality to load/store api.Config's from/to
+// Package concierge contains functionality to load/store Config's from/to
 // some source.
-package config
+package concierge
 
 import (
 	"fmt"
@@ -13,7 +13,6 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"go.pinniped.dev/internal/constable"
-	"go.pinniped.dev/pkg/config/api"
 )
 
 const (
@@ -21,20 +20,20 @@ const (
 	about9Months = 60 * 60 * 24 * 30 * 9
 )
 
-// FromPath loads an api.Config from a provided local file path, inserts any
+// FromPath loads an Config from a provided local file path, inserts any
-// defaults (from the api.Config documentation), and verifies that the config is
+// defaults (from the Config documentation), and verifies that the config is
-// valid (per the api.Config documentation).
+// valid (per the Config documentation).
 //
-// Note! The api.Config file should contain base64-encoded WebhookCABundle data.
+// Note! The Config file should contain base64-encoded WebhookCABundle data.
 // This function will decode that base64-encoded data to PEM bytes to be stored
-// in the api.Config.
+// in the Config.
-func FromPath(path string) (*api.Config, error) {
+func FromPath(path string) (*Config, error) {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("read file: %w", err)
 	}
 
-	var config api.Config
+	var config Config
 	if err := yaml.Unmarshal(data, &config); err != nil {
 		return nil, fmt.Errorf("decode yaml: %w", err)
 	}
@@ -57,7 +56,7 @@ func FromPath(path string) (*api.Config, error) {
 	return &config, nil
 }
 
-func maybeSetAPIDefaults(apiConfig *api.APIConfigSpec) {
+func maybeSetAPIDefaults(apiConfig *APIConfigSpec) {
 	if apiConfig.ServingCertificateConfig.DurationSeconds == nil {
 		apiConfig.ServingCertificateConfig.DurationSeconds = int64Ptr(aboutAYear)
 	}
@@ -67,7 +66,7 @@ func maybeSetAPIDefaults(apiConfig *api.APIConfigSpec) {
 	}
 }
 
-func maybeSetKubeCertAgentDefaults(cfg *api.KubeCertAgentSpec) {
+func maybeSetKubeCertAgentDefaults(cfg *KubeCertAgentSpec) {
 	if cfg.NamePrefix == nil {
 		cfg.NamePrefix = stringPtr("pinniped-kube-cert-agent-")
 	}
@@ -77,7 +76,7 @@ func maybeSetKubeCertAgentDefaults(cfg *api.KubeCertAgentSpec) {
 	}
 }
 
-func validateNames(names *api.NamesConfigSpec) error {
+func validateNames(names *NamesConfigSpec) error {
 	missingNames := []string{}
 	if names == nil {
 		missingNames = append(missingNames, "servingCertificateSecret", "credentialIssuerConfig", "apiService")
@@ -98,7 +97,7 @@ func validateNames(names *api.NamesConfigSpec) error {
 	return nil
 }
 
-func validateAPI(apiConfig *api.APIConfigSpec) error {
+func validateAPI(apiConfig *APIConfigSpec) error {
 	if *apiConfig.ServingCertificateConfig.DurationSeconds < *apiConfig.ServingCertificateConfig.RenewBeforeSeconds {
 		return constable.Error("durationSeconds cannot be smaller than renewBeforeSeconds")
 	}

internal/config/concierge/config_test.go

@@ -1,7 +1,7 @@
 // Copyright 2020 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package config
+package concierge
 
 import (
 	"io/ioutil"
@@ -11,14 +11,13 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.pinniped.dev/internal/here"
-	"go.pinniped.dev/pkg/config/api"
 )
 
 func TestFromPath(t *testing.T) {
 	tests := []struct {
 		name       string
 		yaml       string
-		wantConfig *api.Config
+		wantConfig *Config
 		wantError  string
 	}{
 		{
@@ -44,17 +43,17 @@ func TestFromPath(t *testing.T) {
 				  image: kube-cert-agent-image
 				  imagePullSecrets: [kube-cert-agent-image-pull-secret]
 			`),
-			wantConfig: &api.Config{
+			wantConfig: &Config{
-				DiscoveryInfo: api.DiscoveryInfoSpec{
+				DiscoveryInfo: DiscoveryInfoSpec{
 					URL: stringPtr("https://some.discovery/url"),
 				},
-				APIConfig: api.APIConfigSpec{
+				APIConfig: APIConfigSpec{
-					ServingCertificateConfig: api.ServingCertificateConfigSpec{
+					ServingCertificateConfig: ServingCertificateConfigSpec{
 						DurationSeconds:    int64Ptr(3600),
 						RenewBeforeSeconds: int64Ptr(2400),
 					},
 				},
-				NamesConfig: api.NamesConfigSpec{
+				NamesConfig: NamesConfigSpec{
 					ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate",
 					CredentialIssuerConfig:   "pinniped-config",
 					APIService:               "pinniped-api",
@@ -63,7 +62,7 @@ func TestFromPath(t *testing.T) {
 					"myLabelKey1": "myLabelValue1",
 					"myLabelKey2": "myLabelValue2",
 				},
-				KubeCertAgentConfig: api.KubeCertAgentSpec{
+				KubeCertAgentConfig: KubeCertAgentSpec{
 					NamePrefix:       stringPtr("kube-cert-agent-name-prefix-"),
 					Image:            stringPtr("kube-cert-agent-image"),
 					ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"},
@@ -79,23 +78,23 @@ func TestFromPath(t *testing.T) {
 				  credentialIssuerConfig: pinniped-config
 				  apiService: pinniped-api
 			`),
-			wantConfig: &api.Config{
+			wantConfig: &Config{
-				DiscoveryInfo: api.DiscoveryInfoSpec{
+				DiscoveryInfo: DiscoveryInfoSpec{
 					URL: nil,
 				},
-				APIConfig: api.APIConfigSpec{
+				APIConfig: APIConfigSpec{
-					ServingCertificateConfig: api.ServingCertificateConfigSpec{
+					ServingCertificateConfig: ServingCertificateConfigSpec{
 						DurationSeconds:    int64Ptr(60 * 60 * 24 * 365),    // about a year
 						RenewBeforeSeconds: int64Ptr(60 * 60 * 24 * 30 * 9), // about 9 months
 					},
 				},
-				NamesConfig: api.NamesConfigSpec{
+				NamesConfig: NamesConfigSpec{
 					ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate",
 					CredentialIssuerConfig:   "pinniped-config",
 					APIService:               "pinniped-api",
 				},
 				Labels: map[string]string{},
-				KubeCertAgentConfig: api.KubeCertAgentSpec{
+				KubeCertAgentConfig: KubeCertAgentSpec{
 					NamePrefix: stringPtr("pinniped-kube-cert-agent-"),
 					Image:      stringPtr("debian:latest"),
 				},

internal/config/concierge/types.go

@@ -1,9 +1,9 @@
 // Copyright 2020 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package api
+package concierge
 
-// Config contains knobs to setup an instance of Pinniped.
+// Config contains knobs to setup an instance of the Pinniped Concierge.
 type Config struct {
 	DiscoveryInfo       DiscoveryInfoSpec `json:"discovery"`
 	APIConfig           APIConfigSpec     `json:"api"`
@@ -14,7 +14,7 @@ type Config struct {
 
 // DiscoveryInfoSpec contains configuration knobs specific to
 // pinniped's publishing of discovery information. These values can be
-// viewed as overrides, i.e., if these are set, then pinniped will
+// viewed as overrides, i.e., if these are set, then Pinniped will
 // publish these values in its discovery document instead of the ones it finds.
 type DiscoveryInfoSpec struct {
 	// URL contains the URL at which pinniped can be contacted.
@@ -27,7 +27,7 @@ type APIConfigSpec struct {
 	ServingCertificateConfig ServingCertificateConfigSpec `json:"servingCertificate"`
 }
 
-// NamesConfigSpec configures the names of some Kubernetes resources for Pinniped.
+// NamesConfigSpec configures the names of some Kubernetes resources for the Concierge.
 type NamesConfigSpec struct {
 	ServingCertificateSecret string `json:"servingCertificateSecret"`
 	CredentialIssuerConfig   string `json:"credentialIssuerConfig"`

internal/config/supervisor/config.go

@@ -0,0 +1,34 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package supervisor contains functionality to load/store Config's from/to
+// some source.
+package supervisor
+
+import (
+	"fmt"
+	"io/ioutil"
+
+	"sigs.k8s.io/yaml"
+)
+
+// FromPath loads an Config from a provided local file path, inserts any
+// defaults (from the Config documentation), and verifies that the config is
+// valid (Config documentation).
+func FromPath(path string) (*Config, error) {
+	data, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("read file: %w", err)
+	}
+
+	var config Config
+	if err := yaml.Unmarshal(data, &config); err != nil {
+		return nil, fmt.Errorf("decode yaml: %w", err)
+	}
+
+	if config.Labels == nil {
+		config.Labels = make(map[string]string)
+	}
+
+	return &config, nil
+}

internal/config/supervisor/config_test.go

@@ -0,0 +1,69 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package supervisor
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/here"
+)
+
+func TestFromPath(t *testing.T) {
+	tests := []struct {
+		name       string
+		yaml       string
+		wantConfig *Config
+	}{
+		{
+			name: "Happy",
+			yaml: here.Doc(`
+				---
+				labels:
+				  myLabelKey1: myLabelValue1
+				  myLabelKey2: myLabelValue2
+			`),
+			wantConfig: &Config{
+				Labels: map[string]string{
+					"myLabelKey1": "myLabelValue1",
+					"myLabelKey2": "myLabelValue2",
+				},
+			},
+		},
+		{
+			name: "When only the required fields are present, causes other fields to be defaulted",
+			yaml: here.Doc(`
+				---
+			`),
+			wantConfig: &Config{
+				Labels: map[string]string{},
+			},
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			// Write yaml to temp file
+			f, err := ioutil.TempFile("", "pinniped-test-config-yaml-*")
+			require.NoError(t, err)
+			defer func() {
+				err := os.Remove(f.Name())
+				require.NoError(t, err)
+			}()
+			_, err = f.WriteString(test.yaml)
+			require.NoError(t, err)
+			err = f.Close()
+			require.NoError(t, err)
+
+			// Test FromPath()
+			config, err := FromPath(f.Name())
+
+			require.NoError(t, err)
+			require.Equal(t, test.wantConfig, config)
+		})
+	}
+}

internal/config/supervisor/types.go

@@ -0,0 +1,9 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package supervisor
+
+// Config contains knobs to setup an instance of the Pinniped Supervisor.
+type Config struct {
+	Labels map[string]string `json:"labels"`
+}

internal/controller/supervisorconfig/jwks.go

@@ -56,15 +56,17 @@ func generateECKey(r io.Reader) (interface{}, error) {
 // jwkController holds the fields necessary for the JWKS controller to communicate with OPC's and
 // secrets, both via a cache and via the API.
 type jwksController struct {
-	pinnipedClient pinnipedclientset.Interface
+	jwksSecretLabels map[string]string
-	kubeClient     kubernetes.Interface
+	pinnipedClient   pinnipedclientset.Interface
-	opcInformer    configinformers.OIDCProviderConfigInformer
+	kubeClient       kubernetes.Interface
-	secretInformer corev1informers.SecretInformer
+	opcInformer      configinformers.OIDCProviderConfigInformer
+	secretInformer   corev1informers.SecretInformer
 }
 
 // NewJWKSController returns a controllerlib.Controller that ensures an OPC has a corresponding
 // Secret that contains a valid active JWK and JWKS.
 func NewJWKSController(
+	jwksSecretLabels map[string]string,
 	kubeClient kubernetes.Interface,
 	pinnipedClient pinnipedclientset.Interface,
 	secretInformer corev1informers.SecretInformer,
@@ -75,10 +77,11 @@ func NewJWKSController(
 		controllerlib.Config{
 			Name: "JWKSController",
 			Syncer: &jwksController{
-				kubeClient:     kubeClient,
+				jwksSecretLabels: jwksSecretLabels,
-				pinnipedClient: pinnipedClient,
+				kubeClient:       kubeClient,
-				secretInformer: secretInformer,
+				pinnipedClient:   pinnipedClient,
-				opcInformer:    opcInformer,
+				secretInformer:   secretInformer,
+				opcInformer:      opcInformer,
 			},
 		},
 		// We want to be notified when a OPC's secret gets updated or deleted. When this happens, we
@@ -234,6 +237,7 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig)
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      opc.Name + "-jwks",
 			Namespace: opc.Namespace,
+			Labels:    c.jwksSecretLabels,
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(opc, schema.GroupVersionKind{
 					Group:   configv1alpha1.SchemeGroupVersion.Group,
@@ -241,7 +245,6 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig)
 					Kind:    opcKind,
 				}),
 			},
-			// TODO: custom labels.
 		},
 		Data: map[string][]byte{
 			activeJWKKey: jwkData,

internal/controller/supervisorconfig/jwks_test.go

@@ -151,6 +151,7 @@ func TestJWKSControllerFilterSecret(t *testing.T) {
 			).Config().V1alpha1().OIDCProviderConfigs()
 			withInformer := testutil.NewObservableWithInformerOption()
 			_ = NewJWKSController(
+				nil, // labels, not needed
 				nil, // kubeClient, not needed
 				nil, // pinnipedClient, not needed
 				secretInformer,
@@ -204,6 +205,7 @@ func TestJWKSControllerFilterOPC(t *testing.T) {
 			).Config().V1alpha1().OIDCProviderConfigs()
 			withInformer := testutil.NewObservableWithInformerOption()
 			_ = NewJWKSController(
+				nil, // labels, not needed
 				nil, // kubeClient, not needed
 				nil, // pinnipedClient, not needed
 				secretInformer,
@@ -264,6 +266,10 @@ func TestJWKSControllerSync(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      goodOPCWithStatus.Status.JWKSSecret.Name,
 				Namespace: namespace,
+				Labels: map[string]string{
+					"myLabelKey1": "myLabelValue1",
+					"myLabelKey2": "myLabelValue2",
+				},
 				OwnerReferences: []metav1.OwnerReference{
 					{
 						APIVersion:         opcGVR.GroupVersion().String(),
@@ -648,6 +654,10 @@ func TestJWKSControllerSync(t *testing.T) {
 			)
 
 			c := NewJWKSController(
+				map[string]string{
+					"myLabelKey1": "myLabelValue1",
+					"myLabelKey2": "myLabelValue2",
+				},
 				kubeAPIClient,
 				pinnipedAPIClient,
 				kubeInformers.Core().V1().Secrets(),

internal/controllermanager/prepare_controllers.go

@@ -22,6 +22,7 @@ import (
 	loginv1alpha1 "go.pinniped.dev/generated/1.19/apis/login/v1alpha1"
 	pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned"
 	pinnipedinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions"
+	"go.pinniped.dev/internal/config/concierge"
 	"go.pinniped.dev/internal/controller/apicerts"
 	"go.pinniped.dev/internal/controller/identityprovider/idpcache"
 	"go.pinniped.dev/internal/controller/identityprovider/webhookcachecleaner"
@@ -30,7 +31,6 @@ import (
 	"go.pinniped.dev/internal/controller/kubecertagent"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/dynamiccert"
-	"go.pinniped.dev/pkg/config/api"
 )
 
 const (
@@ -47,11 +47,11 @@ type Config struct {
 
 	// NamesConfig comes from the Pinniped config API (see api.Config). It specifies how Kubernetes
 	// objects should be named.
-	NamesConfig *api.NamesConfigSpec
+	NamesConfig *concierge.NamesConfigSpec
 
 	// KubeCertAgentConfig comes from the Pinniped config API (see api.Config). It configures how
 	// the kubecertagent package's controllers should manage the agent pods.
-	KubeCertAgentConfig *api.KubeCertAgentSpec
+	KubeCertAgentConfig *concierge.KubeCertAgentSpec
 
 	// DiscoveryURLOverride allows a caller to inject a hardcoded discovery URL into Pinniped
 	// discovery document.

test/integration/supervisor_keys_test.go

@@ -49,6 +49,12 @@ func TestSupervisorOIDCKeys(t *testing.T) {
 		Get(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.GetOptions{})
 	require.NoError(t, err)
 
+	// Ensure that the secret was labelled.
+	for k, v := range env.SupervisorCustomLabels {
+		require.Equalf(t, v, secret.Labels[k], "expected secret to have label `%s: %s`", k, v)
+	}
+	require.Equal(t, env.SupervisorAppName, secret.Labels["app"])
+
 	// Ensure the secret has an active key.
 	jwkData, ok := secret.Data["activeJWK"]
 	require.True(t, ok, "secret is missing active jwk")