diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index 2c7338176..d24dbd02a 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -3,7 +3,16 @@ #@ load("@ytt:data", "data") #@ load("@ytt:json", "json") -#@ load("helpers.lib.yaml", "defaultLabel", "labels", "deploymentPodLabel", "namespace", "defaultResourceName", "defaultResourceNameWithSuffix", "getAndValidateLogLevel", "pinnipedDevAPIGroupWithPrefix") +#@ load("helpers.lib.yaml", +#@ "defaultLabel", +#@ "labels", +#@ "deploymentPodLabel", +#@ "namespace", +#@ "defaultResourceName", +#@ "defaultResourceNameWithSuffix", +#@ "getAndValidateLogLevel", +#@ "pinnipedDevAPIGroupWithPrefix", +#@ ) #@ load("@ytt:template", "template") #@ if not data.values.into_namespace: @@ -84,6 +93,7 @@ data: labels: (@= json.encode(labels()).rstrip() @) kubeCertAgent: namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @) + priorityClassName: (@= data.values.kube_cert_agent_priority_class_name @) (@ if data.values.kube_cert_agent_image: @) image: (@= data.values.kube_cert_agent_image @) (@ else: @) @@ -95,12 +105,12 @@ data: (@ end @) (@ if data.values.image_pull_dockerconfigjson: @) imagePullSecrets: - - image-pull-secret + - image-pull-secret (@ end @) (@ if data.values.log_level: @) log: level: (@= getAndValidateLogLevel() @) - (@ end @) + (@ end @) tls: onedottwo: allowedCiphers: (@= str(data.values.allowed_ciphers_for_tls_onedottwo) @) diff --git a/deploy/concierge/helpers.lib.yaml b/deploy/concierge/helpers.lib.yaml index 542fe069b..2ab680a52 100644 --- a/deploy/concierge/helpers.lib.yaml +++ b/deploy/concierge/helpers.lib.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") diff --git a/deploy/concierge/values.yaml b/deploy/concierge/values.yaml index e18b1cc8c..0eb011024 100644 --- a/deploy/concierge/values.yaml +++ b/deploy/concierge/values.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ def validate_strings_map(obj): @@ -68,15 +68,24 @@ image_digest: "" image_tag: latest #@schema/title "Kube Cert Agent image" -#@ kube_cert_agent_image = "Optionally specify a different image for the 'kube-cert-agent' pod which is scheduled \ +#@ kube_cert_agent_image_desc = "Optionally specify a different image for the 'kube-cert-agent' pod which is scheduled \ #@ on the control plane. This image needs only to include `sleep` and `cat` binaries. \ #@ By default, the same image specified for image_repo/image_digest/image_tag will be re-used." -#@schema/desc kube_cert_agent_image +#@schema/desc kube_cert_agent_image_desc #@schema/examples ("Image including tag or digest", "ghcr.io/vmware-tanzu/pinniped/pinniped-server:latest") #@schema/nullable #@schema/validation min_len=1 kube_cert_agent_image: "" +#@schema/title "Kube Cert Agent Priority Class Name" +#@ kube_cert_agent_priority_class_name_desc = "Optionally specify a PriorityClassName for the 'kube-cert-agent' pod. \ +#@ See https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/ for more details. \ +#@ By default, this is the empty string." +#@schema/desc kube_cert_agent_priority_class_name_desc +#@schema/examples ("name of a PriorityClass object", "high-priority") +#@schema/validation min_len=0 +kube_cert_agent_priority_class_name: "" + #@schema/title "Image pull dockerconfigjson" #@ image_pull_dockerconfigjson_desc = "A base64 encoded secret to be used when pulling the `image_repo` container image. \ #@ Can be used when the image_repo is a private registry. Typically, the value would be the output of: \ diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml index 909e424bf..eac3f393d 100644 --- a/deploy/supervisor/deployment.yaml +++ b/deploy/supervisor/deployment.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") @@ -13,7 +13,7 @@ #@ "pinnipedDevAPIGroupWithPrefix", #@ "getPinnipedConfigMapData", #@ "hasUnixNetworkEndpoint", -#@ ) +#@ ) #@ load("@ytt:template", "template") #@ if not data.values.into_namespace: diff --git a/hack/module.sh b/hack/module.sh index 4c86b1a96..09319e743 100755 --- a/hack/module.sh +++ b/hack/module.sh @@ -9,7 +9,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" function usage() { echo "Error: must be specified" - echo " module.sh [tidy, lint, test, unittest]" + echo " module.sh [tidy, lint, unit, unittest]" exit 1 } @@ -49,10 +49,12 @@ function main() { # Temporarily avoid using the race detector for the impersonator package due to https://github.com/kubernetes/kubernetes/issues/128548 KUBE_CACHE_MUTATION_DETECTOR=${kube_cache_mutation_detector} \ KUBE_PANIC_WATCH_DECODE_ERROR=${kube_panic_watch_decode_error} \ + CGO_ENABLED=0 \ go test -short -race $(go list ./... | grep -v internal/concierge/impersonator) # TODO: change this back to using the race detector everywhere KUBE_CACHE_MUTATION_DETECTOR=${kube_cache_mutation_detector} \ KUBE_PANIC_WATCH_DECODE_ERROR=${kube_panic_watch_decode_error} \ + CGO_ENABLED=0 \ go test -short ./internal/concierge/impersonator ;; 'generate') diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index 6bf31f9e8..ad0f43dfc 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -11,6 +11,7 @@ import ( "os" "strings" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/utils/ptr" "sigs.k8s.io/yaml" @@ -85,6 +86,10 @@ func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowe return nil, fmt.Errorf("validate names: %w", err) } + if err := validateKubeCertAgent(&config.KubeCertAgentConfig); err != nil { + return nil, fmt.Errorf("validate kubeCertAgent: %w", err) + } + if err := plog.ValidateAndSetLogLevelAndFormatGlobally(ctx, config.Log); err != nil { return nil, fmt.Errorf("validate log level: %w", err) } @@ -186,6 +191,22 @@ func validateNames(names *NamesConfigSpec) error { return nil } +func validateKubeCertAgent(agentConfig *KubeCertAgentSpec) error { + if len(agentConfig.PriorityClassName) == 0 { + // Optional, so empty is valid. + return nil + } + + // See https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#priorityclass + // for PriorityClassName rules. + errStrings := validation.IsDNS1123Subdomain(agentConfig.PriorityClassName) + if len(errStrings) > 0 { + // Always good enough to return the first error. IsDNS1123Subdomain only has two errors that it can return. + return fmt.Errorf("invalid priorityClassName: %s", errStrings[0]) + } + return nil +} + func validateAPI(apiConfig *APIConfigSpec) error { if *apiConfig.ServingCertificateConfig.DurationSeconds < *apiConfig.ServingCertificateConfig.RenewBeforeSeconds { return constable.Error("durationSeconds cannot be smaller than renewBeforeSeconds") diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index 4767cddb0..0ff1d512b 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "os" + "strings" "testing" "github.com/stretchr/testify/require" @@ -17,6 +18,9 @@ import ( ) func TestFromPath(t *testing.T) { + stringOfLength253 := strings.Repeat("a", 253) + stringOfLength254 := strings.Repeat("a", 254) + tests := []struct { name string yaml string @@ -26,7 +30,7 @@ func TestFromPath(t *testing.T) { }{ { name: "Fully filled out", - yaml: here.Doc(` + yaml: here.Docf(` --- discovery: url: https://some.discovery/url @@ -64,6 +68,7 @@ func TestFromPath(t *testing.T) { namePrefix: kube-cert-agent-name-prefix- image: kube-cert-agent-image imagePullSecrets: [kube-cert-agent-image-pull-secret] + priorityClassName: %s log: level: debug tls: @@ -74,7 +79,7 @@ func TestFromPath(t *testing.T) { - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 audit: logUsernamesAndGroups: enabled - `), + `, stringOfLength253), wantConfig: &Config{ DiscoveryInfo: DiscoveryInfoSpec{ URL: ptr.To("https://some.discovery/url"), @@ -112,9 +117,10 @@ func TestFromPath(t *testing.T) { "myLabelKey2": "myLabelValue2", }, KubeCertAgentConfig: KubeCertAgentSpec{ - NamePrefix: ptr.To("kube-cert-agent-name-prefix-"), - Image: ptr.To("kube-cert-agent-image"), - ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, + NamePrefix: ptr.To("kube-cert-agent-name-prefix-"), + Image: ptr.To("kube-cert-agent-image"), + ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, + PriorityClassName: stringOfLength253, }, Log: plog.LogSpec{ Level: plog.LevelDebug, @@ -173,6 +179,7 @@ func TestFromPath(t *testing.T) { namePrefix: kube-cert-agent-name-prefix- image: kube-cert-agent-image imagePullSecrets: [kube-cert-agent-image-pull-secret] + priorityClassName: kube-cert-agent-priority-class-name log: level: all format: json @@ -216,9 +223,10 @@ func TestFromPath(t *testing.T) { "myLabelKey2": "myLabelValue2", }, KubeCertAgentConfig: KubeCertAgentSpec{ - NamePrefix: ptr.To("kube-cert-agent-name-prefix-"), - Image: ptr.To("kube-cert-agent-image"), - ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, + NamePrefix: ptr.To("kube-cert-agent-name-prefix-"), + Image: ptr.To("kube-cert-agent-image"), + ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, + PriorityClassName: "kube-cert-agent-priority-class-name", }, Log: plog.LogSpec{ Level: plog.LevelAll, @@ -703,6 +711,50 @@ func TestFromPath(t *testing.T) { `), wantError: "validate audit: invalid logUsernamesAndGroups format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')", }, + { + name: "invalid kubeCertAgent.priorityClassName length", + yaml: here.Docf(` + --- + names: + servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate + credentialIssuer: pinniped-config + apiService: pinniped-api + impersonationLoadBalancerService: impersonationLoadBalancerService-value + impersonationClusterIPService: impersonationClusterIPService-value + impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value + impersonationCACertificateSecret: impersonationCACertificateSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + agentServiceAccount: agentServiceAccount-value + impersonationProxyServiceAccount: impersonationProxyServiceAccount-value + impersonationProxyLegacySecret: impersonationProxyLegacySecret-value + kubeCertAgent: + priorityClassName: %s + `, stringOfLength254), + wantError: "validate kubeCertAgent: invalid priorityClassName: must be no more than 253 characters", + }, + { + name: "invalid kubeCertAgent.priorityClassName format", + yaml: here.Doc(` + --- + names: + servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate + credentialIssuer: pinniped-config + apiService: pinniped-api + impersonationLoadBalancerService: impersonationLoadBalancerService-value + impersonationClusterIPService: impersonationClusterIPService-value + impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value + impersonationCACertificateSecret: impersonationCACertificateSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + agentServiceAccount: agentServiceAccount-value + impersonationProxyServiceAccount: impersonationProxyServiceAccount-value + impersonationProxyLegacySecret: impersonationProxyLegacySecret-value + kubeCertAgent: + priorityClassName: thisIsNotAValidPriorityClassName + `), + wantError: `validate kubeCertAgent: invalid priorityClassName: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index 9fbb3a2cb..d86dc8d1d 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -108,4 +108,7 @@ type KubeCertAgentSpec struct { // ImagePullSecrets is a list of names of Kubernetes Secret objects that will be used as // ImagePullSecrets on the kube-cert-agent pods. ImagePullSecrets []string + + // PriorityClassName optionally sets the PriorityClassName for the agent's pod. + PriorityClassName string `json:"priorityClassName"` } diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index 0a9d17a18..f4940c8c1 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package kubecertagent provides controllers that ensure a pod (the kube-cert-agent), is @@ -101,6 +101,9 @@ type AgentConfig struct { // DiscoveryURLOverride is the Kubernetes server endpoint to report in the CredentialIssuer, overriding any // value discovered in the kube-public/cluster-info ConfigMap. DiscoveryURLOverride *string + + // PriorityClassName optionally sets the PriorityClassName for the agent's pod. + PriorityClassName string } // Only select using the unique label which will not match the pods of any other Deployment. @@ -429,12 +432,14 @@ func (c *agentController) createOrUpdateDeployment(ctx context.Context, newestCo updatedDeployment.ObjectMeta = mergeLabelsAndAnnotations(updatedDeployment.ObjectMeta, expectedDeployment.ObjectMeta) desireSelectorUpdate := !apiequality.Semantic.DeepEqual(updatedDeployment.Spec.Selector, existingDeployment.Spec.Selector) desireTemplateLabelsUpdate := !apiequality.Semantic.DeepEqual(updatedDeployment.Spec.Template.Labels, existingDeployment.Spec.Template.Labels) + // The user might want to set PriorityClassName back to the default value of empty string. DeepDerivative() won't detect this case below. + desirePriorityClassNameUpdate := updatedDeployment.Spec.Template.Spec.PriorityClassName != existingDeployment.Spec.Template.Spec.PriorityClassName // If the existing Deployment already matches our desired spec, we're done. if apiequality.Semantic.DeepDerivative(updatedDeployment, existingDeployment) { // DeepDerivative allows the map fields of updatedDeployment to be a subset of existingDeployment, // but we want to check that certain of those map fields are exactly equal before deciding to skip the update. - if !desireSelectorUpdate && !desireTemplateLabelsUpdate { + if !desireSelectorUpdate && !desireTemplateLabelsUpdate && !desirePriorityClassNameUpdate { return nil // already equal enough, so skip update } } @@ -661,7 +666,8 @@ func (c *agentController) newAgentDeployment(controllerManagerPod *corev1.Pod) * RunAsUser: ptr.To[int64](0), RunAsGroup: ptr.To[int64](0), }, - HostNetwork: controllerManagerPod.Spec.HostNetwork, + HostNetwork: controllerManagerPod.Spec.HostNetwork, + PriorityClassName: c.cfg.PriorityClassName, }, }, diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index c6b715300..aa1c6f336 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -229,6 +229,7 @@ func TestAgentController(t *testing.T) { tests := []struct { name string discoveryURLOverride *string + agentPriorityClassName string pinnipedObjects []runtime.Object kubeObjects []runtime.Object addKubeReactions func(*kubefake.Clientset) @@ -396,6 +397,59 @@ func TestAgentController(t *testing.T) { LastUpdateTime: metav1.NewTime(now), }, }, + { + name: "created new deployment with overridden priorityClassName, no agent pods running yet", + agentPriorityClassName: "some-custom-priority-class-name-for-agent", + pinnipedObjects: []runtime.Object{ + initialCredentialIssuer, + }, + kubeObjects: []runtime.Object{ + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kube-system", + Name: "kube-controller-manager-3", + Labels: map[string]string{"component": "kube-controller-manager"}, + CreationTimestamp: metav1.NewTime(now.Add(-1 * time.Hour)), + }, + Spec: corev1.PodSpec{NodeName: schedulableControllerManagerNode.Name}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + healthyKubeControllerManagerPod, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kube-system", + Name: "kube-controller-manager-2", + Labels: map[string]string{"component": "kube-controller-manager"}, + CreationTimestamp: metav1.NewTime(now.Add(-2 * time.Hour)), + }, + Spec: corev1.PodSpec{NodeName: schedulableControllerManagerNode.Name}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + pendingAgentPod, + schedulableControllerManagerNode, + }, + wantDistinctErrors: []string{ + "could not find a healthy agent pod (1 candidate)", + }, + alsoAllowUndesiredDistinctErrors: []string{ + // due to the high amount of nondeterminism in this test, this error will sometimes also happen, but is not required to happen + `could not ensure agent deployment: deployments.apps "pinniped-concierge-kube-cert-agent" already exists`, + }, + wantDistinctLogs: []string{ + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"kube-cert-agent-controller","caller":"kubecertagent/kubecertagent.go:$kubecertagent.(*agentController).createOrUpdateDeployment","message":"creating new deployment","deployment":{"name":"pinniped-concierge-kube-cert-agent","namespace":"concierge"},"templatePod":{"name":"kube-controller-manager-1","namespace":"kube-system"}}`, + }, + wantAgentDeployment: modifiedHealthyHealthyAgentDeployment(func(deployment *appsv1.Deployment) { + deployment.Spec.Template.Spec.PriorityClassName = "some-custom-priority-class-name-for-agent" + }), + wantDeploymentActionVerbs: []string{"list", "watch", "create"}, + wantStrategy: &conciergeconfigv1alpha1.CredentialIssuerStrategy{ + Type: conciergeconfigv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: conciergeconfigv1alpha1.ErrorStrategyStatus, + Reason: conciergeconfigv1alpha1.CouldNotFetchKeyStrategyReason, + Message: "could not find a healthy agent pod (1 candidate)", + LastUpdateTime: metav1.NewTime(now), + }, + }, { name: "created new deployment based on alternate supported controller-manager CLI flags, no agent pods running yet", pinnipedObjects: []runtime.Object{ @@ -1385,6 +1439,82 @@ func TestAgentController(t *testing.T) { }, }, }, + { + name: "deployment exists, configmap is valid, exec succeeds, overridden priorityClassName is updated into the deployment", + pinnipedObjects: []runtime.Object{ + initialCredentialIssuer, + }, + kubeObjects: []runtime.Object{ + healthyKubeControllerManagerPod, + healthyAgentDeployment, + healthyAgentPod, + validClusterInfoConfigMap, + schedulableControllerManagerNode, + }, + agentPriorityClassName: "some-custom-priority-class-name-for-agent", + mocks: mockExecSucceeds, + wantDistinctErrors: []string{""}, + wantAgentDeployment: modifiedHealthyHealthyAgentDeployment(func(deployment *appsv1.Deployment) { + deployment.Spec.Template.Spec.PriorityClassName = "some-custom-priority-class-name-for-agent" + }), + wantDeploymentActionVerbs: []string{"list", "watch", "update"}, + wantDistinctLogs: []string{ + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"kube-cert-agent-controller","caller":"kubecertagent/kubecertagent.go:$kubecertagent.(*agentController).createOrUpdateDeployment","message":"updating existing deployment","deployment":{"name":"pinniped-concierge-kube-cert-agent","namespace":"concierge"},"templatePod":{"name":"kube-controller-manager-1","namespace":"kube-system"}}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"kube-cert-agent-controller","caller":"kubecertagent/kubecertagent.go:$kubecertagent.(*agentController).loadSigningKey","message":"successfully loaded signing key from agent pod into cache"}`, + }, + wantStrategy: &conciergeconfigv1alpha1.CredentialIssuerStrategy{ + Type: conciergeconfigv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: conciergeconfigv1alpha1.SuccessStrategyStatus, + Reason: conciergeconfigv1alpha1.FetchedKeyStrategyReason, + Message: "key was fetched successfully", + LastUpdateTime: metav1.NewTime(now), + Frontend: &conciergeconfigv1alpha1.CredentialIssuerFrontend{ + Type: conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType, + TokenCredentialRequestAPIInfo: &conciergeconfigv1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://test-kubernetes-endpoint.example.com", + CertificateAuthorityData: "dGVzdC1rdWJlcm5ldGVzLWNh", + }, + }, + }, + }, + { + name: "deployment exists with a non-empty priorityClassName, configmap is valid, exec succeeds, priorityClassName config is empty string so priorityClassName is set back to empty", + pinnipedObjects: []runtime.Object{ + initialCredentialIssuer, + }, + kubeObjects: []runtime.Object{ + healthyKubeControllerManagerPod, + modifiedHealthyHealthyAgentDeployment(func(deployment *appsv1.Deployment) { + deployment.Spec.Template.Spec.PriorityClassName = "some-custom-priority-class-name-for-agent" + }), + healthyAgentPod, + validClusterInfoConfigMap, + schedulableControllerManagerNode, + }, + agentPriorityClassName: "", + mocks: mockExecSucceeds, + wantDistinctErrors: []string{""}, + wantAgentDeployment: healthyAgentDeployment, + wantDeploymentActionVerbs: []string{"list", "watch", "update"}, + wantDistinctLogs: []string{ + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"kube-cert-agent-controller","caller":"kubecertagent/kubecertagent.go:$kubecertagent.(*agentController).createOrUpdateDeployment","message":"updating existing deployment","deployment":{"name":"pinniped-concierge-kube-cert-agent","namespace":"concierge"},"templatePod":{"name":"kube-controller-manager-1","namespace":"kube-system"}}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"kube-cert-agent-controller","caller":"kubecertagent/kubecertagent.go:$kubecertagent.(*agentController).loadSigningKey","message":"successfully loaded signing key from agent pod into cache"}`, + }, + wantStrategy: &conciergeconfigv1alpha1.CredentialIssuerStrategy{ + Type: conciergeconfigv1alpha1.KubeClusterSigningCertificateStrategyType, + Status: conciergeconfigv1alpha1.SuccessStrategyStatus, + Reason: conciergeconfigv1alpha1.FetchedKeyStrategyReason, + Message: "key was fetched successfully", + LastUpdateTime: metav1.NewTime(now), + Frontend: &conciergeconfigv1alpha1.CredentialIssuerFrontend{ + Type: conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType, + TokenCredentialRequestAPIInfo: &conciergeconfigv1alpha1.TokenCredentialRequestAPIInfo{ + Server: "https://test-kubernetes-endpoint.example.com", + CertificateAuthorityData: "dGVzdC1rdWJlcm5ldGVzLWNh", + }, + }, + }, + }, { name: "error while listing nodes", pinnipedObjects: []runtime.Object{ @@ -1492,6 +1622,7 @@ func TestAgentController(t *testing.T) { "app": "anything", }, DiscoveryURLOverride: tt.discoveryURLOverride, + PriorityClassName: tt.agentPriorityClassName, }, &kubeclient.Client{Kubernetes: kubeClientset, PinnipedConcierge: conciergeClientset}, kubeInformers.Core().V1().Pods(), diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index b3aa5468c..ff77157e5 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package controllermanager provides an entrypoint into running all of the controllers that run as @@ -140,6 +140,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol Labels: c.Labels, CredentialIssuerName: c.NamesConfig.CredentialIssuer, DiscoveryURLOverride: c.DiscoveryURLOverride, + PriorityClassName: c.KubeCertAgentConfig.PriorityClassName, } // Create controller manager. diff --git a/test/integration/audit_test.go b/test/integration/audit_test.go index 68c6f7652..9e9a0bfe0 100644 --- a/test/integration/audit_test.go +++ b/test/integration/audit_test.go @@ -62,7 +62,7 @@ func TestAuditLogsDuringLogin_Disruptive(t *testing.T) { testStartTime := metav1.Now() ctx, cancelFunc := context.WithTimeout(context.Background(), 10*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) kubeClient := testlib.NewKubernetesClientset(t) kubeClientForK8sResourcesOnly := kubeClientWithoutPinnipedAPISuffix(t) @@ -377,7 +377,7 @@ func TestAuditLogsDuringLogin_Disruptive(t *testing.T) { func TestAuditLogsEmittedForDiscoveryEndpoints_Parallel(t *testing.T) { ctx, cancelFunc := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) env, kubeClientForK8sResourcesOnly, fakeIssuerForDisplayPurposes, ca, dnsOverrides := auditSetup(t, ctx) @@ -427,7 +427,7 @@ func TestAuditLogsEmittedForDiscoveryEndpoints_Parallel(t *testing.T) { // /oauth2/authorize, /callback, /login, and /oauth2/token. func TestAuditLogsEmittedForEndpointsEvenWhenTheCallsAreInvalid_Parallel(t *testing.T) { ctx, cancelFunc := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) env, kubeClientForK8sResourcesOnly, fakeIssuerForDisplayPurposes, ca, dnsOverrides := auditSetup(t, ctx) diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 36269e339..45439c30f 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -42,7 +42,7 @@ func TestCLIGetKubeconfigStaticToken_Parallel(t *testing.T) { // Create a test webhook configuration to use with the CLI. ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) authenticator := testlib.CreateTestWebhookAuthenticator(ctx, t, &testlib.IntegrationEnv(t).TestWebhook, authenticationv1alpha1.WebhookAuthenticatorPhaseReady) diff --git a/test/integration/concierge_kube_cert_agent_priority_class_test.go b/test/integration/concierge_kube_cert_agent_priority_class_test.go new file mode 100644 index 000000000..647048f98 --- /dev/null +++ b/test/integration/concierge_kube_cert_agent_priority_class_test.go @@ -0,0 +1,140 @@ +// Copyright 2025 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" + + "go.pinniped.dev/internal/config/concierge" + "go.pinniped.dev/test/testlib" +) + +// TestKubeCertAgentPriorityClassName tests the feature which allows the user to configure +// the priorityClassName for the kube cert agent Deployment's pods. This test is Disruptive because +// it restarts the Concierge to reconfigure this setting, and then restarts it +// again to put back the original configuration. +func TestKubeCertAgentPriorityClassName_Disruptive(t *testing.T) { + env := testEnvForPodShutdownTests(t) + + // The name of the Concierge static configmap. + staticConfigMapName := env.ConciergeAppName + "-config" + + ctx, cancelFunc := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancelFunc) + + kubeClient := testlib.NewKubernetesClientset(t) + + // Get the Concierge's static configmap. + staticConfigMap, err := kubeClient.CoreV1().ConfigMaps(env.ConciergeNamespace). + Get(ctx, staticConfigMapName, metav1.GetOptions{}) + require.NoError(t, err) + + // Parse the content of the static configmap and check some preconditions. + originalConfigYAML := staticConfigMap.Data["pinniped.yaml"] + require.NotEmpty(t, originalConfigYAML) + var originalConfig concierge.Config + err = yaml.Unmarshal([]byte(originalConfigYAML), &originalConfig) + require.NoError(t, err) + + // Should have a name for the kube cert agent configured. + require.NotNil(t, originalConfig.KubeCertAgentConfig.NamePrefix) + require.NotEmpty(t, *originalConfig.KubeCertAgentConfig.NamePrefix) + // The default value should end with "-". + require.True(t, strings.HasSuffix(*originalConfig.KubeCertAgentConfig.NamePrefix, "-")) + kubeCertAgentDeploymentName := strings.TrimSuffix(*originalConfig.KubeCertAgentConfig.NamePrefix, "-") + require.NotEmpty(t, kubeCertAgentDeploymentName) + + // PriorityClassName configuration should be empty by default. + require.Empty(t, originalConfig.KubeCertAgentConfig.PriorityClassName) + + // Get the actual kube cert agent deployment. + originalKubeCertAgentDeployment, err := kubeClient.AppsV1().Deployments(env.ConciergeNamespace). + Get(ctx, kubeCertAgentDeploymentName, metav1.GetOptions{}) + require.NoError(t, err) + // Should not have PriorityClassName by default. + require.Empty(t, originalKubeCertAgentDeployment.Spec.Template.Spec.PriorityClassName) + + // PriorityClass "system-cluster-critical" exists on all Kubernetes clusters by default. + // See https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#how-to-use-priority-and-preemption. + newlyConfiguredPriorityClassName := "system-cluster-critical" + + // Schedule this now so it runs after the cleanup scheduled by updateStaticConfigMapAndRestartApp() below. + t.Cleanup(func() { + // After some time, the kube cert agent deployment should revert back to its default value + // of not having any PriorityClassName configured on it. + t.Log("waiting for kube cert agent deployment to get original (empty) PriorityClassName") + testlib.RequireEventually(t, + func(requireEventually *require.Assertions) { + // The deployment should be updated. + updatedKubeCertAgentDeployment, err := kubeClient.AppsV1().Deployments(env.ConciergeNamespace). + Get(ctx, kubeCertAgentDeploymentName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventually.Empty(updatedKubeCertAgentDeployment.Spec.Template.Spec.PriorityClassName) + + // The deployment should have rolled out a new pod which has the original (empty) PriorityClassName. + newPods := getRunningPodsByNamePrefix(t, env.ConciergeNamespace, kubeCertAgentDeploymentName+"-", "") + requireEventually.Equal(len(newPods), 1) + requireEventually.True(allPodsReady(newPods), "wanted new pod to be ready") + requireEventually.Empty(newPods[0].Spec.PriorityClassName) + + t.Log("observed that kube cert agent deployment and pod both got original (empty) PriorityClassName") + }, + 2*time.Minute, + 250*time.Millisecond, + ) + }) + + t.Log("updating Concierge's static ConfigMap and restarting the pods") + updateStaticConfigMapAndRestartApp(t, + ctx, + env.ConciergeNamespace, + staticConfigMapName, + env.ConciergeAppName, + true, + func(t *testing.T, configMapData string) string { + t.Helper() + + var config concierge.Config + err := yaml.Unmarshal([]byte(configMapData), &config) + require.NoError(t, err) + + config.KubeCertAgentConfig.PriorityClassName = newlyConfiguredPriorityClassName + + updatedConfig, err := yaml.Marshal(config) + require.NoError(t, err) + return string(updatedConfig) + }, + ) + + // After some time, the kube cert agent deployment should have the new PriorityClassName configured on it. + t.Log("waiting for kube cert agent deployment to get new PriorityClassName") + testlib.RequireEventually(t, + func(requireEventually *require.Assertions) { + // The deployment should be updated. + updatedKubeCertAgentDeployment, err := kubeClient.AppsV1().Deployments(env.ConciergeNamespace). + Get(ctx, kubeCertAgentDeploymentName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventually.Equal(newlyConfiguredPriorityClassName, updatedKubeCertAgentDeployment.Spec.Template.Spec.PriorityClassName) + + // The deployment should have rolled out a new pod which has the new PriorityClassName. + newPods := getRunningPodsByNamePrefix(t, env.ConciergeNamespace, kubeCertAgentDeploymentName+"-", "") + requireEventually.Equal(len(newPods), 1) + requireEventually.True(allPodsReady(newPods), "wanted new pod to be ready") + requireEventually.Equal(newlyConfiguredPriorityClassName, newPods[0].Spec.PriorityClassName) + + t.Log("observed that kube cert agent deployment and pod both got new PriorityClassName") + }, + // Wait 5 minutes in case the Concierge was just redeployed, in which case it can take time for + // the controllers to be ready again. This test could theoretically get run as the very first test in the whole suite. + 5*time.Minute, + 250*time.Millisecond, + ) +} diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index eed238f08..ca0881992 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -64,7 +64,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { } topSetupCtx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) supervisorClient := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t) temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret( diff --git a/test/integration/kube_api_discovery_test.go b/test/integration/kube_api_discovery_test.go index 94bc399a3..ded484662 100644 --- a/test/integration/kube_api_discovery_test.go +++ b/test/integration/kube_api_discovery_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -519,7 +519,7 @@ func TestCRDAdditionalPrinterColumns_Parallel(t *testing.T) { env := testlib.IntegrationEnv(t) ctx, cancelFunc := context.WithTimeout(context.Background(), time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) // AdditionalPrinterColumns are not returned by the Kube discovery endpoints, // so "discover" them in the CRD definitions instead. diff --git a/test/integration/supervisor_warnings_test.go b/test/integration/supervisor_warnings_test.go index 2c91487f9..26e975880 100644 --- a/test/integration/supervisor_warnings_test.go +++ b/test/integration/supervisor_warnings_test.go @@ -1,4 +1,4 @@ -// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -40,7 +40,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { env := testlib.IntegrationEnv(t) ctx, cancelFunc := context.WithTimeout(context.Background(), 10*time.Minute) - defer cancelFunc() + t.Cleanup(cancelFunc) // Build pinniped CLI. pinnipedExe := testlib.PinnipedCLIPath(t)