fix: set defaults for AWS CP and Worker instanceType (#504)

Moved
d2iq-labs#51
Addressed the pending review comments
**What problem does this PR solve?:**

- sets defaults for aws nodes CP: m5.xlarge and Workers: m5.2xlarge
- sets PLACEHOLDER value as defaults in AWSMachineTemplates because
instanceType field is required by CAPA.
- sets instance type as required field in the schema since CAREN will be
using the default if not provided by the user.
update existing unit tests to use default schema
**NOTE**
Once this PR is finalized, I will create follow up PR to set instance
profile and some other changes in the API types that would ensure that
it creates variable schema from the object instead of creating new one.

**Which issue(s) this PR fixes:**
Fixes #
#485

**How Has This Been Tested?:**

Tried to test locally using make e2e-test E2E_LABEL='provider:AWS &&
cni:Cilium' but local environment timed outs during initializations.
Looking into fixing local environment (colima +docker +kind on mac)

Tested manually be creating AWS cluster.

aws-quick-start cluster class has following
```
 instanceType:
                      default: m5.xlarge
                      description: The AWS instance type to use for the cluster Machines
                      type: string
                  required:
                  - instanceType
```
and workers
```
                instanceType:
                  default: m5.2xlarge
                  description: The AWS instance type to use for the cluster Machines
                  type: string
              required:
              - instanceType
```
Created AWS cluster without adding instanceType variable, CAREN patched
AWSMachineTemplate with default instanceType.

** Special notes for your reviewer:**

The approach taken in this PR will be used as guide to set default for
other variables.

Author: supershal

api/v1alpha1/aws_node_types.go

@@ -5,7 +5,15 @@ package v1alpha1
 
 import (
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
+)
+
+const (
+	AWSControlPlaneInstanceType InstanceType = "m5.xlarge"
+	AWSWorkerInstanceType       InstanceType = "m5.2xlarge"
 )
 
 type AWSNodeSpec struct {
@@ -24,6 +32,18 @@ type AWSNodeSpec struct {
 	AdditionalSecurityGroups AdditionalSecurityGroup `json:"additionalSecurityGroups,omitempty"`
 }
 
+func NewAWSControlPlaneNodeSpec() *AWSNodeSpec {
+	return &AWSNodeSpec{
+		InstanceType: ptr.To(AWSControlPlaneInstanceType),
+	}
+}
+
+func NewAWSWorkerNodeSpec() *AWSNodeSpec {
+	return &AWSNodeSpec{
+		InstanceType: ptr.To(AWSWorkerInstanceType),
+	}
+}
+
 type AdditionalSecurityGroup []SecurityGroup
 
 type SecurityGroup struct {
@@ -49,17 +69,18 @@ func (AdditionalSecurityGroup) VariableSchema() clusterv1.VariableSchema {
 	}
 }
 
-func (AWSNodeSpec) VariableSchema() clusterv1.VariableSchema {
+func (a AWSNodeSpec) VariableSchema() clusterv1.VariableSchema {
 	return clusterv1.VariableSchema{
 		OpenAPIV3Schema: clusterv1.JSONSchemaProps{
 			Description: "AWS Node configuration",
 			Type:        "object",
 			Properties: map[string]clusterv1.JSONSchemaProps{
 				"iamInstanceProfile":       IAMInstanceProfile("").VariableSchema().OpenAPIV3Schema,
-				"instanceType":             InstanceType("").VariableSchema().OpenAPIV3Schema,
+				"instanceType":             a.InstanceType.VariableSchema().OpenAPIV3Schema,
 				"ami":                      AMISpec{}.VariableSchema().OpenAPIV3Schema,
 				"additionalSecurityGroups": AdditionalSecurityGroup{}.VariableSchema().OpenAPIV3Schema,
 			},
+			Required: []string{"instanceType"},
 		},
 	}
 }
@@ -77,11 +98,12 @@ func (IAMInstanceProfile) VariableSchema() clusterv1.VariableSchema {
 
 type InstanceType string
 
-func (InstanceType) VariableSchema() clusterv1.VariableSchema {
+func (i InstanceType) VariableSchema() clusterv1.VariableSchema {
 	return clusterv1.VariableSchema{
 		OpenAPIV3Schema: clusterv1.JSONSchemaProps{
 			Type:        "string",
 			Description: "The AWS instance type to use for the cluster Machines",
+			Default:     variables.MustMarshal(i),
 		},
 	}
 }

api/v1alpha1/clusterconfig_types.go

@@ -57,16 +57,13 @@ type ClusterConfigSpec struct {
 
 func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint:gocritic,lll // Passed by value for no potential side-effect.
 	clusterConfigProps := GenericClusterConfig{}.VariableSchema()
-
 	switch {
 	case s.AWS != nil:
 		maps.Copy(
 			clusterConfigProps.OpenAPIV3Schema.Properties,
 			map[string]clusterv1.JSONSchemaProps{
-				AWSVariableName: AWSSpec{}.VariableSchema().OpenAPIV3Schema,
-				"controlPlane": NodeConfigSpec{
-					AWS: &AWSNodeSpec{},
-				}.VariableSchema().OpenAPIV3Schema,
+				AWSVariableName: s.AWS.VariableSchema().OpenAPIV3Schema,
+				"controlPlane":  s.ControlPlane.VariableSchema().OpenAPIV3Schema,
 			},
 		)
 	case s.Docker != nil:
@@ -94,6 +91,15 @@ func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint:
 	return clusterConfigProps
 }
 
+func NewAWSClusterConfigSpec() *ClusterConfigSpec {
+	return &ClusterConfigSpec{
+		AWS: &AWSSpec{},
+		ControlPlane: &NodeConfigSpec{
+			AWS: NewAWSControlPlaneNodeSpec(),
+		},
+	}
+}
+
 // GenericClusterConfig defines the generic cluster configdesired.
 type GenericClusterConfig struct {
 	// +optional

api/v1alpha1/node_types.go

@@ -41,7 +41,7 @@ func (s NodeConfigSpec) VariableSchema() clusterv1.VariableSchema {
 		maps.Copy(
 			nodeConfigProps.OpenAPIV3Schema.Properties,
 			map[string]clusterv1.JSONSchemaProps{
-				AWSVariableName: AWSNodeSpec{}.VariableSchema().OpenAPIV3Schema,
+				AWSVariableName: s.AWS.VariableSchema().OpenAPIV3Schema,
 			},
 		)
 	case s.Docker != nil:
@@ -63,6 +63,12 @@ func (s NodeConfigSpec) VariableSchema() clusterv1.VariableSchema {
 	return nodeConfigProps
 }
 
+func NewAWSWorkerConfigSpec() *NodeConfigSpec {
+	return &NodeConfigSpec{
+		AWS: NewAWSWorkerNodeSpec(),
+	}
+}
+
 type GenericNodeConfig struct{}
 
 func (GenericNodeConfig) VariableSchema() clusterv1.VariableSchema {

charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml

@@ -106,7 +106,7 @@ spec:
   template:
     spec:
       iamInstanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
-      instanceType: m5.xlarge
+      instanceType: PLACEHOLDER
       sshKeyName: ""
 ---
 apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
@@ -119,7 +119,7 @@ spec:
   template:
     spec:
       iamInstanceProfile: nodes.cluster-api-provider-aws.sigs.k8s.io
-      instanceType: m5.2xlarge
+      instanceType: PLACEHOLDER
       sshKeyName: ""
 ---
 apiVersion: bootstrap.cluster.x-k8s.io/v1beta1

hack/examples/overlays/clusterclasses/aws/kustomization.yaml.tmpl

@@ -51,11 +51,11 @@ patches:
     patch: |-
       - op: "add"
         path: "/spec/template/spec/instanceType"
-        value: "m5.2xlarge"
+        value: "PLACEHOLDER"
   - target:
       kind: AWSMachineTemplate
       name: quick-start-control-plane
     patch: |-
       - op: "add"
         path: "/spec/template/spec/instanceType"
-        value: "m5.xlarge"
+        value: "PLACEHOLDER"

pkg/handlers/aws/clusterconfig/variables.go

@@ -43,7 +43,7 @@ func (h *awsClusterConfigVariableHandler) DiscoverVariables(
 	resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
 		Name:     clusterconfig.MetaVariableName,
 		Required: true,
-		Schema:   v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema(),
+		Schema:   v1alpha1.NewAWSClusterConfigSpec().VariableSchema(),
 	})
 	resp.SetStatus(runtimehooksv1.ResponseStatusSuccess)
 }

pkg/handlers/aws/mutation/ami/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/controlplaneloadbalancer/variables_test.go

@@ -19,7 +19,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/iaminstanceprofile/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/instancetype/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/network/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/region/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/mutation/securitygroups/variables_test.go

@@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		clusterconfig.MetaVariableName,
-		ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()),
 		true,
 		awsclusterconfig.NewVariable,
 		capitest.VariableTestDef{

pkg/handlers/aws/workerconfig/variables.go

@@ -43,7 +43,7 @@ func (h *awsWorkerConfigVariableHandler) DiscoverVariables(
 	resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
 		Name:     workerconfig.MetaVariableName,
 		Required: false,
-		Schema:   v1alpha1.NodeConfigSpec{AWS: &v1alpha1.AWSNodeSpec{}}.VariableSchema(),
+		Schema:   v1alpha1.NewAWSWorkerConfigSpec().VariableSchema(),
 	})
 	resp.SetStatus(runtimehooksv1.ResponseStatusSuccess)
 }

pkg/handlers/aws/workerconfig/variables_test.go

@@ -17,7 +17,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		workerconfig.MetaVariableName,
-		ptr.To(v1alpha1.NodeConfigSpec{AWS: &v1alpha1.AWSNodeSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSWorkerConfigSpec().VariableSchema()),
 		false,
 		NewVariable,
 		capitest.VariableTestDef{