Add api server extra args map to cluster spec (#7755)

* Add api server extra args map to cluster spec * Add unit tests for apiServerExtraArgs * Add e2e tests for apiServerExtraArgs create and upgrade cluster * Add support for both PodIAMConfig and APIServerExtraArgs service-account-issuer * Allow users to skip api server extra args preflight validation * Add feature flag to configure api server extra args * Revert "Allow users to skip api server extra args preflight validation" This reverts commit e4b9ef4. * Add validation for oidc flags configured in apiServerExtraArgs * Update error messages
aws · Mar 20, 2024 · 6c07643 · 6c07643
1 parent 6d13da7
commit 6c07643
Show file tree

Hide file tree

Showing 23 changed files with 617 additions and 11 deletions.
diff --git a/config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml b/config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml
@@ -151,6 +151,12 @@ spec:
                 type: object
               controlPlaneConfiguration:
                 properties:
+                  apiServerExtraArgs:
+                    additionalProperties:
+                      type: string
+                    description: APIServerExtraArgs defines the flags to configure
+                      for the API server.
+                    type: object
                   certSans:
                     description: CertSANs is a slice of domain names or IPs to be
                       added as Subject Name Alternatives of the Kube API Servers Certificate.

diff --git a/config/manifest/eksa-components.yaml b/config/manifest/eksa-components.yaml
@@ -3854,6 +3854,12 @@ spec:
                 type: object
               controlPlaneConfiguration:
                 properties:
+                  apiServerExtraArgs:
+                    additionalProperties:
+                      type: string
+                    description: APIServerExtraArgs defines the flags to configure
+                      for the API server.
+                    type: object
                   certSans:
                     description: CertSANs is a slice of domain names or IPs to be
                       added as Subject Name Alternatives of the Kube API Servers Certificate.

diff --git a/internal/pkg/api/cluster.go b/internal/pkg/api/cluster.go
@@ -126,6 +126,26 @@ func WithControlPlaneLabel(key string, val string) ClusterFiller {
 	}
 }
 
+// WithControlPlaneAPIServerExtraArgs adds the APIServerExtraArgs to the cluster spec.
+func WithControlPlaneAPIServerExtraArgs() ClusterFiller {
+	return func(c *anywherev1.Cluster) {
+		if c.Spec.ControlPlaneConfiguration.APIServerExtraArgs == nil {
+			c.Spec.ControlPlaneConfiguration.APIServerExtraArgs = map[string]string{}
+		}
+		issuerURL := "https://" + c.Spec.ControlPlaneConfiguration.Endpoint.Host
+		c.Spec.ControlPlaneConfiguration.APIServerExtraArgs["service-account-jwks-uri"] = issuerURL + "/openid/v1/jwks"
+	}
+}
+
+// RemoveAllAPIServerExtraArgs removes all the API server flags from the cluster spec.
+func RemoveAllAPIServerExtraArgs() ClusterFiller {
+	return func(c *anywherev1.Cluster) {
+		for k := range c.Spec.ControlPlaneConfiguration.APIServerExtraArgs {
+			delete(c.Spec.ControlPlaneConfiguration.APIServerExtraArgs, k)
+		}
+	}
+}
+
 // WithPodCidr sets an explicit pod CIDR, overriding the provider's default.
 func WithPodCidr(podCidr string) ClusterFiller {
 	return func(c *anywherev1.Cluster) {

diff --git a/internal/pkg/api/cluster_test.go b/internal/pkg/api/cluster_test.go
@@ -231,3 +231,100 @@ func TestWithPodCidr(t *testing.T) {
 		g.Expect(cluster.Spec.ClusterNetwork.Pods.CidrBlocks).To(Equal([]string{"10.0.0.0/16", "172.16.42.0/20"}))
 	})
 }
+
+func TestWithControlPlaneAPIServerExtraArgs(t *testing.T) {
+	tests := []struct {
+		name    string
+		cluster *anywherev1.Cluster
+		want    anywherev1.ControlPlaneConfiguration
+	}{
+		{
+			name: "no control plane api server extra args",
+			cluster: &anywherev1.Cluster{
+				Spec: anywherev1.ClusterSpec{
+					ControlPlaneConfiguration: anywherev1.ControlPlaneConfiguration{
+						Endpoint: &anywherev1.Endpoint{
+							Host: "10.20.30.40",
+						},
+					},
+				},
+			},
+			want: anywherev1.ControlPlaneConfiguration{
+				APIServerExtraArgs: map[string]string{
+					"service-account-jwks-uri": "https://10.20.30.40/openid/v1/jwks",
+				},
+				Endpoint: &anywherev1.Endpoint{
+					Host: "10.20.30.40",
+				},
+			},
+		},
+		{
+			name: "with control plane api server extra args",
+			cluster: &anywherev1.Cluster{
+				Spec: anywherev1.ClusterSpec{
+					ControlPlaneConfiguration: anywherev1.ControlPlaneConfiguration{
+						APIServerExtraArgs: map[string]string{
+							"service-account-jwks-uri": "https://40.50.60.70/openid/v1/jwks",
+						},
+						Endpoint: &anywherev1.Endpoint{
+							Host: "10.20.30.40",
+						},
+					},
+				},
+			},
+			want: anywherev1.ControlPlaneConfiguration{
+				APIServerExtraArgs: map[string]string{
+					"service-account-jwks-uri": "https://10.20.30.40/openid/v1/jwks",
+				},
+				Endpoint: &anywherev1.Endpoint{
+					Host: "10.20.30.40",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(
+			tt.name,
+			func(t *testing.T) {
+				api.WithControlPlaneAPIServerExtraArgs()(tt.cluster)
+				g := NewWithT(t)
+				g.Expect(tt.cluster.Spec.ControlPlaneConfiguration).To(Equal(tt.want))
+			},
+		)
+	}
+}
+
+func TestRemoveAllAPIServerExtraArgs(t *testing.T) {
+	tests := []struct {
+		name    string
+		cluster *anywherev1.Cluster
+		want    anywherev1.ControlPlaneConfiguration
+	}{
+		{
+			name: "with control plane api server extra args",
+			cluster: &anywherev1.Cluster{
+				Spec: anywherev1.ClusterSpec{
+					ControlPlaneConfiguration: anywherev1.ControlPlaneConfiguration{
+						APIServerExtraArgs: map[string]string{
+							"service-account-issuer":   "test-service-account-issuer-url",
+							"service-account-jwks-uri": "test-service-account-jwks-uri",
+						},
+					},
+				},
+			},
+			want: anywherev1.ControlPlaneConfiguration{
+				APIServerExtraArgs: map[string]string{},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(
+			tt.name,
+			func(t *testing.T) {
+				api.RemoveAllAPIServerExtraArgs()(tt.cluster)
+				g := NewWithT(t)
+				g.Expect(tt.cluster.Spec.ControlPlaneConfiguration).To(Equal(tt.want))
+			},
+		)
+	}
+}
diff --git a/pkg/api/v1alpha1/cluster.go b/pkg/api/v1alpha1/cluster.go
@@ -190,6 +190,8 @@ var clusterConfigValidations = []func(*Cluster) error{
 	validatePackageControllerConfiguration,
 	validateEksaVersion,
 	validateControlPlaneCertSANs,
+	validateControlPlaneAPIServerExtraArgs,
+	validateControlPlaneAPIServerOIDCExtraArgs,
 }
 
 // GetClusterConfig parses a Cluster object from a multiobject yaml file in disk
@@ -494,6 +496,37 @@ func validateControlPlaneCertSANs(cfg *Cluster) error {
 	return nil
 }
 
+func validateControlPlaneAPIServerExtraArgs(clusterConfig *Cluster) error {
+	if clusterConfig.Spec.ControlPlaneConfiguration.APIServerExtraArgs != nil && !features.IsActive(features.APIServerExtraArgsEnabled()) {
+		return fmt.Errorf("configuring APIServerExtraArgs is not supported. Set env var %v to enable", features.APIServerExtraArgsEnabledEnvVar)
+	}
+	return nil
+}
+
+func validateControlPlaneAPIServerOIDCExtraArgs(clusterConfig *Cluster) error {
+	oidcFlags := []string{
+		"oidc-issuer-url",
+		"oidc-client-id",
+		"oidc-groups-claim",
+		"oidc-groups-prefix",
+		"oidc-required-claim",
+		"oidc-username-claim",
+		"oidc-username-prefix",
+	}
+	if clusterConfig.Spec.IdentityProviderRefs != nil {
+		for _, ref := range clusterConfig.Spec.IdentityProviderRefs {
+			if ref.Kind == OIDCConfigKind {
+				for _, flag := range oidcFlags {
+					if _, has := clusterConfig.Spec.ControlPlaneConfiguration.APIServerExtraArgs[flag]; has {
+						return fmt.Errorf("the following flags cannot be configured if OIDCConfig is configured for cluster.spec.identityProviderRefs: %v. Remove from apiServerExtraArgs", oidcFlags)
+					}
+				}
+			}
+		}
+	}
+	return nil
+}
+
 func validateWorkerNodeGroups(clusterConfig *Cluster) error {
 	workerNodeGroupConfigs := clusterConfig.Spec.WorkerNodeGroupConfigurations
 	if len(workerNodeGroupConfigs) <= 0 {

diff --git a/pkg/api/v1alpha1/cluster_types.go b/pkg/api/v1alpha1/cluster_types.go
@@ -307,6 +307,8 @@ type ControlPlaneConfiguration struct {
 	CertSANs []string `json:"certSans,omitempty"`
 	// MachineHealthCheck is a control-plane level override for the timeouts and maxUnhealthy specified in the top-level MHC configuration. If not configured, the defaults in the top-level MHC configuration are used.
 	MachineHealthCheck *MachineHealthCheck `json:"machineHealthCheck,omitempty"`
+	// APIServerExtraArgs defines the flags to configure for the API server.
+	APIServerExtraArgs map[string]string `json:"apiServerExtraArgs,omitempty"`
 }
 
 // MachineHealthCheck allows to configure timeouts for machine health checks. Machine Health Checks are responsible for remediating unhealthy Machines.
@@ -363,7 +365,7 @@ func (n *ControlPlaneConfiguration) Equal(o *ControlPlaneConfiguration) bool {
 	}
 	return n.Count == o.Count && n.MachineGroupRef.Equal(o.MachineGroupRef) &&
 		TaintsSliceEqual(n.Taints, o.Taints) && MapEqual(n.Labels, o.Labels) &&
-		SliceEqual(n.CertSANs, o.CertSANs)
+		SliceEqual(n.CertSANs, o.CertSANs) && MapEqual(n.APIServerExtraArgs, o.APIServerExtraArgs)
 }
 
 type Endpoint struct {

diff --git a/pkg/api/v1alpha1/cluster_types_test.go b/pkg/api/v1alpha1/cluster_types_test.go
@@ -1460,6 +1460,10 @@ func TestControlPlaneConfigurationEqual(t *testing.T) {
 	taints1DiffOrder := []corev1.Taint{taint2, taint1}
 	taints2 := []corev1.Taint{taint1}
 
+	var emptyAPIServerExtraArgs map[string]string
+	apiServerExtraArgs1 := map[string]string{"key1": "value1"}
+	apiServerExtraArgs2 := map[string]string{"key2": "value2"}
+
 	testCases := []struct {
 		testName                           string
 		cluster1CPConfig, cluster2CPConfig *v1alpha1.ControlPlaneConfiguration
@@ -1595,6 +1599,52 @@ func TestControlPlaneConfigurationEqual(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			testName: "both api server extra args equal",
+			cluster1CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: apiServerExtraArgs1,
+			},
+			cluster2CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: apiServerExtraArgs1,
+			},
+			want: true,
+		},
+		{
+			testName: "different api server extra args",
+			cluster1CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: apiServerExtraArgs1,
+			},
+			cluster2CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: apiServerExtraArgs2,
+			},
+			want: false,
+		},
+		{
+			testName: "one api server extra args not present",
+			cluster1CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: apiServerExtraArgs1,
+			},
+			cluster2CPConfig: &v1alpha1.ControlPlaneConfiguration{},
+			want:             false,
+		},
+		{
+			testName: "one api server extra args not present and other empty",
+			cluster1CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: emptyAPIServerExtraArgs,
+			},
+			cluster2CPConfig: &v1alpha1.ControlPlaneConfiguration{},
+			want:             true,
+		},
+		{
+			testName: "both api server extra args empty",
+			cluster1CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: emptyAPIServerExtraArgs,
+			},
+			cluster2CPConfig: &v1alpha1.ControlPlaneConfiguration{
+				APIServerExtraArgs: emptyAPIServerExtraArgs,
+			},
+			want: true,
+		},
 	}
 	for _, tt := range testCases {
 		t.Run(tt.testName, func(t *testing.T) {

diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/clusterapi/extraargs.go b/pkg/clusterapi/extraargs.go
@@ -53,6 +53,15 @@ func EtcdEncryptionExtraArgs(config *[]v1alpha1.EtcdEncryption) ExtraArgs {
 	return args
 }
 
+// APIServerExtraArgs takes a map of API Server extra args and returns the relevant API server extra args if it's not nil or empty.
+func APIServerExtraArgs(apiServerExtraArgs map[string]string) ExtraArgs {
+	args := ExtraArgs{}
+	for k, v := range apiServerExtraArgs {
+		args.AddIfNotEmpty(k, v)
+	}
+	return args
+}
+
 func PodIAMAuthExtraArgs(podIAMConfig *v1alpha1.PodIAMConfig) ExtraArgs {
 	if podIAMConfig == nil {
 		return nil
@@ -136,6 +145,17 @@ func (e ExtraArgs) Append(args ExtraArgs) ExtraArgs {
 	return e
 }
 
+// SetPodIAMAuthExtraArgs sets the api server extra args for the podIAMConfig.
+func SetPodIAMAuthExtraArgs(podIAMConfig *v1alpha1.PodIAMConfig, apiServerExtraArgs map[string]string) {
+	if podIAMFlags := PodIAMAuthExtraArgs(podIAMConfig); podIAMFlags != nil {
+		if v, has := apiServerExtraArgs["service-account-issuer"]; has {
+			apiServerExtraArgs["service-account-issuer"] = strings.Join([]string{v, podIAMFlags["service-account-issuer"]}, ",")
+		} else {
+			apiServerExtraArgs["service-account-issuer"] = podIAMFlags["service-account-issuer"]
+		}
+	}
+}
+
 func (e ExtraArgs) ToPartialYaml() templater.PartialYaml {
 	p := templater.PartialYaml{}
 	for k, v := range e {