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.