From d6b0563e2da0a08cf588e887be356121d73bbef1 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Wed, 28 Feb 2024 19:58:21 -0800 Subject: [PATCH 1/9] Add api server extra args map to cluster spec --- .../crd/bases/anywhere.eks.amazonaws.com_clusters.yaml | 6 ++++++ config/manifest/eksa-components.yaml | 6 ++++++ pkg/api/v1alpha1/cluster_types.go | 4 +++- pkg/api/v1alpha1/zz_generated.deepcopy.go | 7 +++++++ pkg/clusterapi/extraargs.go | 9 +++++++++ pkg/providers/cloudstack/template.go | 1 + pkg/providers/docker/docker.go | 1 + pkg/providers/nutanix/template.go | 3 ++- pkg/providers/snow/apibuilder.go | 5 +++++ pkg/providers/tinkerbell/template.go | 3 ++- pkg/providers/vsphere/template.go | 1 + 11 files changed, 43 insertions(+), 3 deletions(-) diff --git a/config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml b/config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml index bb23ca0f7523..0f323ebf4a00 100644 --- 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 index 21679cf2c80f..a113b8cabaff 100644 --- 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/pkg/api/v1alpha1/cluster_types.go b/pkg/api/v1alpha1/cluster_types.go index 88936b4317ce..4173209b3cfe 100644 --- 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/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index 6104d7cacd9b..e77f5f744048 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha1/zz_generated.deepcopy.go @@ -878,6 +878,13 @@ func (in *ControlPlaneConfiguration) DeepCopyInto(out *ControlPlaneConfiguration *out = new(MachineHealthCheck) (*in).DeepCopyInto(*out) } + if in.APIServerExtraArgs != nil { + in, out := &in.APIServerExtraArgs, &out.APIServerExtraArgs + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControlPlaneConfiguration. diff --git a/pkg/clusterapi/extraargs.go b/pkg/clusterapi/extraargs.go index 027a79a335c7..2ae614dbac27 100644 --- 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 diff --git a/pkg/providers/cloudstack/template.go b/pkg/providers/cloudstack/template.go index a5ab2ffb0cf6..d6ad013bf239 100644 --- a/pkg/providers/cloudstack/template.go +++ b/pkg/providers/cloudstack/template.go @@ -123,6 +123,7 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). Append(sharedExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). diff --git a/pkg/providers/docker/docker.go b/pkg/providers/docker/docker.go index 80f8837b9186..52042b03f9fc 100644 --- a/pkg/providers/docker/docker.go +++ b/pkg/providers/docker/docker.go @@ -295,6 +295,7 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). Append(sharedExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.NodeCIDRMaskExtraArgs(&clusterSpec.Cluster.Spec.ClusterNetwork)) diff --git a/pkg/providers/nutanix/template.go b/pkg/providers/nutanix/template.go index 1294c8a3bad1..10564ff78d84 100644 --- a/pkg/providers/nutanix/template.go +++ b/pkg/providers/nutanix/template.go @@ -162,7 +162,8 @@ func buildTemplateMapCP( apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). - Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)) + Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)) kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)). Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration)) diff --git a/pkg/providers/snow/apibuilder.go b/pkg/providers/snow/apibuilder.go index 5e4f294f3f02..1794770f81cc 100644 --- a/pkg/providers/snow/apibuilder.go +++ b/pkg/providers/snow/apibuilder.go @@ -48,6 +48,11 @@ func KubeadmControlPlane(log logr.Logger, clusterSpec *cluster.Spec, snowMachine return nil, fmt.Errorf("setting kube-vip: %v", err) } + apiServerExtraArgs := clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs + for k, v := range apiServerExtraArgs { + kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs[k] = v + } + initConfigKubeletExtraArg := kcp.Spec.KubeadmConfigSpec.InitConfiguration.NodeRegistration.KubeletExtraArgs initConfigKubeletExtraArg["provider-id"] = "aws-snow:////'{{ ds.meta_data.instance_id }}'" diff --git a/pkg/providers/tinkerbell/template.go b/pkg/providers/tinkerbell/template.go index 5ece85852b20..645c22eca298 100644 --- a/pkg/providers/tinkerbell/template.go +++ b/pkg/providers/tinkerbell/template.go @@ -399,7 +399,8 @@ func buildTemplateMapCP( apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)) + Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)) kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)). diff --git a/pkg/providers/vsphere/template.go b/pkg/providers/vsphere/template.go index 9cd200392057..903ff4020f9b 100644 --- a/pkg/providers/vsphere/template.go +++ b/pkg/providers/vsphere/template.go @@ -145,6 +145,7 @@ func buildTemplateMapCP( Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). Append(sharedExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.NodeCIDRMaskExtraArgs(&clusterSpec.Cluster.Spec.ClusterNetwork)) From 9d31fb643bd8e117644ccda4af86b3d4e26caee5 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Sat, 2 Mar 2024 14:30:25 -0800 Subject: [PATCH 2/9] Add unit tests for apiServerExtraArgs --- internal/pkg/api/cluster_test.go | 97 +++++++++++++++++++ pkg/api/v1alpha1/cluster_types_test.go | 50 ++++++++++ pkg/clusterapi/extraargs_test.go | 78 +++++++++++++++ pkg/clusterapi/identity_test.go | 127 +++++++++++++++++++++++++ 4 files changed, 352 insertions(+) diff --git a/internal/pkg/api/cluster_test.go b/internal/pkg/api/cluster_test.go index 075c21846f85..c057b93726b1 100644 --- 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_types_test.go b/pkg/api/v1alpha1/cluster_types_test.go index 934a21927ffc..036cd117223a 100644 --- 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/clusterapi/extraargs_test.go b/pkg/clusterapi/extraargs_test.go index b5769134867b..295d46ed4ce7 100644 --- a/pkg/clusterapi/extraargs_test.go +++ b/pkg/clusterapi/extraargs_test.go @@ -179,6 +179,39 @@ func TestExtraArgsToPartialYaml(t *testing.T) { } } +func TestAPIServerExtraArgs(t *testing.T) { + tests := []struct { + testName string + apiServerExtraArgs map[string]string + want clusterapi.ExtraArgs + }{ + { + testName: "no args", + apiServerExtraArgs: map[string]string{}, + want: clusterapi.ExtraArgs{}, + }, + { + testName: "with args", + apiServerExtraArgs: map[string]string{ + "service-account-issuer": "https://my-custom-issuer-url", + "service-account-jwks-uri": "http://my-custom-jwks-uri/openid/v1/jwks", + }, + want: clusterapi.ExtraArgs{ + "service-account-issuer": "https://my-custom-issuer-url", + "service-account-jwks-uri": "http://my-custom-jwks-uri/openid/v1/jwks", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + if got := clusterapi.APIServerExtraArgs(tt.apiServerExtraArgs); !reflect.DeepEqual(got, tt.want) { + t.Errorf("APIServerExtraArgs() = %v, want %v", got, tt.want) + } + }) + } +} + func TestAwsIamAuthExtraArgs(t *testing.T) { tests := []struct { testName string @@ -310,6 +343,51 @@ func TestSecureEtcdTlsCipherSuitesExtraArgs(t *testing.T) { } } +func TestSetPodIAMAuthExtraArgs(t *testing.T) { + tests := []struct { + testName string + podIAMConfig *v1alpha1.PodIAMConfig + apiServerExtraArgs map[string]string + want map[string]string + }{ + { + testName: "with pod iam config", + podIAMConfig: &v1alpha1.PodIAMConfig{ + ServiceAccountIssuer: "https://pod-iam-config", + }, + apiServerExtraArgs: map[string]string{ + "service-account-issuer": "https://api-server-extra-args", + "service-account-jwks-uri": "https://api-server-extra-args/openid/v1/jwks", + }, + want: map[string]string{ + "service-account-issuer": "https://api-server-extra-args,https://pod-iam-config", + "service-account-jwks-uri": "https://api-server-extra-args/openid/v1/jwks", + }, + }, + { + testName: "with api server extra args and pod iam config", + podIAMConfig: &v1alpha1.PodIAMConfig{ + ServiceAccountIssuer: "https://pod-iam-config", + }, + apiServerExtraArgs: map[string]string{ + "service-account-jwks-uri": "https://api-server-extra-args/openid/v1/jwks", + }, + want: map[string]string{ + "service-account-issuer": "https://pod-iam-config", + "service-account-jwks-uri": "https://api-server-extra-args/openid/v1/jwks", + }, + }, + } + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + clusterapi.SetPodIAMAuthExtraArgs(tt.podIAMConfig, tt.apiServerExtraArgs) + if !reflect.DeepEqual(tt.apiServerExtraArgs, tt.want) { + t.Errorf("SetPodIAMAuthExtraArgs() = %v, want %v", tt.apiServerExtraArgs, tt.want) + } + }) + } +} + func TestCgroupDriverCgroupfsExtraArgs(t *testing.T) { tests := []struct { testName string diff --git a/pkg/clusterapi/identity_test.go b/pkg/clusterapi/identity_test.go index 95455c5a4ac3..6f1fc312329f 100644 --- a/pkg/clusterapi/identity_test.go +++ b/pkg/clusterapi/identity_test.go @@ -13,6 +13,133 @@ import ( "github.com/aws/eks-anywhere/pkg/clusterapi" ) +func TestConfigureAPIServerExtraArgsInKubeadmControlPlane(t *testing.T) { + replicas := int32(3) + tests := []struct { + name string + apiServerExtraArgs map[string]string + want *controlplanev1.KubeadmControlPlane + }{ + { + name: "no api server extra args", + apiServerExtraArgs: nil, + want: wantKubeadmControlPlane(), + }, + { + name: "with api server extra args", + apiServerExtraArgs: map[string]string{ + "service-account-issuer": "https://test", + "service-account-jwks-uri": "https://test/openid/v1/jwks", + }, + want: &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "KubeadmControlPlane", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "eksa-system", + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + InfrastructureRef: v1.ObjectReference{ + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "ProviderMachineTemplate", + Name: "provider-template", + }, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ImageRepository: "public.ecr.aws/eks-distro/kubernetes", + DNS: bootstrapv1.DNS{ + ImageMeta: bootstrapv1.ImageMeta{ + ImageRepository: "public.ecr.aws/eks-distro/coredns", + ImageTag: "v1.8.4-eks-1-21-9", + }, + }, + Etcd: bootstrapv1.Etcd{ + Local: &bootstrapv1.LocalEtcd{ + ImageMeta: bootstrapv1.ImageMeta{ + ImageRepository: "public.ecr.aws/eks-distro/etcd-io", + ImageTag: "v3.4.16-eks-1-21-9", + }, + ExtraArgs: map[string]string{ + "cipher-suites": "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + }, + }, + }, + APIServer: bootstrapv1.APIServer{ + ControlPlaneComponent: bootstrapv1.ControlPlaneComponent{ + ExtraArgs: map[string]string{ + "service-account-issuer": "https://test", + "service-account-jwks-uri": "https://test/openid/v1/jwks", + }, + ExtraVolumes: []bootstrapv1.HostPathMount{}, + }, + CertSANs: []string{"foo.bar", "11.11.11.11"}, + }, + ControllerManager: bootstrapv1.ControlPlaneComponent{ + ExtraArgs: tlsCipherSuitesArgs(), + ExtraVolumes: []bootstrapv1.HostPathMount{}, + }, + Scheduler: bootstrapv1.ControlPlaneComponent{ + ExtraArgs: map[string]string{}, + ExtraVolumes: []bootstrapv1.HostPathMount{}, + }, + }, + InitConfiguration: &bootstrapv1.InitConfiguration{ + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + KubeletExtraArgs: map[string]string{ + "tls-cipher-suites": "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "node-labels": "key1=val1,key2=val2", + }, + Taints: []v1.Taint{ + { + Key: "key1", + Value: "val1", + Effect: v1.TaintEffectNoExecute, + TimeAdded: nil, + }, + }, + }, + }, + JoinConfiguration: &bootstrapv1.JoinConfiguration{ + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + KubeletExtraArgs: map[string]string{ + "tls-cipher-suites": "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "node-labels": "key1=val1,key2=val2", + }, + Taints: []v1.Taint{ + { + Key: "key1", + Value: "val1", + Effect: v1.TaintEffectNoExecute, + TimeAdded: nil, + }, + }, + }, + }, + PreKubeadmCommands: []string{}, + PostKubeadmCommands: []string{}, + Files: []bootstrapv1.File{}, + }, + Replicas: &replicas, + Version: "v1.21.5-eks-1-21-9", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := newApiBuilerTest(t) + got := wantKubeadmControlPlane() + g.clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs = tt.apiServerExtraArgs + clusterapi.SetIdentityAuthInKubeadmControlPlane(got, g.clusterSpec) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + func TestConfigureAWSIAMAuthInKubeadmControlPlane(t *testing.T) { replicas := int32(3) tests := []struct { From efcd3afe698ab6c312cef1fa3291be0649482543 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Sun, 3 Mar 2024 12:10:30 -0800 Subject: [PATCH 3/9] Add e2e tests for apiServerExtraArgs create and upgrade cluster --- internal/pkg/api/cluster.go | 20 +++++++++ test/e2e/api_server_extra_args.go | 19 +++++++++ test/e2e/cloudstack_test.go | 42 +++++++++++++++++++ test/e2e/vsphere_test.go | 42 +++++++++++++++++++ test/framework/cluster/validations/cluster.go | 24 +++++++++++ 5 files changed, 147 insertions(+) create mode 100644 test/e2e/api_server_extra_args.go diff --git a/internal/pkg/api/cluster.go b/internal/pkg/api/cluster.go index 8bae35e43d72..07cb59fe0660 100644 --- 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/test/e2e/api_server_extra_args.go b/test/e2e/api_server_extra_args.go new file mode 100644 index 000000000000..50e8039f27e5 --- /dev/null +++ b/test/e2e/api_server_extra_args.go @@ -0,0 +1,19 @@ +//go:build e2e +// +build e2e + +package e2e + +import ( + "github.com/aws/eks-anywhere/test/framework" +) + +func runAPIServerExtraArgsUpgradeFlow(test *framework.ClusterE2ETest, clusterOpts ...[]framework.ClusterE2ETestOpt) { + test.GenerateClusterConfig() + test.CreateCluster() + for _, opts := range clusterOpts { + test.UpgradeClusterWithNewConfig(opts) + test.ValidateClusterState() + test.StopIfFailed() + } + test.DeleteCluster() +} diff --git a/test/e2e/cloudstack_test.go b/test/e2e/cloudstack_test.go index 3c872ec42355..192cb2d97832 100644 --- a/test/e2e/cloudstack_test.go +++ b/test/e2e/cloudstack_test.go @@ -17,6 +17,48 @@ import ( "github.com/aws/eks-anywhere/test/framework" ) +// APIServerExtraArgs +func TestCloudStackKubernetes129RedHat8APIServerExtraArgsSimpleFlow(t *testing.T) { + test := framework.NewClusterE2ETest( + t, + framework.NewCloudStack(t, framework.WithCloudStackRedhat129()), + ).WithClusterConfig( + api.ClusterToConfigFiller( + api.WithKubernetesVersion(v1alpha1.Kube129), + api.WithControlPlaneAPIServerExtraArgs(), + ), + ) + runSimpleFlowWithoutClusterConfigGeneration(test) +} + +// TODO: Investigate why this test takes long time to pass with service-account-issuer flag +func TestCloudStackKubernetes129Redhat8APIServerExtraArgsUpgradeFlow(t *testing.T) { + var addAPIServerExtraArgsclusterOpts []framework.ClusterE2ETestOpt + var removeAPIServerExtraArgsclusterOpts []framework.ClusterE2ETestOpt + test := framework.NewClusterE2ETest( + t, + framework.NewCloudStack(t, framework.WithCloudStackRedhat129()), + framework.WithClusterFiller(api.WithKubernetesVersion(v1alpha1.Kube129)), + ) + addAPIServerExtraArgsclusterOpts = append( + addAPIServerExtraArgsclusterOpts, + framework.WithClusterUpgrade( + api.WithControlPlaneAPIServerExtraArgs(), + ), + ) + removeAPIServerExtraArgsclusterOpts = append( + removeAPIServerExtraArgsclusterOpts, + framework.WithClusterUpgrade( + api.RemoveAllAPIServerExtraArgs(), + ), + ) + runAPIServerExtraArgsUpgradeFlow( + test, + addAPIServerExtraArgsclusterOpts, + removeAPIServerExtraArgsclusterOpts, + ) +} + // AWS IAM Auth func TestCloudStackKubernetes125AWSIamAuth(t *testing.T) { test := framework.NewClusterE2ETest( diff --git a/test/e2e/vsphere_test.go b/test/e2e/vsphere_test.go index b5b47ba71c39..162aea62fb72 100644 --- a/test/e2e/vsphere_test.go +++ b/test/e2e/vsphere_test.go @@ -18,6 +18,48 @@ import ( "github.com/aws/eks-anywhere/test/framework" ) +// APIServerExtraArgs +func TestVSphereKubernetes129BottlerocketAPIServerExtraArgsSimpleFlow(t *testing.T) { + test := framework.NewClusterE2ETest( + t, + framework.NewVSphere(t, framework.WithBottleRocket129()), + ).WithClusterConfig( + api.ClusterToConfigFiller( + api.WithKubernetesVersion(v1alpha1.Kube129), + api.WithControlPlaneAPIServerExtraArgs(), + ), + ) + runSimpleFlowWithoutClusterConfigGeneration(test) +} + +// TODO: Investigate why this test takes long time to pass with service-account-issuer flag +func TestVSphereKubernetes129BottlerocketAPIServerExtraArgsUpgradeFlow(t *testing.T) { + var addAPIServerExtraArgsclusterOpts []framework.ClusterE2ETestOpt + var removeAPIServerExtraArgsclusterOpts []framework.ClusterE2ETestOpt + test := framework.NewClusterE2ETest( + t, + framework.NewVSphere(t, framework.WithBottleRocket129()), + framework.WithClusterFiller(api.WithKubernetesVersion(v1alpha1.Kube129)), + ) + addAPIServerExtraArgsclusterOpts = append( + addAPIServerExtraArgsclusterOpts, + framework.WithClusterUpgrade( + api.WithControlPlaneAPIServerExtraArgs(), + ), + ) + removeAPIServerExtraArgsclusterOpts = append( + removeAPIServerExtraArgsclusterOpts, + framework.WithClusterUpgrade( + api.RemoveAllAPIServerExtraArgs(), + ), + ) + runAPIServerExtraArgsUpgradeFlow( + test, + addAPIServerExtraArgsclusterOpts, + removeAPIServerExtraArgsclusterOpts, + ) +} + // Autoimport func TestVSphereKubernetes125BottlerocketAutoimport(t *testing.T) { provider := framework.NewVSphere(t, diff --git a/test/framework/cluster/validations/cluster.go b/test/framework/cluster/validations/cluster.go index 47668c7abe4f..e803a8f1a457 100644 --- a/test/framework/cluster/validations/cluster.go +++ b/test/framework/cluster/validations/cluster.go @@ -40,6 +40,9 @@ func ValidateClusterReady(ctx context.Context, vc clusterf.StateValidationConfig if clus.Spec.ControlPlaneConfiguration.UpgradeRolloutStrategy != nil && clus.Spec.ControlPlaneConfiguration.UpgradeRolloutStrategy.Type == v1alpha1.InPlaceStrategyType { return validateCAPIobjectsForInPlace(ctx, vc) } + if clus.Spec.ControlPlaneConfiguration.APIServerExtraArgs != nil { + return validateKCPobjectForAPIServerExtraArgs(ctx, vc) + } return nil } @@ -360,3 +363,24 @@ func getWorkerNodeMachineSets(ctx context.Context, vc clusterf.StateValidationCo } return ms.Items, nil } + +func validateKCPobjectForAPIServerExtraArgs(ctx context.Context, vc clusterf.StateValidationConfig) error { + kcp, err := controller.GetKubeadmControlPlane(ctx, vc.ManagementClusterClient, vc.ClusterSpec.Cluster) + if err != nil { + return fmt.Errorf("failed to retrieve kcp: %s", err) + } + if kcp == nil { + return errors.New("KubeadmControlPlane object not found") + } + apiServerExtraArgsKCP := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs + apiServerExtraArgsSpec := vc.ClusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs + if apiServerExtraArgsKCP == nil { + return fmt.Errorf("kcp object APIServerExtraArgs is nil expected: %v", apiServerExtraArgsSpec) + } + for k, v := range apiServerExtraArgsSpec { + if val, ok := apiServerExtraArgsKCP[k]; !ok || val != v { + return fmt.Errorf("kcp object does not have required APIServerExtraArgs expected: %v, actual: %v", apiServerExtraArgsSpec, apiServerExtraArgsKCP) + } + } + return nil +} From dd6c5e846078e8f4c796aec62919187dbe7ac1ce Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Tue, 5 Mar 2024 17:37:07 -0800 Subject: [PATCH 4/9] Add support for both PodIAMConfig and APIServerExtraArgs service-account-issuer --- pkg/clusterapi/extraargs.go | 11 +++++++++++ pkg/clusterapi/identity.go | 15 ++++++++++++--- pkg/providers/cloudstack/template.go | 5 ++--- pkg/providers/docker/docker.go | 2 +- pkg/providers/nutanix/template.go | 6 +++--- pkg/providers/snow/apibuilder.go | 5 ----- pkg/providers/tinkerbell/template.go | 3 +-- pkg/providers/vsphere/template.go | 4 ++-- 8 files changed, 32 insertions(+), 19 deletions(-) diff --git a/pkg/clusterapi/extraargs.go b/pkg/clusterapi/extraargs.go index 2ae614dbac27..25272e818b99 100644 --- a/pkg/clusterapi/extraargs.go +++ b/pkg/clusterapi/extraargs.go @@ -145,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 { diff --git a/pkg/clusterapi/identity.go b/pkg/clusterapi/identity.go index 01fd354b0ae7..d6e915728e6e 100644 --- a/pkg/clusterapi/identity.go +++ b/pkg/clusterapi/identity.go @@ -103,19 +103,28 @@ func configureOIDCInKubeadmControlPlane(kcp *controlplanev1.KubeadmControlPlane, } } +func configureAPIServerExtraArgsInKubeadmControlPlane(kcp *controlplanev1.KubeadmControlPlane, apiServerExtraArgs map[string]string) { + if apiServerExtraArgs == nil { + return + } + + for k, v := range apiServerExtraArgs { + kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs[k] = v + } +} + func configurePodIamAuthInKubeadmControlPlane(kcp *controlplanev1.KubeadmControlPlane, podIamConfig *v1alpha1.PodIAMConfig) { if podIamConfig == nil { return } apiServerExtraArgs := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs - for k, v := range PodIAMAuthExtraArgs(podIamConfig) { - apiServerExtraArgs[k] = v - } + SetPodIAMAuthExtraArgs(podIamConfig, apiServerExtraArgs) } func SetIdentityAuthInKubeadmControlPlane(kcp *controlplanev1.KubeadmControlPlane, clusterSpec *cluster.Spec) { configureOIDCInKubeadmControlPlane(kcp, clusterSpec.OIDCConfig) configureAWSIAMAuthInKubeadmControlPlane(kcp, clusterSpec.AWSIamConfig) + configureAPIServerExtraArgsInKubeadmControlPlane(kcp, clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs) configurePodIamAuthInKubeadmControlPlane(kcp, clusterSpec.Cluster.Spec.PodIAMConfig) } diff --git a/pkg/providers/cloudstack/template.go b/pkg/providers/cloudstack/template.go index d6ad013bf239..53b4cce84cb4 100644 --- a/pkg/providers/cloudstack/template.go +++ b/pkg/providers/cloudstack/template.go @@ -121,11 +121,10 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration)) apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). - Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). + Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). Append(sharedExtraArgs) - + clusterapi.SetPodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig, apiServerExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.NodeCIDRMaskExtraArgs(&clusterSpec.Cluster.Spec.ClusterNetwork)) diff --git a/pkg/providers/docker/docker.go b/pkg/providers/docker/docker.go index 52042b03f9fc..b74665a028b3 100644 --- a/pkg/providers/docker/docker.go +++ b/pkg/providers/docker/docker.go @@ -294,9 +294,9 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). Append(sharedExtraArgs) + clusterapi.SetPodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig, apiServerExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.NodeCIDRMaskExtraArgs(&clusterSpec.Cluster.Spec.ClusterNetwork)) diff --git a/pkg/providers/nutanix/template.go b/pkg/providers/nutanix/template.go index 10564ff78d84..5b33ad2c90fb 100644 --- a/pkg/providers/nutanix/template.go +++ b/pkg/providers/nutanix/template.go @@ -161,9 +161,9 @@ func buildTemplateMapCP( format := "cloud-config" apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). - Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). - Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)) + Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). + Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)) + clusterapi.SetPodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig, apiServerExtraArgs) kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)). Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration)) diff --git a/pkg/providers/snow/apibuilder.go b/pkg/providers/snow/apibuilder.go index 1794770f81cc..5e4f294f3f02 100644 --- a/pkg/providers/snow/apibuilder.go +++ b/pkg/providers/snow/apibuilder.go @@ -48,11 +48,6 @@ func KubeadmControlPlane(log logr.Logger, clusterSpec *cluster.Spec, snowMachine return nil, fmt.Errorf("setting kube-vip: %v", err) } - apiServerExtraArgs := clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs - for k, v := range apiServerExtraArgs { - kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs[k] = v - } - initConfigKubeletExtraArg := kcp.Spec.KubeadmConfigSpec.InitConfiguration.NodeRegistration.KubeletExtraArgs initConfigKubeletExtraArg["provider-id"] = "aws-snow:////'{{ ds.meta_data.instance_id }}'" diff --git a/pkg/providers/tinkerbell/template.go b/pkg/providers/tinkerbell/template.go index 645c22eca298..d829723d8f66 100644 --- a/pkg/providers/tinkerbell/template.go +++ b/pkg/providers/tinkerbell/template.go @@ -399,9 +399,8 @@ func buildTemplateMapCP( apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)) - + clusterapi.SetPodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig, apiServerExtraArgs) kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)). Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration)) diff --git a/pkg/providers/vsphere/template.go b/pkg/providers/vsphere/template.go index 903ff4020f9b..2e24f7a8d614 100644 --- a/pkg/providers/vsphere/template.go +++ b/pkg/providers/vsphere/template.go @@ -143,10 +143,10 @@ func buildTemplateMapCP( Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration)) apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig). Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)). - Append(clusterapi.PodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig)). - Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)). + Append(clusterapi.EtcdEncryptionExtraArgs(clusterSpec.Cluster.Spec.EtcdEncryption)). Append(sharedExtraArgs) + clusterapi.SetPodIAMAuthExtraArgs(clusterSpec.Cluster.Spec.PodIAMConfig, apiServerExtraArgs) controllerManagerExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs(). Append(clusterapi.NodeCIDRMaskExtraArgs(&clusterSpec.Cluster.Spec.ClusterNetwork)) From 42146bc025146cea640436ec82ec56d88a9e8a07 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Thu, 7 Mar 2024 14:37:53 -0800 Subject: [PATCH 5/9] Allow users to skip api server extra args preflight validation --- .../createvalidations/createvalidations.go | 1 + .../createvalidations/preflightvalidations.go | 11 +++++++++++ pkg/validations/skipvalidations.go | 7 ++++--- pkg/validations/skipvalidations_test.go | 10 ++++++---- .../upgradevalidations/preflightvalidations.go | 11 +++++++++++ .../upgradevalidations/upgradevalidations.go | 1 + 6 files changed, 34 insertions(+), 7 deletions(-) diff --git a/pkg/validations/createvalidations/createvalidations.go b/pkg/validations/createvalidations/createvalidations.go index a42049530fdc..61489db582b8 100644 --- a/pkg/validations/createvalidations/createvalidations.go +++ b/pkg/validations/createvalidations/createvalidations.go @@ -7,6 +7,7 @@ import ( // SkippableValidations represents all the validations we offer for users to skip. var SkippableValidations = []string{ validations.VSphereUserPriv, + validations.APIServerExtraArgs, } func New(opts *validations.Opts) *CreateValidations { diff --git a/pkg/validations/createvalidations/preflightvalidations.go b/pkg/validations/createvalidations/preflightvalidations.go index 76fa21e31a6c..660fa43b71a3 100644 --- a/pkg/validations/createvalidations/preflightvalidations.go +++ b/pkg/validations/createvalidations/preflightvalidations.go @@ -107,5 +107,16 @@ func (v *CreateValidations) PreflightValidations(ctx context.Context) []validati ) } + if !v.Opts.SkippedValidations[validations.APIServerExtraArgs] { + createValidations = append( + createValidations, + func() *validations.ValidationResult { + return &validations.ValidationResult{ + Name: "validate api server extra args", + Remediation: "ensure apiServerExtraArgs have only supported flags (service-account-issuer, service-account-jwks-uri)", + Err: validations.ValidateAPIServerExtraArgs(v.Opts.Spec), + } + }) + } return createValidations } diff --git a/pkg/validations/skipvalidations.go b/pkg/validations/skipvalidations.go index 268b372ab918..241554c373e6 100644 --- a/pkg/validations/skipvalidations.go +++ b/pkg/validations/skipvalidations.go @@ -7,9 +7,10 @@ import ( // string values of supported validation names that can be skipped. const ( - PDB = "pod-disruption" - VSphereUserPriv = "vsphere-user-privilege" - EksaVersionSkew = "eksa-version-skew" + PDB = "pod-disruption" + VSphereUserPriv = "vsphere-user-privilege" + EksaVersionSkew = "eksa-version-skew" + APIServerExtraArgs = "api-server-extra-args" ) // ValidSkippableValidationsMap returns a map for all valid skippable validations as keys, defaulting values to false. diff --git a/pkg/validations/skipvalidations_test.go b/pkg/validations/skipvalidations_test.go index 0fff9b0f4693..fd05186a8ada 100644 --- a/pkg/validations/skipvalidations_test.go +++ b/pkg/validations/skipvalidations_test.go @@ -29,9 +29,10 @@ func TestValidateSkippableValidation(t *testing.T) { { name: "valid upgrade validation param", want: map[string]bool{ - validations.PDB: true, - validations.VSphereUserPriv: false, - validations.EksaVersionSkew: false, + validations.PDB: true, + validations.VSphereUserPriv: false, + validations.EksaVersionSkew: false, + validations.APIServerExtraArgs: false, }, wantErr: nil, skippedValidations: []string{validations.PDB}, @@ -40,7 +41,8 @@ func TestValidateSkippableValidation(t *testing.T) { { name: "valid create validation param", want: map[string]bool{ - validations.VSphereUserPriv: true, + validations.VSphereUserPriv: true, + validations.APIServerExtraArgs: false, }, wantErr: nil, skippedValidations: []string{validations.VSphereUserPriv}, diff --git a/pkg/validations/upgradevalidations/preflightvalidations.go b/pkg/validations/upgradevalidations/preflightvalidations.go index 650bd9f941e2..55aebfb45767 100644 --- a/pkg/validations/upgradevalidations/preflightvalidations.go +++ b/pkg/validations/upgradevalidations/preflightvalidations.go @@ -166,6 +166,17 @@ func (u *UpgradeValidations) PreflightValidations(ctx context.Context) []validat } }) } + if !u.Opts.SkippedValidations[validations.APIServerExtraArgs] { + upgradeValidations = append( + upgradeValidations, + func() *validations.ValidationResult { + return &validations.ValidationResult{ + Name: "validate api server extra args", + Remediation: "ensure apiServerExtraArgs have only supported flags (service-account-issuer, service-account-jwks-uri)", + Err: validations.ValidateAPIServerExtraArgs(u.Opts.Spec), + } + }) + } return upgradeValidations } diff --git a/pkg/validations/upgradevalidations/upgradevalidations.go b/pkg/validations/upgradevalidations/upgradevalidations.go index 149074350709..ab68e35da386 100644 --- a/pkg/validations/upgradevalidations/upgradevalidations.go +++ b/pkg/validations/upgradevalidations/upgradevalidations.go @@ -9,6 +9,7 @@ var SkippableValidations = []string{ validations.PDB, validations.VSphereUserPriv, validations.EksaVersionSkew, + validations.APIServerExtraArgs, } func New(opts *validations.Opts) *UpgradeValidations { From 5a67013faa556b779a3a667a676ed9447b88df19 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Thu, 14 Mar 2024 00:42:13 -0700 Subject: [PATCH 6/9] Add feature flag to configure api server extra args --- pkg/api/v1alpha1/cluster.go | 8 ++++++++ pkg/features/features.go | 9 +++++++++ pkg/features/features_test.go | 8 ++++++++ test/e2e/cloudstack_test.go | 2 ++ test/e2e/vsphere_test.go | 2 ++ 5 files changed, 29 insertions(+) diff --git a/pkg/api/v1alpha1/cluster.go b/pkg/api/v1alpha1/cluster.go index 578c43c2a8b1..f3e886e320ee 100644 --- a/pkg/api/v1alpha1/cluster.go +++ b/pkg/api/v1alpha1/cluster.go @@ -190,6 +190,7 @@ var clusterConfigValidations = []func(*Cluster) error{ validatePackageControllerConfiguration, validateEksaVersion, validateControlPlaneCertSANs, + validateControlPlaneAPIServerExtraArgs, } // GetClusterConfig parses a Cluster object from a multiobject yaml file in disk @@ -494,6 +495,13 @@ func validateControlPlaneCertSANs(cfg *Cluster) error { return nil } +func validateControlPlaneAPIServerExtraArgs(clusterConfig *Cluster) error { + if clusterConfig.Spec.ControlPlaneConfiguration.APIServerExtraArgs != nil && !features.IsActive(features.APIServerExtraArgsEnabled()) { + return errors.New("please enable feature flag to configure APIServerExtraArgs") + } + return nil +} + func validateWorkerNodeGroups(clusterConfig *Cluster) error { workerNodeGroupConfigs := clusterConfig.Spec.WorkerNodeGroupConfigurations if len(workerNodeGroupConfigs) <= 0 { diff --git a/pkg/features/features.go b/pkg/features/features.go index c462ca73da2f..7cb88ec06d58 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -7,6 +7,7 @@ const ( UseNewWorkflowsEnvVar = "USE_NEW_WORKFLOWS" UseControllerForCli = "USE_CONTROLLER_FOR_CLI" VSphereInPlaceEnvVar = "VSPHERE_IN_PLACE_UPGRADE" + APIServerExtraArgsEnabledEnvVar = "API_SERVER_EXTRA_ARGS_ENABLED" ) func FeedGates(featureGates []string) { @@ -55,3 +56,11 @@ func VSphereInPlaceUpgradeEnabled() Feature { IsActive: globalFeatures.isActiveForEnvVar(VSphereInPlaceEnvVar), } } + +// APIServerExtraArgsEnabled is the feature flag for configuring api server extra args. +func APIServerExtraArgsEnabled() Feature { + return Feature{ + Name: "Configure api server extra args", + IsActive: globalFeatures.isActiveForEnvVar(APIServerExtraArgsEnabledEnvVar), + } +} diff --git a/pkg/features/features_test.go b/pkg/features/features_test.go index de3a262f8801..739d5f4c146c 100644 --- a/pkg/features/features_test.go +++ b/pkg/features/features_test.go @@ -77,3 +77,11 @@ func TestVSphereInPlaceUpgradeEnabledFeatureFlag(t *testing.T) { g.Expect(os.Setenv(VSphereInPlaceEnvVar, "true")).To(Succeed()) g.Expect(IsActive(VSphereInPlaceUpgradeEnabled())).To(BeTrue()) } + +func TestAPIServerExtraArgsEnabledFeatureFlag(t *testing.T) { + g := NewWithT(t) + setupContext(t) + + g.Expect(os.Setenv(APIServerExtraArgsEnabledEnvVar, "true")).To(Succeed()) + g.Expect(IsActive(APIServerExtraArgsEnabled())).To(BeTrue()) +} diff --git a/test/e2e/cloudstack_test.go b/test/e2e/cloudstack_test.go index 192cb2d97832..560a4f96b3b6 100644 --- a/test/e2e/cloudstack_test.go +++ b/test/e2e/cloudstack_test.go @@ -22,6 +22,7 @@ func TestCloudStackKubernetes129RedHat8APIServerExtraArgsSimpleFlow(t *testing.T test := framework.NewClusterE2ETest( t, framework.NewCloudStack(t, framework.WithCloudStackRedhat129()), + framework.WithEnvVar(features.APIServerExtraArgsEnabledEnvVar, "true"), ).WithClusterConfig( api.ClusterToConfigFiller( api.WithKubernetesVersion(v1alpha1.Kube129), @@ -39,6 +40,7 @@ func TestCloudStackKubernetes129Redhat8APIServerExtraArgsUpgradeFlow(t *testing. t, framework.NewCloudStack(t, framework.WithCloudStackRedhat129()), framework.WithClusterFiller(api.WithKubernetesVersion(v1alpha1.Kube129)), + framework.WithEnvVar(features.APIServerExtraArgsEnabledEnvVar, "true"), ) addAPIServerExtraArgsclusterOpts = append( addAPIServerExtraArgsclusterOpts, diff --git a/test/e2e/vsphere_test.go b/test/e2e/vsphere_test.go index 162aea62fb72..53b31606bd71 100644 --- a/test/e2e/vsphere_test.go +++ b/test/e2e/vsphere_test.go @@ -23,6 +23,7 @@ func TestVSphereKubernetes129BottlerocketAPIServerExtraArgsSimpleFlow(t *testing test := framework.NewClusterE2ETest( t, framework.NewVSphere(t, framework.WithBottleRocket129()), + framework.WithEnvVar(features.APIServerExtraArgsEnabledEnvVar, "true"), ).WithClusterConfig( api.ClusterToConfigFiller( api.WithKubernetesVersion(v1alpha1.Kube129), @@ -40,6 +41,7 @@ func TestVSphereKubernetes129BottlerocketAPIServerExtraArgsUpgradeFlow(t *testin t, framework.NewVSphere(t, framework.WithBottleRocket129()), framework.WithClusterFiller(api.WithKubernetesVersion(v1alpha1.Kube129)), + framework.WithEnvVar(features.APIServerExtraArgsEnabledEnvVar, "true"), ) addAPIServerExtraArgsclusterOpts = append( addAPIServerExtraArgsclusterOpts, From 8ac450a0365c79febeaa4670b8ab4576cd9e59d8 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Thu, 14 Mar 2024 12:41:20 -0700 Subject: [PATCH 7/9] Revert "Allow users to skip api server extra args preflight validation" This reverts commit e4b9ef4816a5622fa2cb2d21fab23be9f1bf8ba0. --- .../createvalidations/createvalidations.go | 1 - .../createvalidations/preflightvalidations.go | 11 ----------- pkg/validations/skipvalidations.go | 7 +++---- pkg/validations/skipvalidations_test.go | 10 ++++------ .../upgradevalidations/preflightvalidations.go | 11 ----------- .../upgradevalidations/upgradevalidations.go | 1 - 6 files changed, 7 insertions(+), 34 deletions(-) diff --git a/pkg/validations/createvalidations/createvalidations.go b/pkg/validations/createvalidations/createvalidations.go index 61489db582b8..a42049530fdc 100644 --- a/pkg/validations/createvalidations/createvalidations.go +++ b/pkg/validations/createvalidations/createvalidations.go @@ -7,7 +7,6 @@ import ( // SkippableValidations represents all the validations we offer for users to skip. var SkippableValidations = []string{ validations.VSphereUserPriv, - validations.APIServerExtraArgs, } func New(opts *validations.Opts) *CreateValidations { diff --git a/pkg/validations/createvalidations/preflightvalidations.go b/pkg/validations/createvalidations/preflightvalidations.go index 660fa43b71a3..76fa21e31a6c 100644 --- a/pkg/validations/createvalidations/preflightvalidations.go +++ b/pkg/validations/createvalidations/preflightvalidations.go @@ -107,16 +107,5 @@ func (v *CreateValidations) PreflightValidations(ctx context.Context) []validati ) } - if !v.Opts.SkippedValidations[validations.APIServerExtraArgs] { - createValidations = append( - createValidations, - func() *validations.ValidationResult { - return &validations.ValidationResult{ - Name: "validate api server extra args", - Remediation: "ensure apiServerExtraArgs have only supported flags (service-account-issuer, service-account-jwks-uri)", - Err: validations.ValidateAPIServerExtraArgs(v.Opts.Spec), - } - }) - } return createValidations } diff --git a/pkg/validations/skipvalidations.go b/pkg/validations/skipvalidations.go index 241554c373e6..268b372ab918 100644 --- a/pkg/validations/skipvalidations.go +++ b/pkg/validations/skipvalidations.go @@ -7,10 +7,9 @@ import ( // string values of supported validation names that can be skipped. const ( - PDB = "pod-disruption" - VSphereUserPriv = "vsphere-user-privilege" - EksaVersionSkew = "eksa-version-skew" - APIServerExtraArgs = "api-server-extra-args" + PDB = "pod-disruption" + VSphereUserPriv = "vsphere-user-privilege" + EksaVersionSkew = "eksa-version-skew" ) // ValidSkippableValidationsMap returns a map for all valid skippable validations as keys, defaulting values to false. diff --git a/pkg/validations/skipvalidations_test.go b/pkg/validations/skipvalidations_test.go index fd05186a8ada..0fff9b0f4693 100644 --- a/pkg/validations/skipvalidations_test.go +++ b/pkg/validations/skipvalidations_test.go @@ -29,10 +29,9 @@ func TestValidateSkippableValidation(t *testing.T) { { name: "valid upgrade validation param", want: map[string]bool{ - validations.PDB: true, - validations.VSphereUserPriv: false, - validations.EksaVersionSkew: false, - validations.APIServerExtraArgs: false, + validations.PDB: true, + validations.VSphereUserPriv: false, + validations.EksaVersionSkew: false, }, wantErr: nil, skippedValidations: []string{validations.PDB}, @@ -41,8 +40,7 @@ func TestValidateSkippableValidation(t *testing.T) { { name: "valid create validation param", want: map[string]bool{ - validations.VSphereUserPriv: true, - validations.APIServerExtraArgs: false, + validations.VSphereUserPriv: true, }, wantErr: nil, skippedValidations: []string{validations.VSphereUserPriv}, diff --git a/pkg/validations/upgradevalidations/preflightvalidations.go b/pkg/validations/upgradevalidations/preflightvalidations.go index 55aebfb45767..650bd9f941e2 100644 --- a/pkg/validations/upgradevalidations/preflightvalidations.go +++ b/pkg/validations/upgradevalidations/preflightvalidations.go @@ -166,17 +166,6 @@ func (u *UpgradeValidations) PreflightValidations(ctx context.Context) []validat } }) } - if !u.Opts.SkippedValidations[validations.APIServerExtraArgs] { - upgradeValidations = append( - upgradeValidations, - func() *validations.ValidationResult { - return &validations.ValidationResult{ - Name: "validate api server extra args", - Remediation: "ensure apiServerExtraArgs have only supported flags (service-account-issuer, service-account-jwks-uri)", - Err: validations.ValidateAPIServerExtraArgs(u.Opts.Spec), - } - }) - } return upgradeValidations } diff --git a/pkg/validations/upgradevalidations/upgradevalidations.go b/pkg/validations/upgradevalidations/upgradevalidations.go index ab68e35da386..149074350709 100644 --- a/pkg/validations/upgradevalidations/upgradevalidations.go +++ b/pkg/validations/upgradevalidations/upgradevalidations.go @@ -9,7 +9,6 @@ var SkippableValidations = []string{ validations.PDB, validations.VSphereUserPriv, validations.EksaVersionSkew, - validations.APIServerExtraArgs, } func New(opts *validations.Opts) *UpgradeValidations { From a8b6a35b74c95149be6323c496abf3c480d25ce1 Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Tue, 19 Mar 2024 11:00:42 -0700 Subject: [PATCH 8/9] Add validation for oidc flags configured in apiServerExtraArgs --- pkg/api/v1alpha1/cluster.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/api/v1alpha1/cluster.go b/pkg/api/v1alpha1/cluster.go index f3e886e320ee..9db31d96fb26 100644 --- a/pkg/api/v1alpha1/cluster.go +++ b/pkg/api/v1alpha1/cluster.go @@ -191,6 +191,7 @@ var clusterConfigValidations = []func(*Cluster) error{ validateEksaVersion, validateControlPlaneCertSANs, validateControlPlaneAPIServerExtraArgs, + validateControlPlaneAPIServerOIDCExtraArgs, } // GetClusterConfig parses a Cluster object from a multiobject yaml file in disk @@ -502,6 +503,30 @@ func validateControlPlaneAPIServerExtraArgs(clusterConfig *Cluster) error { 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("%s flag is already configured in OIDCConfig. please remove it from apiServerExtraArgs", flag) + } + } + } + } + } + return nil +} + func validateWorkerNodeGroups(clusterConfig *Cluster) error { workerNodeGroupConfigs := clusterConfig.Spec.WorkerNodeGroupConfigurations if len(workerNodeGroupConfigs) <= 0 { From 0a7452c933b3033a85f2645b26297d6c091db4fb Mon Sep 17 00:00:00 2001 From: Saurabh Parekh Date: Tue, 19 Mar 2024 17:29:07 -0700 Subject: [PATCH 9/9] Update error messages --- pkg/api/v1alpha1/cluster.go | 4 ++-- test/framework/cluster/validations/cluster.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/api/v1alpha1/cluster.go b/pkg/api/v1alpha1/cluster.go index 9db31d96fb26..0d5d0d1ca6e4 100644 --- a/pkg/api/v1alpha1/cluster.go +++ b/pkg/api/v1alpha1/cluster.go @@ -498,7 +498,7 @@ func validateControlPlaneCertSANs(cfg *Cluster) error { func validateControlPlaneAPIServerExtraArgs(clusterConfig *Cluster) error { if clusterConfig.Spec.ControlPlaneConfiguration.APIServerExtraArgs != nil && !features.IsActive(features.APIServerExtraArgsEnabled()) { - return errors.New("please enable feature flag to configure APIServerExtraArgs") + return fmt.Errorf("configuring APIServerExtraArgs is not supported. Set env var %v to enable", features.APIServerExtraArgsEnabledEnvVar) } return nil } @@ -518,7 +518,7 @@ func validateControlPlaneAPIServerOIDCExtraArgs(clusterConfig *Cluster) error { if ref.Kind == OIDCConfigKind { for _, flag := range oidcFlags { if _, has := clusterConfig.Spec.ControlPlaneConfiguration.APIServerExtraArgs[flag]; has { - return fmt.Errorf("%s flag is already configured in OIDCConfig. please remove it from apiServerExtraArgs", flag) + return fmt.Errorf("the following flags cannot be configured if OIDCConfig is configured for cluster.spec.identityProviderRefs: %v. Remove from apiServerExtraArgs", oidcFlags) } } } diff --git a/test/framework/cluster/validations/cluster.go b/test/framework/cluster/validations/cluster.go index e803a8f1a457..31c7e477fe49 100644 --- a/test/framework/cluster/validations/cluster.go +++ b/test/framework/cluster/validations/cluster.go @@ -41,7 +41,7 @@ func ValidateClusterReady(ctx context.Context, vc clusterf.StateValidationConfig return validateCAPIobjectsForInPlace(ctx, vc) } if clus.Spec.ControlPlaneConfiguration.APIServerExtraArgs != nil { - return validateKCPobjectForAPIServerExtraArgs(ctx, vc) + return validateKCPForAPIServerExtraArgs(ctx, vc) } return nil } @@ -364,7 +364,7 @@ func getWorkerNodeMachineSets(ctx context.Context, vc clusterf.StateValidationCo return ms.Items, nil } -func validateKCPobjectForAPIServerExtraArgs(ctx context.Context, vc clusterf.StateValidationConfig) error { +func validateKCPForAPIServerExtraArgs(ctx context.Context, vc clusterf.StateValidationConfig) error { kcp, err := controller.GetKubeadmControlPlane(ctx, vc.ManagementClusterClient, vc.ClusterSpec.Cluster) if err != nil { return fmt.Errorf("failed to retrieve kcp: %s", err)