From 94f20e57b19f2dcf6d1385b8fb102afe32eccc10 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 15 Oct 2020 10:14:23 -0700 Subject: [PATCH] Concierge controllers add labels to all created resources --- cmd/local-user-authenticator/main.go | 3 + deploy/concierge/deployment.yaml | 2 + deploy/concierge/values.yaml | 3 +- deploy/supervisor/deployment.yaml | 2 + deploy/supervisor/values.yaml | 3 +- hack/lib/tilt/Tiltfile | 4 +- hack/prepare-for-integration-tests.sh | 10 ++- internal/concierge/server/server.go | 1 + internal/controller/apicerts/certs_manager.go | 4 ++ .../controller/apicerts/certs_manager_test.go | 9 +++ ...eate_or_update_credential_issuer_config.go | 7 +- ...or_update_credential_issuer_config_test.go | 65 +++++++++++++++++-- .../kube_config_info_publisher.go | 4 ++ .../kube_config_info_publisher_test.go | 9 +++ .../controller/kubecertagent/annotater.go | 8 +-- internal/controller/kubecertagent/creater.go | 5 ++ .../controller/kubecertagent/creater_test.go | 22 ++++++- internal/controller/kubecertagent/execer.go | 24 +------ .../controller/kubecertagent/kubecertagent.go | 16 +++-- .../kubecertagent/kubecertagent_test.go | 2 + .../controllermanager/prepare_controllers.go | 7 ++ pkg/config/api/types.go | 1 + pkg/config/config.go | 4 ++ pkg/config/config_test.go | 8 +++ .../concierge_api_serving_certs_test.go | 8 +++ .../concierge_credentialissuerconfig_test.go | 6 ++ .../concierge_kubecertagent_test.go | 8 +++ test/library/env.go | 29 +++++++-- 28 files changed, 220 insertions(+), 54 deletions(-) diff --git a/cmd/local-user-authenticator/main.go b/cmd/local-user-authenticator/main.go index 4616c65d..f774500c 100644 --- a/cmd/local-user-authenticator/main.go +++ b/cmd/local-user-authenticator/main.go @@ -310,6 +310,9 @@ func startControllers( apicerts.NewCertsManagerController( namespace, certsSecretResourceName, + map[string]string{ + "app": "local-user-authenticator", + }, kubeClient, kubeInformers.Core().V1().Secrets(), controllerlib.WithInformer, diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index 1b54a34f..0a91e772 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -2,6 +2,7 @@ #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") +#@ load("@ytt:json", "json") #@ load("helpers.lib.yaml", "defaultLabel", "labels", "namespace", "defaultResourceName", "defaultResourceNameWithSuffix") #@ if not data.values.into_namespace: @@ -40,6 +41,7 @@ data: servingCertificateSecret: (@= defaultResourceNameWithSuffix("api-tls-serving-certificate") @) credentialIssuerConfig: (@= defaultResourceNameWithSuffix("config") @) apiService: (@= defaultResourceNameWithSuffix("api") @) + labels: (@= json.encode(labels()).rstrip() @) kubeCertAgent: namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @) (@ if data.values.kube_cert_agent_image: @) diff --git a/deploy/concierge/values.yaml b/deploy/concierge/values.yaml index bcb16a7a..00eddbdb 100644 --- a/deploy/concierge/values.yaml +++ b/deploy/concierge/values.yaml @@ -14,7 +14,8 @@ into_namespace: #! e.g. my-preexisting-namespace #! All resources created statically by yaml at install-time and all resources created dynamically #! by controllers at runtime will be labelled with `app: $app_name` and also with the labels -#! specified here. The app can be uninstalled either by: +#! specified here. The value of `custom_labels` must be a map of string keys to string values. +#! The app can be uninstalled either by: #! 1. Deleting the static install-time yaml resources including the static namespace, which will cascade and also delete #! resources that were dynamically created by controllers at runtime #! 2. Or, deleting all resources by label, which does not assume that there was a static install-time yaml namespace. diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml index bec84c79..150aadcb 100644 --- a/deploy/supervisor/deployment.yaml +++ b/deploy/supervisor/deployment.yaml @@ -2,6 +2,7 @@ #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") +#@ load("@ytt:json", "json") #@ load("helpers.lib.yaml", "defaultLabel", "labels", "namespace", "defaultResourceName", "defaultResourceNameWithSuffix") #@ if not data.values.into_namespace: @@ -31,6 +32,7 @@ data: pinniped.yaml: | names: dynamicConfigMap: (@= defaultResourceNameWithSuffix("dynamic-config") @) + labels: (@= json.encode(labels()).rstrip() @) --- #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "": apiVersion: v1 diff --git a/deploy/supervisor/values.yaml b/deploy/supervisor/values.yaml index 732d9759..982b96ed 100644 --- a/deploy/supervisor/values.yaml +++ b/deploy/supervisor/values.yaml @@ -14,7 +14,8 @@ into_namespace: #! e.g. my-preexisting-namespace #! All resources created statically by yaml at install-time and all resources created dynamically #! by controllers at runtime will be labelled with `app: $app_name` and also with the labels -#! specified here. The app can be uninstalled either by: +#! specified here. The value of `custom_labels` must be a map of string keys to string values. +#! The app can be uninstalled either by: #! 1. Deleting the static install-time yaml resources including the static namespace, which will cascade and also delete #! resources that were dynamically created by controllers at runtime #! 2. Or, deleting all resources by label, which does not assume that there was a static install-time yaml namespace. diff --git a/hack/lib/tilt/Tiltfile b/hack/lib/tilt/Tiltfile index 4f96c775..0ab02ce7 100644 --- a/hack/lib/tilt/Tiltfile +++ b/hack/lib/tilt/Tiltfile @@ -94,6 +94,7 @@ k8s_yaml(local([ '--data-value', 'image_tag=tilt-dev', '--data-value-yaml', 'replicas=1', '--data-value-yaml', 'service_nodeport_port=31234', + '--data-value-yaml', 'custom_labels={mySupervisorCustomLabelName: mySupervisorCustomLabelValue}', ])) # Tell tilt to watch all of those files for changes. watch_file('../../../deploy/supervisor') @@ -135,7 +136,8 @@ k8s_yaml(local([ '--data-value image_tag=tilt-dev ' + '--data-value kube_cert_agent_image=debian:10.5-slim ' + '--data-value discovery_url=$(TERM=dumb kubectl cluster-info | awk \'/Kubernetes master/ {print $NF}\') ' + - '--data-value-yaml replicas=1', + '--data-value-yaml replicas=1 ' + + '--data-value-yaml "custom_labels={myConciergeCustomLabelName: myConciergeCustomLabelValue}"' ])) # Tell tilt to watch all of those files for changes. watch_file('../../../deploy/concierge') diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index f0eb92ca..1216a6a9 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -212,6 +212,7 @@ kubectl create secret generic "$test_username" \ # supervisor_app_name="pinniped-supervisor" supervisor_namespace="supervisor" +supervisor_custom_labels="{mySupervisorCustomLabelName: mySupervisorCustomLabelValue}" if ! tilt_mode; then pushd deploy/supervisor >/dev/null @@ -222,6 +223,7 @@ if ! tilt_mode; then --data-value "namespace=$supervisor_namespace" \ --data-value "image_repo=$registry_repo" \ --data-value "image_tag=$tag" \ + --data-value-yaml "custom_labels=$supervisor_custom_labels" \ --data-value-yaml 'service_nodeport_port=31234' >"$manifest" kapp deploy --yes --app "$supervisor_app_name" --diff-changes --file "$manifest" @@ -230,21 +232,23 @@ if ! tilt_mode; then fi # -# Deploy Pinniped +# Deploy the Pinniped Concierge # concierge_app_name="pinniped-concierge" concierge_namespace="concierge" webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authenticate" webhook_ca_bundle="$(kubectl get secret local-user-authenticator-tls-serving-certificate --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')" discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" +concierge_custom_labels="{myConciergeCustomLabelName: myConciergeCustomLabelValue}" if ! tilt_mode; then pushd deploy/concierge >/dev/null - log_note "Deploying the Pinniped app to the cluster..." + log_note "Deploying the Pinniped Concierge app to the cluster..." ytt --file . \ --data-value "app_name=$concierge_app_name" \ --data-value "namespace=$concierge_namespace" \ + --data-value-yaml "custom_labels=$concierge_custom_labels" \ --data-value "image_repo=$registry_repo" \ --data-value "image_tag=$tag" \ --data-value "discovery_url=$discovery_url" >"$manifest" @@ -264,6 +268,7 @@ cat </tmp/integration-test-env # The following env vars should be set before running 'go test -v -count 1 ./test/integration' export PINNIPED_TEST_CONCIERGE_NAMESPACE=${concierge_namespace} export PINNIPED_TEST_CONCIERGE_APP_NAME=${concierge_app_name} +export PINNIPED_TEST_CONCIERGE_CUSTOM_LABELS='${concierge_custom_labels}' export PINNIPED_TEST_USER_USERNAME=${test_username} export PINNIPED_TEST_USER_GROUPS=${test_groups} export PINNIPED_TEST_USER_TOKEN=${test_username}:${test_password} @@ -271,6 +276,7 @@ export PINNIPED_TEST_WEBHOOK_ENDPOINT=${webhook_url} export PINNIPED_TEST_WEBHOOK_CA_BUNDLE=${webhook_ca_bundle} export PINNIPED_TEST_SUPERVISOR_NAMESPACE=${supervisor_namespace} export PINNIPED_TEST_SUPERVISOR_APP_NAME=${supervisor_app_name} +export PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS='${supervisor_custom_labels}' export PINNIPED_TEST_SUPERVISOR_ADDRESS="127.0.0.1:12345" export PINNIPED_TEST_CLI_OIDC_ISSUER=http://127.0.0.1:12346/dex export PINNIPED_TEST_CLI_OIDC_CLIENT_ID=pinniped-cli diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 23ad1258..904376d7 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -124,6 +124,7 @@ func (a *App) runServer(ctx context.Context) error { &controllermanager.Config{ ServerInstallationNamespace: serverInstallationNamespace, NamesConfig: &cfg.NamesConfig, + Labels: cfg.Labels, KubeCertAgentConfig: &cfg.KubeCertAgentConfig, DiscoveryURLOverride: cfg.DiscoveryInfo.URL, DynamicServingCertProvider: dynamicServingCertProvider, diff --git a/internal/controller/apicerts/certs_manager.go b/internal/controller/apicerts/certs_manager.go index b8bc8313..541eaf62 100644 --- a/internal/controller/apicerts/certs_manager.go +++ b/internal/controller/apicerts/certs_manager.go @@ -29,6 +29,7 @@ const ( type certsManagerController struct { namespace string certsSecretResourceName string + certsSecretLabels map[string]string k8sClient kubernetes.Interface secretInformer corev1informers.SecretInformer @@ -43,6 +44,7 @@ type certsManagerController struct { func NewCertsManagerController( namespace string, certsSecretResourceName string, + certsSecretLabels map[string]string, k8sClient kubernetes.Interface, secretInformer corev1informers.SecretInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, @@ -57,6 +59,7 @@ func NewCertsManagerController( Syncer: &certsManagerController{ namespace: namespace, certsSecretResourceName: certsSecretResourceName, + certsSecretLabels: certsSecretLabels, k8sClient: k8sClient, secretInformer: secretInformer, certDuration: certDuration, @@ -116,6 +119,7 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error { ObjectMeta: metav1.ObjectMeta{ Name: c.certsSecretResourceName, Namespace: c.namespace, + Labels: c.certsSecretLabels, }, StringData: map[string]string{ caCertificateSecretKey: string(aggregatedAPIServerCA.Bundle()), diff --git a/internal/controller/apicerts/certs_manager_test.go b/internal/controller/apicerts/certs_manager_test.go index f7e99bdd..d43babf6 100644 --- a/internal/controller/apicerts/certs_manager_test.go +++ b/internal/controller/apicerts/certs_manager_test.go @@ -42,6 +42,7 @@ func TestManagerControllerOptions(t *testing.T) { _ = NewCertsManagerController( installedInNamespace, certsSecretResourceName, + make(map[string]string), nil, secretsInformer, observableWithInformerOption.WithInformer, @@ -135,6 +136,10 @@ func TestManagerControllerSync(t *testing.T) { subject = NewCertsManagerController( installedInNamespace, certsSecretResourceName, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, kubeAPIClient, kubeInformers.Core().V1().Secrets(), controllerlib.WithInformer, @@ -198,6 +203,10 @@ func TestManagerControllerSync(t *testing.T) { actualSecret := actualAction.GetObject().(*corev1.Secret) r.Equal(certsSecretResourceName, actualSecret.Name) r.Equal(installedInNamespace, actualSecret.Namespace) + r.Equal(map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, actualSecret.Labels) actualCACert := actualSecret.StringData["caCertificate"] actualPrivateKey := actualSecret.StringData["tlsPrivateKey"] actualCertChain := actualSecret.StringData["tlsCertificateChain"] diff --git a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go b/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go index f420b850..a382d7c8 100644 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go +++ b/internal/controller/issuerconfig/create_or_update_credential_issuer_config.go @@ -21,6 +21,7 @@ func CreateOrUpdateCredentialIssuerConfig( ctx context.Context, credentialIssuerConfigNamespace string, credentialIssuerConfigResourceName string, + credentialIssuerConfigLabels map[string]string, pinnipedClient pinnipedclientset.Interface, applyUpdatesToCredentialIssuerConfigFunc func(configToUpdate *configv1alpha1.CredentialIssuerConfig), ) error { @@ -39,7 +40,9 @@ func CreateOrUpdateCredentialIssuerConfig( if notFound { // Create it - credentialIssuerConfig := minimalValidCredentialIssuerConfig(credentialIssuerConfigResourceName, credentialIssuerConfigNamespace) + credentialIssuerConfig := minimalValidCredentialIssuerConfig( + credentialIssuerConfigResourceName, credentialIssuerConfigNamespace, credentialIssuerConfigLabels, + ) applyUpdatesToCredentialIssuerConfigFunc(credentialIssuerConfig) if _, err := credentialIssuerConfigsClient.Create(ctx, credentialIssuerConfig, metav1.CreateOptions{}); err != nil { @@ -71,12 +74,14 @@ func CreateOrUpdateCredentialIssuerConfig( func minimalValidCredentialIssuerConfig( credentialIssuerConfigName string, credentialIssuerConfigNamespace string, + credentialIssuerConfigLabels map[string]string, ) *configv1alpha1.CredentialIssuerConfig { return &configv1alpha1.CredentialIssuerConfig{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigName, Namespace: credentialIssuerConfigNamespace, + Labels: credentialIssuerConfigLabels, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{}, diff --git a/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go b/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go index 13d1abc2..b20a4756 100644 --- a/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go +++ b/internal/controller/issuerconfig/create_or_update_credential_issuer_config_test.go @@ -45,7 +45,15 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { when("the config does not exist", func() { it("creates a new config which includes only the updates made by the func parameter", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.KubeConfigInfo = &configv1alpha1.CredentialIssuerConfigKubeConfigInfo{ CertificateAuthorityData: "some-ca-value", @@ -64,6 +72,10 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigResourceName, Namespace: installationNamespace, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{}, @@ -86,7 +98,12 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{}, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) {}, ) r.EqualError(err, "could not create or update credentialissuerconfig: create failed: error on create") @@ -103,6 +120,9 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigResourceName, Namespace: installationNamespace, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + }, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ @@ -124,7 +144,15 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("updates the existing config to only apply the updates made by the func parameter", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, @@ -142,7 +170,12 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("avoids the cost of an update if the local updates made by the func parameter did not actually change anything", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{}, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "initial-ca-value" @@ -166,7 +199,12 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{}, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) {}, ) r.EqualError(err, "could not create or update credentialissuerconfig: get failed: error on get") @@ -181,7 +219,12 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("returns an error", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{}, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, @@ -215,7 +258,15 @@ func TestCreateOrUpdateCredentialIssuerConfig(t *testing.T) { }) it("retries updates on conflict", func() { - err := CreateOrUpdateCredentialIssuerConfig(ctx, installationNamespace, credentialIssuerConfigResourceName, pinnipedAPIClient, + err := CreateOrUpdateCredentialIssuerConfig( + ctx, + installationNamespace, + credentialIssuerConfigResourceName, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, + pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { configToUpdate.Status.KubeConfigInfo.CertificateAuthorityData = "new-ca-value" }, diff --git a/internal/controller/issuerconfig/kube_config_info_publisher.go b/internal/controller/issuerconfig/kube_config_info_publisher.go index 3705a3d0..03f2b480 100644 --- a/internal/controller/issuerconfig/kube_config_info_publisher.go +++ b/internal/controller/issuerconfig/kube_config_info_publisher.go @@ -27,6 +27,7 @@ const ( type kubeConigInfoPublisherController struct { credentialIssuerConfigNamespaceName string credentialIssuerConfigResourceName string + credentialIssuerConfigLabels map[string]string serverOverride *string pinnipedClient pinnipedclientset.Interface configMapInformer corev1informers.ConfigMapInformer @@ -38,6 +39,7 @@ type kubeConigInfoPublisherController struct { func NewKubeConfigInfoPublisherController( credentialIssuerConfigNamespaceName string, credentialIssuerConfigResourceName string, + credentialIssuerConfigLabels map[string]string, serverOverride *string, pinnipedClient pinnipedclientset.Interface, configMapInformer corev1informers.ConfigMapInformer, @@ -49,6 +51,7 @@ func NewKubeConfigInfoPublisherController( Syncer: &kubeConigInfoPublisherController{ credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, credentialIssuerConfigNamespaceName: credentialIssuerConfigNamespaceName, + credentialIssuerConfigLabels: credentialIssuerConfigLabels, serverOverride: serverOverride, pinnipedClient: pinnipedClient, configMapInformer: configMapInformer, @@ -114,6 +117,7 @@ func (c *kubeConigInfoPublisherController) Sync(ctx controllerlib.Context) error ctx.Context, c.credentialIssuerConfigNamespaceName, c.credentialIssuerConfigResourceName, + c.credentialIssuerConfigLabels, 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 index f3750c1a..73e8afea 100644 --- a/internal/controller/issuerconfig/kube_config_info_publisher_test.go +++ b/internal/controller/issuerconfig/kube_config_info_publisher_test.go @@ -43,6 +43,7 @@ func TestInformerFilters(t *testing.T) { _ = NewKubeConfigInfoPublisherController( installedInNamespace, credentialIssuerConfigResourceName, + map[string]string{}, nil, nil, configMapInformer, @@ -127,6 +128,10 @@ func TestSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigResourceName, Namespace: expectedNamespace, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{}, @@ -146,6 +151,10 @@ func TestSync(t *testing.T) { subject = NewKubeConfigInfoPublisherController( installedInNamespace, credentialIssuerConfigResourceName, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, serverOverride, pinnipedAPIClient, kubeInformers.Core().V1().ConfigMaps(), diff --git a/internal/controller/kubecertagent/annotater.go b/internal/controller/kubecertagent/annotater.go index 9e6272bb..d7727784 100644 --- a/internal/controller/kubecertagent/annotater.go +++ b/internal/controller/kubecertagent/annotater.go @@ -122,13 +122,7 @@ func (c *annotaterController) Sync(ctx controllerlib.Context) error { keyPath, ); err != nil { err = fmt.Errorf("cannot update agent pod: %w", err) - strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig( - ctx.Context, - *c.credentialIssuerConfigLocationConfig, - c.clock, - c.pinnipedAPIClient, - err, - ) + strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig(ctx.Context, *c.credentialIssuerConfigLocationConfig, nil, c.clock, c.pinnipedAPIClient, err) if strategyResultUpdateErr != nil { // If the CIC update fails, then we probably want to try again. This controller will get // called again because of the pod create failure, so just try the CIC update again then. diff --git a/internal/controller/kubecertagent/creater.go b/internal/controller/kubecertagent/creater.go index 5d3200b3..e284f110 100644 --- a/internal/controller/kubecertagent/creater.go +++ b/internal/controller/kubecertagent/creater.go @@ -23,6 +23,7 @@ import ( type createrController struct { agentPodConfig *AgentPodConfig credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig + credentialIssuerConfigLabels map[string]string clock clock.Clock k8sClient kubernetes.Interface pinnipedAPIClient pinnipedclientset.Interface @@ -38,6 +39,7 @@ type createrController struct { func NewCreaterController( agentPodConfig *AgentPodConfig, credentialIssuerConfigLocationConfig *CredentialIssuerConfigLocationConfig, + credentialIssuerConfigLabels map[string]string, clock clock.Clock, k8sClient kubernetes.Interface, pinnipedAPIClient pinnipedclientset.Interface, @@ -53,6 +55,7 @@ func NewCreaterController( Syncer: &createrController{ agentPodConfig: agentPodConfig, credentialIssuerConfigLocationConfig: credentialIssuerConfigLocationConfig, + credentialIssuerConfigLabels: credentialIssuerConfigLabels, clock: clock, k8sClient: k8sClient, pinnipedAPIClient: pinnipedAPIClient, @@ -95,6 +98,7 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { return createOrUpdateCredentialIssuerConfig( ctx.Context, *c.credentialIssuerConfigLocationConfig, + c.credentialIssuerConfigLabels, c.clock, c.pinnipedAPIClient, constable.Error("did not find kube-controller-manager pod(s)"), @@ -129,6 +133,7 @@ func (c *createrController) Sync(ctx controllerlib.Context) error { strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig( ctx.Context, *c.credentialIssuerConfigLocationConfig, + c.credentialIssuerConfigLabels, c.clock, c.pinnipedAPIClient, err, diff --git a/internal/controller/kubecertagent/creater_test.go b/internal/controller/kubecertagent/creater_test.go index 58d6c4bd..e6fb80c3 100644 --- a/internal/controller/kubecertagent/creater_test.go +++ b/internal/controller/kubecertagent/creater_test.go @@ -42,7 +42,8 @@ func TestCreaterControllerFilter(t *testing.T) { _ = NewCreaterController( agentPodConfig, credentialIssuerConfigLocationConfig, - nil, // clock, shound't matter + map[string]string{}, + nil, // clock, shouldn't matter nil, // k8sClient, shouldn't matter nil, // pinnipedAPIClient, shouldn't matter kubeSystemPodInformer, @@ -66,7 +67,8 @@ func TestCreaterControllerInitialEvent(t *testing.T) { _ = NewCreaterController( nil, // agentPodConfig, shouldn't matter nil, // credentialIssuerConfigLocationConfig, shouldn't matter - nil, // clock, shound't matter + map[string]string{}, + nil, // clock, shouldn't matter nil, // k8sClient, shouldn't matter nil, // pinnipedAPIClient, shouldn't matter kubeSystemInformers.Core().V1().Pods(), @@ -111,11 +113,19 @@ func TestCreaterControllerSync(t *testing.T) { ContainerImage: "some-agent-image", PodNamePrefix: "some-agent-name-", ContainerImagePullSecrets: []string{"some-image-pull-secret"}, + AdditionalLabels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, }, &CredentialIssuerConfigLocationConfig{ Namespace: credentialIssuerConfigNamespaceName, Name: credentialIssuerConfigResourceName, }, + map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, clock.NewFakeClock(frozenNow), kubeAPIClient, pinnipedAPIClient, @@ -361,6 +371,10 @@ func TestCreaterControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigResourceName, Namespace: credentialIssuerConfigNamespaceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ @@ -502,6 +516,10 @@ func TestCreaterControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: credentialIssuerConfigResourceName, Namespace: credentialIssuerConfigNamespaceName, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, }, Status: configv1alpha1.CredentialIssuerConfigStatus{ Strategies: []configv1alpha1.CredentialIssuerConfigStrategy{ diff --git a/internal/controller/kubecertagent/execer.go b/internal/controller/kubecertagent/execer.go index 6791c6fd..ebc43f78 100644 --- a/internal/controller/kubecertagent/execer.go +++ b/internal/controller/kubecertagent/execer.go @@ -87,39 +87,21 @@ func (c *execerController) Sync(ctx controllerlib.Context) error { certPEM, err := c.podCommandExecutor.Exec(agentPod.Namespace, agentPod.Name, "cat", certPath) if err != nil { - strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig( - ctx.Context, - *c.credentialIssuerConfigLocationConfig, - c.clock, - c.pinnipedAPIClient, - err, - ) + strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig(ctx.Context, *c.credentialIssuerConfigLocationConfig, nil, c.clock, c.pinnipedAPIClient, err) klog.ErrorS(strategyResultUpdateErr, "could not create or update CredentialIssuerConfig with strategy success") return err } keyPEM, err := c.podCommandExecutor.Exec(agentPod.Namespace, agentPod.Name, "cat", keyPath) if err != nil { - strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig( - ctx.Context, - *c.credentialIssuerConfigLocationConfig, - c.clock, - c.pinnipedAPIClient, - err, - ) + strategyResultUpdateErr := createOrUpdateCredentialIssuerConfig(ctx.Context, *c.credentialIssuerConfigLocationConfig, nil, c.clock, c.pinnipedAPIClient, err) klog.ErrorS(strategyResultUpdateErr, "could not create or update CredentialIssuerConfig with strategy success") return err } c.dynamicCertProvider.Set([]byte(certPEM), []byte(keyPEM)) - err = createOrUpdateCredentialIssuerConfig( - ctx.Context, - *c.credentialIssuerConfigLocationConfig, - c.clock, - c.pinnipedAPIClient, - nil, // nil error = success! yay! - ) + err = createOrUpdateCredentialIssuerConfig(ctx.Context, *c.credentialIssuerConfigLocationConfig, nil, c.clock, c.pinnipedAPIClient, nil) if err != nil { return err } diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index 00e5e00e..7f816b39 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -61,12 +61,15 @@ type AgentPodConfig struct { // The container image used for the agent pods. ContainerImage string - // The name prefix for each of the agent pods. + // The name prefix for each of the agent pods. PodNamePrefix string // ContainerImagePullSecrets is a list of names of Kubernetes Secret objects that will be used as // ImagePullSecrets on the kube-cert-agent pods. ContainerImagePullSecrets []string + + // Additional labels that should be added to every agent pod during creation. + AdditionalLabels map[string]string } type CredentialIssuerConfigLocationConfig struct { @@ -78,9 +81,13 @@ type CredentialIssuerConfigLocationConfig struct { } func (c *AgentPodConfig) Labels() map[string]string { - return map[string]string{ + labels := map[string]string{ agentPodLabelKey: agentPodLabelValue, } + for k, v := range c.AdditionalLabels { + labels[k] = v + } + return labels } func (c *AgentPodConfig) PodTemplate() *corev1.Pod { @@ -258,9 +265,9 @@ func findControllerManagerPodForSpecificAgentPod( return maybeControllerManagerPod, nil } -func createOrUpdateCredentialIssuerConfig( - ctx context.Context, +func createOrUpdateCredentialIssuerConfig(ctx context.Context, cicConfig CredentialIssuerConfigLocationConfig, + credentialIssuerConfigLabels map[string]string, clock clock.Clock, pinnipedAPIClient pinnipedclientset.Interface, err error, @@ -269,6 +276,7 @@ func createOrUpdateCredentialIssuerConfig( ctx, cicConfig.Namespace, cicConfig.Name, + credentialIssuerConfigLabels, pinnipedAPIClient, func(configToUpdate *configv1alpha1.CredentialIssuerConfig) { var strategyResult configv1alpha1.CredentialIssuerConfigStrategy diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 6c8da06a..26011dde 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -79,6 +79,8 @@ func exampleControllerManagerAndAgentPods( Namespace: agentPodNamespace, Labels: map[string]string{ "kube-cert-agent.pinniped.dev": "true", + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", }, Annotations: map[string]string{ "kube-cert-agent.pinniped.dev/controller-manager-name": controllerManagerPod.Name, diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 88296f3d..633e0b36 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -72,6 +72,9 @@ type Config struct { // IDPCache is a cache of authenticators shared amongst various IDP-related controllers. IDPCache *idpcache.Cache + + // Labels are labels that should be added to any resources created by the controllers. + Labels map[string]string } // Prepare the controllers and their informers and return a function that will start them when called. @@ -96,6 +99,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { ContainerImage: *c.KubeCertAgentConfig.Image, PodNamePrefix: *c.KubeCertAgentConfig.NamePrefix, ContainerImagePullSecrets: c.KubeCertAgentConfig.ImagePullSecrets, + AdditionalLabels: c.Labels, } credentialIssuerConfigLocationConfig := &kubecertagent.CredentialIssuerConfigLocationConfig{ Namespace: c.ServerInstallationNamespace, @@ -112,6 +116,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { issuerconfig.NewKubeConfigInfoPublisherController( c.ServerInstallationNamespace, c.NamesConfig.CredentialIssuerConfig, + c.Labels, c.DiscoveryURLOverride, pinnipedClient, informers.kubePublicNamespaceK8s.Core().V1().ConfigMaps(), @@ -125,6 +130,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { apicerts.NewCertsManagerController( c.ServerInstallationNamespace, c.NamesConfig.ServingCertificateSecret, + c.Labels, k8sClient, informers.installationNamespaceK8s.Core().V1().Secrets(), controllerlib.WithInformer, @@ -174,6 +180,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { kubecertagent.NewCreaterController( agentPodConfig, credentialIssuerConfigLocationConfig, + c.Labels, clock.RealClock{}, k8sClient, pinnipedClient, diff --git a/pkg/config/api/types.go b/pkg/config/api/types.go index a5052222..da8c59e0 100644 --- a/pkg/config/api/types.go +++ b/pkg/config/api/types.go @@ -9,6 +9,7 @@ type Config struct { APIConfig APIConfigSpec `json:"api"` NamesConfig NamesConfigSpec `json:"names"` KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` + Labels map[string]string `json:"labels"` } // DiscoveryInfoSpec contains configuration knobs specific to diff --git a/pkg/config/config.go b/pkg/config/config.go index a7f4430c..97280001 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -50,6 +50,10 @@ func FromPath(path string) (*api.Config, error) { return nil, fmt.Errorf("validate names: %w", err) } + if config.Labels == nil { + config.Labels = make(map[string]string) + } + return &config, nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 8f9dd202..37fb0328 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -36,6 +36,9 @@ func TestFromPath(t *testing.T) { credentialIssuerConfig: pinniped-config apiService: pinniped-api kubeCertAgentPrefix: kube-cert-agent-prefix + labels: + myLabelKey1: myLabelValue1 + myLabelKey2: myLabelValue2 KubeCertAgent: namePrefix: kube-cert-agent-name-prefix- image: kube-cert-agent-image @@ -56,6 +59,10 @@ func TestFromPath(t *testing.T) { CredentialIssuerConfig: "pinniped-config", APIService: "pinniped-api", }, + Labels: map[string]string{ + "myLabelKey1": "myLabelValue1", + "myLabelKey2": "myLabelValue2", + }, KubeCertAgentConfig: api.KubeCertAgentSpec{ NamePrefix: stringPtr("kube-cert-agent-name-prefix-"), Image: stringPtr("kube-cert-agent-image"), @@ -87,6 +94,7 @@ func TestFromPath(t *testing.T) { CredentialIssuerConfig: "pinniped-config", APIService: "pinniped-api", }, + Labels: map[string]string{}, KubeCertAgentConfig: api.KubeCertAgentSpec{ NamePrefix: stringPtr("pinniped-kube-cert-agent-"), Image: stringPtr("debian:latest"), diff --git a/test/integration/concierge_api_serving_certs_test.go b/test/integration/concierge_api_serving_certs_test.go index 94f40e12..4ed38c45 100644 --- a/test/integration/concierge_api_serving_certs_test.go +++ b/test/integration/concierge_api_serving_certs_test.go @@ -90,6 +90,10 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { require.NotEmpty(t, initialCACert) require.NotEmpty(t, initialPrivateKey) require.NotEmpty(t, initialCertChain) + for k, v := range env.ConciergeCustomLabels { + require.Equalf(t, v, secret.Labels[k], "expected secret to have label %s: %s", k, v) + } + require.Equal(t, env.ConciergeAppName, secret.Labels["app"]) // Check that the APIService has the same CA. apiService, err := aggregatedClient.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{}) @@ -115,6 +119,10 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { require.NotEqual(t, initialCACert, regeneratedCACert) require.NotEqual(t, initialPrivateKey, regeneratedPrivateKey) require.NotEqual(t, initialCertChain, regeneratedCertChain) + for k, v := range env.ConciergeCustomLabels { + require.Equalf(t, v, secret.Labels[k], "expected secret to have label `%s: %s`", k, v) + } + require.Equal(t, env.ConciergeAppName, secret.Labels["app"]) // Expect that the APIService was also updated with the new CA. aggregatedAPIUpdated := func() bool { diff --git a/test/integration/concierge_credentialissuerconfig_test.go b/test/integration/concierge_credentialissuerconfig_test.go index 5616b09d..a31f63bd 100644 --- a/test/integration/concierge_credentialissuerconfig_test.go +++ b/test/integration/concierge_credentialissuerconfig_test.go @@ -33,8 +33,14 @@ func TestCredentialIssuerConfig(t *testing.T) { require.Len(t, actualConfigList.Items, 1) + actualConfig := actualConfigList.Items[0] actualStatusKubeConfigInfo := actualConfigList.Items[0].Status.KubeConfigInfo + for k, v := range env.ConciergeCustomLabels { + require.Equalf(t, v, actualConfig.Labels[k], "expected cic to have label `%s: %s`", k, v) + } + require.Equal(t, env.ConciergeAppName, actualConfig.Labels["app"]) + // Verify the cluster strategy status based on what's expected of the test cluster's ability to share signing keys. actualStatusStrategies := actualConfigList.Items[0].Status.Strategies require.Len(t, actualStatusStrategies, 1) diff --git a/test/integration/concierge_kubecertagent_test.go b/test/integration/concierge_kubecertagent_test.go index 6549fa4d..7066dfae 100644 --- a/test/integration/concierge_kubecertagent_test.go +++ b/test/integration/concierge_kubecertagent_test.go @@ -44,6 +44,14 @@ func TestKubeCertAgent(t *testing.T) { require.NotEmpty(t, originalAgentPods.Items) sortPods(originalAgentPods) + for _, agentPod := range originalAgentPods.Items { + // All agent pods should contain all custom labels + for k, v := range env.ConciergeCustomLabels { + require.Equalf(t, v, agentPod.Labels[k], "expected agent pod to have label `%s: %s`", k, v) + } + require.Equal(t, env.ConciergeAppName, agentPod.Labels["app"]) + } + agentPodsReconciled := func() bool { var currentAgentPods *corev1.PodList currentAgentPods, err = kubeClient.CoreV1().Pods(env.ConciergeNamespace).List(ctx, metav1.ListOptions{ diff --git a/test/library/env.go b/test/library/env.go index eba2a0d1..2d1b4a3b 100644 --- a/test/library/env.go +++ b/test/library/env.go @@ -26,13 +26,15 @@ const ( type TestEnv struct { t *testing.T - ConciergeNamespace string `json:"conciergeNamespace"` - SupervisorNamespace string `json:"supervisorNamespace"` - ConciergeAppName string `json:"conciergeAppName"` - SupervisorAppName string `json:"supervisorAppName"` - Capabilities map[Capability]bool `json:"capabilities"` - TestWebhook idpv1alpha1.WebhookIdentityProviderSpec `json:"testWebhook"` - SupervisorAddress string `json:"supervisorAddress"` + ConciergeNamespace string `json:"conciergeNamespace"` + SupervisorNamespace string `json:"supervisorNamespace"` + ConciergeAppName string `json:"conciergeAppName"` + SupervisorAppName string `json:"supervisorAppName"` + SupervisorCustomLabels map[string]string `json:"supervisorCustomLabels"` + ConciergeCustomLabels map[string]string `json:"conciergeCustomLabels"` + Capabilities map[Capability]bool `json:"capabilities"` + TestWebhook idpv1alpha1.WebhookIdentityProviderSpec `json:"testWebhook"` + SupervisorAddress string `json:"supervisorAddress"` TestUser struct { Token string `json:"token"` @@ -89,6 +91,19 @@ func IntegrationEnv(t *testing.T) *TestEnv { result.SupervisorAddress = needEnv("PINNIPED_TEST_SUPERVISOR_ADDRESS") result.TestWebhook.TLS = &idpv1alpha1.TLSSpec{CertificateAuthorityData: needEnv("PINNIPED_TEST_WEBHOOK_CA_BUNDLE")} + conciergeCustomLabelsYAML := needEnv("PINNIPED_TEST_CONCIERGE_CUSTOM_LABELS") + var conciergeCustomLabels map[string]string + err = yaml.Unmarshal([]byte(conciergeCustomLabelsYAML), &conciergeCustomLabels) + require.NoErrorf(t, err, "PINNIPED_TEST_CONCIERGE_CUSTOM_LABELS must be a YAML map of string to string") + result.ConciergeCustomLabels = conciergeCustomLabels + require.NotEmpty(t, result.ConciergeCustomLabels, "PINNIPED_TEST_CONCIERGE_CUSTOM_LABELS cannot be empty") + supervisorCustomLabelsYAML := needEnv("PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS") + var supervisorCustomLabels map[string]string + err = yaml.Unmarshal([]byte(supervisorCustomLabelsYAML), &supervisorCustomLabels) + require.NoErrorf(t, err, "PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS must be a YAML map of string to string") + result.SupervisorCustomLabels = supervisorCustomLabels + require.NotEmpty(t, result.SupervisorCustomLabels, "PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS cannot be empty") + result.OIDCUpstream.Issuer = needEnv("PINNIPED_TEST_CLI_OIDC_ISSUER") result.OIDCUpstream.ClientID = needEnv("PINNIPED_TEST_CLI_OIDC_CLIENT_ID") result.OIDCUpstream.LocalhostPort, _ = strconv.Atoi(needEnv("PINNIPED_TEST_CLI_OIDC_LOCALHOST_PORT"))