Merge pull request #303 from chiragkyal/istio-clusterID

CM-680: [istio-csr] Enable multi-cluster support with clusterID

Author: openshift-merge-bot[bot]

api/operator/v1alpha1/istiocsr_types.go

@@ -212,6 +212,12 @@ type IstiodTLSConfig struct {
 // ServerConfig is for configuring the server endpoint used by istio
 // for obtaining the certificates.
 type ServerConfig struct {
+	// clusterID is the istio cluster ID to verify incoming CSRs.
+	// +kubebuilder:default:="Kubernetes"
+	// +kubebuilder:validation:Optional
+	// +optional
+	ClusterID string `json:"clusterID,omitempty"`
+
 	// port to serve istio-csr gRPC service.
 	// +kubebuilder:default:=443
 	// +kubebuilder:validation:XValidation:rule="oldSelf == 0 || self == oldSelf",message="port is immutable once set"

bundle/manifests/operator.openshift.io_istiocsrs.yaml

@@ -1221,6 +1221,11 @@ spec:
                       server is for configuring the server endpoint used by istio
                       for obtaining the certificates.
                     properties:
+                      clusterID:
+                        default: Kubernetes
+                        description: clusterID is the istio cluster ID to verify incoming
+                          CSRs.
+                        type: string
                       port:
                         default: 443
                         description: port to serve istio-csr gRPC service.

config/crd/bases/operator.openshift.io_istiocsrs.yaml

@@ -1221,6 +1221,11 @@ spec:
                       server is for configuring the server endpoint used by istio
                       for obtaining the certificates.
                     properties:
+                      clusterID:
+                        default: Kubernetes
+                        description: clusterID is the istio cluster ID to verify incoming
+                          CSRs.
+                        type: string
                       port:
                         default: 443
                         description: port to serve istio-csr gRPC service.

pkg/controller/istiocsr/deployments.go

@@ -139,6 +139,11 @@ func updatePodTemplateLabels(deployment *appsv1.Deployment, resourceLabels map[s
 
 func updateArgList(deployment *appsv1.Deployment, istiocsr *v1alpha1.IstioCSR) {
 	istiocsrConfigs := istiocsr.Spec.IstioCSRConfig
+	// Default clusterID to "Kubernetes" if not provided.
+	clusterID := "Kubernetes"
+	if istiocsrConfigs.Server != nil && istiocsrConfigs.Server.ClusterID != "" {
+		clusterID = istiocsrConfigs.Server.ClusterID
+	}
 	args := []string{
 		fmt.Sprintf("--log-level=%d", istiocsrConfigs.LogLevel),
 		fmt.Sprintf("--log-format=%s", istiocsrConfigs.LogFormat),
@@ -152,7 +157,7 @@ func updateArgList(deployment *appsv1.Deployment, istiocsr *v1alpha1.IstioCSR) {
 		fmt.Sprintf("--serving-certificate-dns-names=cert-manager-istio-csr.%s.svc", istiocsr.GetNamespace()),
 		fmt.Sprintf("--serving-certificate-duration=%.0fm", istiocsrConfigs.IstiodTLSConfig.CertificateDuration.Minutes()),
 		fmt.Sprintf("--trust-domain=%s", istiocsrConfigs.IstiodTLSConfig.TrustDomain),
-		"--cluster-id=Kubernetes",
+		fmt.Sprintf("--cluster-id=%s", clusterID),
 		fmt.Sprintf("--max-client-certificate-duration=%.0fm", istiocsrConfigs.IstiodTLSConfig.MaxCertificateDuration.Minutes()),
 		"--serving-address=0.0.0.0:6443",
 		fmt.Sprintf("--serving-certificate-key-size=%d", istiocsrConfigs.IstiodTLSConfig.PrivateKeySize),

pkg/controller/istiocsr/deployments_test.go

@@ -2,6 +2,8 @@ package istiocsr
 
 import (
 	"context"
+	"fmt"
+	"slices"
 	"testing"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -736,3 +738,81 @@ func TestCreateOrApplyDeployments(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateArgList(t *testing.T) {
+	tests := []struct {
+		name           string
+		updateIstioCSR func(*v1alpha1.IstioCSR)
+		expectedArgs   map[string]string // key is arg name (without --), value is expected value
+	}{
+		{
+			name: "clusterID not provided should default to Kubernetes",
+			updateIstioCSR: func(istiocsr *v1alpha1.IstioCSR) {
+				// Server config is nil, so clusterID should default
+			},
+			expectedArgs: map[string]string{
+				"cluster-id": "Kubernetes",
+			},
+		},
+		{
+			name: "clusterID empty string should default to Kubernetes",
+			updateIstioCSR: func(istiocsr *v1alpha1.IstioCSR) {
+				istiocsr.Spec.IstioCSRConfig.Server = &v1alpha1.ServerConfig{
+					ClusterID: "",
+				}
+			},
+			expectedArgs: map[string]string{
+				"cluster-id": "Kubernetes",
+			},
+		},
+		{
+			name: "clusterID provided should use custom value",
+			updateIstioCSR: func(istiocsr *v1alpha1.IstioCSR) {
+				istiocsr.Spec.IstioCSRConfig.Server = &v1alpha1.ServerConfig{
+					ClusterID: "cluster-123_dev.local",
+				}
+			},
+			expectedArgs: map[string]string{
+				"cluster-id": "cluster-123_dev.local",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			deployment := testDeployment()
+			istiocsr := testIstioCSR()
+			if tt.updateIstioCSR != nil {
+				tt.updateIstioCSR(istiocsr)
+			}
+
+			updateArgList(deployment, istiocsr)
+
+			// Find the istio-csr container and check its arguments
+			var containerArgs []string
+			for _, container := range deployment.Spec.Template.Spec.Containers {
+				if container.Name == istiocsrContainerName {
+					containerArgs = container.Args
+					break
+				}
+			}
+
+			if len(containerArgs) == 0 {
+				t.Fatalf("Expected container args to be set, but got empty args")
+			}
+
+			// Verify each expected argument
+			for argName, expectedValue := range tt.expectedArgs {
+				expectedArg := fmt.Sprintf("--%s=%s", argName, expectedValue)
+				if !containsArg(containerArgs, expectedArg) {
+					t.Errorf("Expected to find argument %q in container args, but it was not found. Args: %v", expectedArg, containerArgs)
+				}
+			}
+		})
+	}
+}
+
+// containsArg checks if the given argument is present in the args list
+func containsArg(args []string, targetArg string) bool {
+	return slices.Contains(args, targetArg)
+}

pkg/operator/applyconfigurations/operator/v1alpha1/serverconfig.go

@@ -25,6 +25,8 @@ type CertificateConfig struct {
 type IstioCSRGRPCurlJobConfig struct {
 	CertificateSigningRequest string
 	IstioCSRStatus            v1alpha1.IstioCSRStatus
+	ClusterID                 string
+	JobName                   string
 }
 
 // replaceWithTemplate puts field values from a template struct