Merge pull request #2690 from vmware/configurable_kube_cert_agent_strategy_type

allow the kube cert agent deployment's strategy type to be configured

Author: cfryanr

go.mod

@@ -34,7 +34,7 @@ require (
 	github.com/gofrs/flock v0.13.0
 	github.com/google/cel-go v0.26.1
 	github.com/google/go-cmp v0.7.0
-	github.com/google/go-github/v74 v74.0.0
+	github.com/google/go-github/v75 v75.0.0
 	github.com/google/gofuzz v1.2.0
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/securecookie v1.1.2
@@ -49,7 +49,7 @@ require (
 	github.com/spf13/cobra v1.10.1
 	github.com/spf13/pflag v1.0.10
 	github.com/stretchr/testify v1.11.1
-	github.com/tdewolff/minify/v2 v2.24.4
+	github.com/tdewolff/minify/v2 v2.24.5
 	go.uber.org/mock v0.6.0
 	go.uber.org/zap v1.27.0
 	golang.org/x/crypto v0.43.0
@@ -147,7 +147,7 @@ require (
 	github.com/spf13/viper v1.16.0 // indirect
 	github.com/stoewer/go-strcase v1.3.0 // indirect
 	github.com/subosito/gotenv v1.4.2 // indirect
-	github.com/tdewolff/parse/v2 v2.8.4 // indirect
+	github.com/tdewolff/parse/v2 v2.8.5-0.20251020133559-0efcf90bef1a // indirect
 	github.com/x448/float16 v0.8.4 // indirect
 	go.etcd.io/etcd/api/v3 v3.5.21 // indirect
 	go.etcd.io/etcd/client/pkg/v3 v3.5.21 // indirect

go.sum

@@ -247,8 +247,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
 github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
 github.com/google/go-github/v73 v73.0.0 h1:aR+Utnh+Y4mMkS+2qLQwcQ/cF9mOTpdwnzlaw//rG24=
 github.com/google/go-github/v73 v73.0.0/go.mod h1:fa6w8+/V+edSU0muqdhCVY7Beh1M8F1IlQPZIANKIYw=
-github.com/google/go-github/v74 v74.0.0 h1:yZcddTUn8DPbj11GxnMrNiAnXH14gNs559AsUpNpPgM=
-github.com/google/go-github/v74 v74.0.0/go.mod h1:ubn/YdyftV80VPSI26nSJvaEsTOnsjrxG3o9kJhcyak=
+github.com/google/go-github/v75 v75.0.0 h1:k7q8Bvg+W5KxRl9Tjq16a9XEgVY1pwuiG5sIL7435Ic=
+github.com/google/go-github/v75 v75.0.0/go.mod h1:H3LUJEA1TCrzuUqtdAQniBNwuKiQIqdGKgBo1/M/uqI=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
 github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
@@ -564,10 +564,10 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8=
 github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0=
-github.com/tdewolff/minify/v2 v2.24.4 h1:pQyr6eWDa+RXtAoZg+6wurh0jB9ojqw/qc5LlU7/z6c=
-github.com/tdewolff/minify/v2 v2.24.4/go.mod h1:iD9Qn7/brhKY9d0KLKMkZrqS8/bqxSxRKruBi7V6m+w=
-github.com/tdewolff/parse/v2 v2.8.4 h1:A6slgBLGGDPBMGA28KQZfHpaKffuNvhOe7zSag+x/rw=
-github.com/tdewolff/parse/v2 v2.8.4/go.mod h1:Hwlni2tiVNKyzR1o6nUs4FOF07URA+JLBLd6dlIXYqo=
+github.com/tdewolff/minify/v2 v2.24.5 h1:ytxthX3xSxrK3Xx5B38flg5moCKs/dB8VwiD/RzJViU=
+github.com/tdewolff/minify/v2 v2.24.5/go.mod h1:q09KtNnVai7TyEzGEZeWPAnK+c8Z+NI8prCXZW652bo=
+github.com/tdewolff/parse/v2 v2.8.5-0.20251020133559-0efcf90bef1a h1:Rmq+utdraciok/97XHRweYdsAo/M4LOswpCboo3yvN4=
+github.com/tdewolff/parse/v2 v2.8.5-0.20251020133559-0efcf90bef1a/go.mod h1:Hwlni2tiVNKyzR1o6nUs4FOF07URA+JLBLd6dlIXYqo=
 github.com/tdewolff/test v1.0.11 h1:FdLbwQVHxqG16SlkGveC0JVyrJN62COWTRyUFzfbtBE=
 github.com/tdewolff/test v1.0.11/go.mod h1:XPuWBzvdUzhCuxWO1ojpXsyzsA5bFoS3tO/Q3kFuTG8=
 github.com/tidwall/gjson v1.14.3 h1:9jvXn7olKEHU1S9vwoMGliaT8jq1vJ7IH/n9zD9Dnlw=

internal/config/concierge/config.go

@@ -11,6 +11,8 @@ import (
 	"os"
 	"strings"
 
+	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/yaml"
@@ -200,6 +202,12 @@ func validateKubeCertAgent(agentConfig *KubeCertAgentSpec) error {
 		return constable.Error(fmt.Sprintf("runAsGroup must be 0 or greater (instead of %d)", *agentConfig.RunAsGroup))
 	}
 
+	allowedStrategyTypes := sets.New(appsv1.RecreateDeploymentStrategyType, appsv1.RollingUpdateDeploymentStrategyType)
+	if agentConfig.DeploymentStrategyType != nil && !allowedStrategyTypes.Has(*agentConfig.DeploymentStrategyType) {
+		return constable.Error(fmt.Sprintf("deploymentStrategyType must be one of %s (instead of %s)",
+			sets.List(allowedStrategyTypes), *agentConfig.DeploymentStrategyType))
+	}
+
 	if len(agentConfig.PriorityClassName) == 0 {
 		// Optional, so empty is valid.
 		return nil

internal/config/concierge/config_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/utils/ptr"
 
 	"go.pinniped.dev/internal/here"
@@ -71,6 +72,7 @@ func TestFromPath(t *testing.T) {
 				  priorityClassName: %s
 				  runAsUser: 1
 				  runAsGroup: 2
+				  deploymentStrategyType: Recreate
 				log:
 				  level: debug
 				tls:
@@ -119,12 +121,13 @@ 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"},
-					PriorityClassName: stringOfLength253,
-					RunAsUser:         ptr.To(int64(1)),
-					RunAsGroup:        ptr.To(int64(2)),
+					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,
+					RunAsUser:              ptr.To(int64(1)),
+					RunAsGroup:             ptr.To(int64(2)),
+					DeploymentStrategyType: ptr.To(appsv1.RecreateDeploymentStrategyType),
 				},
 				Log: plog.LogSpec{
 					Level: plog.LevelDebug,
@@ -184,6 +187,9 @@ func TestFromPath(t *testing.T) {
 				  image: kube-cert-agent-image
 				  imagePullSecrets: [kube-cert-agent-image-pull-secret]
 				  priorityClassName: kube-cert-agent-priority-class-name
+				  runAsUser: 1
+				  runAsGroup: 2
+				  deploymentStrategyType: RollingUpdate
 				log:
 				  level: all
 				  format: json
@@ -227,10 +233,13 @@ 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"},
-					PriorityClassName: "kube-cert-agent-priority-class-name",
+					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",
+					RunAsUser:              ptr.To(int64(1)),
+					RunAsGroup:             ptr.To(int64(2)),
+					DeploymentStrategyType: ptr.To(appsv1.RollingUpdateDeploymentStrategyType),
 				},
 				Log: plog.LogSpec{
 					Level:  plog.LevelAll,
@@ -801,6 +810,27 @@ func TestFromPath(t *testing.T) {
 			`),
 			wantError: `validate kubeCertAgent: runAsGroup must be 0 or greater (instead of -1)`,
 		},
+		{
+			name: "invalid deploymentStrategyType",
+			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
+				  agentServiceAccount: agentServiceAccount-value
+				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
+				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
+				kubeCertAgent:
+				  deploymentStrategyType: thisIsInvalid
+			`),
+			wantError: `validate kubeCertAgent: deploymentStrategyType must be one of [Recreate RollingUpdate] (instead of thisIsInvalid)`,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

internal/config/concierge/types.go

@@ -3,7 +3,11 @@
 
 package concierge
 
-import "go.pinniped.dev/internal/plog"
+import (
+	appsv1 "k8s.io/api/apps/v1"
+
+	"go.pinniped.dev/internal/plog"
+)
 
 const (
 	Enabled  = "enabled"
@@ -120,4 +124,9 @@ type KubeCertAgentSpec struct {
 	// The GID to run the entrypoint of the kube-cert-agent container.
 	// Defaults to 0 (root). No validation is performed on this value.
 	RunAsGroup *int64 `json:"runAsGroup"`
+
+	// DeploymentStrategyType will be set as the agent Deployment's deployment strategy type.
+	// When nil, the Deployment will not specify any deployment strategy type, and will therefore have its
+	// deployment strategy type set by Kubernetes default behavior (currently RollingUpdate).
+	DeploymentStrategyType *appsv1.DeploymentStrategyType `json:"deploymentStrategyType"`
 }

internal/controller/kubecertagent/kubecertagent.go

@@ -105,11 +105,16 @@ type AgentConfig struct {
 	// PriorityClassName optionally sets the PriorityClassName for the agent's pod.
 	PriorityClassName string
 
-	// RunAsUser is the UID to run the entrypoint of the container process
+	// RunAsUser is the UID to run the entrypoint of the container process.
 	RunAsUser *int64
 
-	// RunAsGroup is the GID to run the entrypoint of the container process
+	// RunAsGroup is the GID to run the entrypoint of the container process.
 	RunAsGroup *int64
+
+	// DeploymentStrategyType will be set as the agent Deployment's deployment strategy type.
+	// When nil, the Deployment will not specify any deployment strategy type, and will therefore have its
+	// deployment strategy type set by Kubernetes default behavior (currently RollingUpdate).
+	DeploymentStrategyType *appsv1.DeploymentStrategyType
 }
 
 // Only select using the unique label which will not match the pods of any other Deployment.
@@ -440,12 +445,14 @@ func (c *agentController) createOrUpdateDeployment(ctx context.Context, newestCo
 	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
+	// The user might want to set deploymentStrategyType back to the default value. DeepDerivative() won't detect this case below.
+	desireDeploymentStrategyTypeUpdate := updatedDeployment.Spec.Strategy.Type != existingDeployment.Spec.Strategy.Type
 
 	// 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 && !desirePriorityClassNameUpdate {
+		if !desireSelectorUpdate && !desireTemplateLabelsUpdate && !desirePriorityClassNameUpdate && !desireDeploymentStrategyTypeUpdate {
 			return nil // already equal enough, so skip update
 		}
 	}
@@ -614,6 +621,14 @@ func (c *agentController) getPodSecurityContext() *corev1.PodSecurityContext {
 	return podSecurityContext
 }
 
+func (c *agentController) getDeploymentStrategy() appsv1.DeploymentStrategy {
+	s := appsv1.DeploymentStrategy{}
+	if c.cfg.DeploymentStrategyType != nil {
+		s.Type = *c.cfg.DeploymentStrategyType
+	}
+	return s
+}
+
 func (c *agentController) newAgentDeployment(controllerManagerPod *corev1.Pod) *appsv1.Deployment {
 	var volumeMounts []corev1.VolumeMount
 	if len(controllerManagerPod.Spec.Containers) > 0 {
@@ -699,6 +714,9 @@ func (c *agentController) newAgentDeployment(controllerManagerPod *corev1.Pod) *
 
 			// Setting MinReadySeconds prevents the agent pods from being churned too quickly by the deployments controller.
 			MinReadySeconds: 10,
+
+			// Allow the user to optionally configure the deployment strategy type.
+			Strategy: c.getDeploymentStrategy(),
 		},
 	}
 }