Fix comments

Signed-off-by: serngawy <[email protected]>

Author: serngawy

config/crd/bases/controlplane.cluster.x-k8s.io_rosacontrolplanes.yaml

@@ -783,8 +783,8 @@ spec:
                   rule: self == oldSelf
               rosaRoleConfigRef:
                 description: |-
-                  RosaRoleConfigRef is a reference to a RosaRoleConfig resource that contains account and operator roles and OIDC configuration.
-                  If specified, the roles and OIDC configuration will be taken from the referenced RosaRoleConfig instead of the direct fields.
+                  RosaRoleConfigRef is a reference to a RosaRoleConfig resource that contains account roles, operator roles and OIDC configuration.
+                  RosaRoleConfigRef and role fields such as installerRoleARN, supportRoleARN, workerRoleARN, rolesRef and oidcID are mutually exclusive.
                 properties:
                   name:
                     default: ""

config/crd/bases/infrastructure.cluster.x-k8s.io_rosaroleconfigs.yaml

@@ -48,31 +48,44 @@ spec:
                   creating your ROSA cluster.
                 properties:
                   path:
+                    description: The arn path for the account/operator roles as well
+                      as their policies.
                     type: string
                   permissionsBoundaryARN:
+                    description: The ARN of the policy that is used to set the permissions
+                      boundary for the account roles.
                     type: string
                   prefix:
-                    description: User-defined prefix for all generated AWS resources
+                    description: User-defined prefix for all generated AWS account
+                      role
                     maxLength: 4
+                    pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$
                     type: string
+                    x-kubernetes-validations:
+                    - message: prefix is immutable
+                      rule: self == oldSelf
                   sharedVPCConfig:
                     description: SharedVPCConfig is used to set up shared VPC.
                     properties:
                       routeRoleARN:
-                        description: ' Role ARN associated with the private hosted
-                          zone used for Hosted Control Plane cluster shared VPC, this
-                          role contains policies to be used with Route 53'
+                        description: Role ARN associated with the private hosted zone
+                          used for Hosted Control Plane cluster shared VPC, this role
+                          contains policies to be used with Route 53
                         type: string
                       vpcEndpointRoleArn:
-                        description: ' Role ARN associated with the shared VPC used
+                        description: Role ARN associated with the shared VPC used
                           for Hosted Control Plane clusters, this role contains policies
-                          to be used with the VPC endpoint'
+                          to be used with the VPC endpoint
                         type: string
                     type: object
                   version:
-                    description: ' Version of OpenShift that will be used to setup
-                      policy tag, for example "4.11"'
+                    description: |-
+                      Version of OpenShift that will be used to the roles tag in formate of x.y.z example; "4.19.0"
+                      Setting the role OpenShift version tag does not affect the associated ROSAControlplane version.
                     type: string
+                    x-kubernetes-validations:
+                    - message: version is immutable
+                      rule: self == oldSelf
                 required:
                 - prefix
                 - version
@@ -93,7 +106,9 @@ spec:
                 type: object
                 x-kubernetes-map-type: atomic
               identityRef:
-                description: AWSIdentityReference specifies a identity.
+                description: |-
+                  IdentityRef is a reference to an identity to be used when reconciling the ROSA Role Config.
+                  If no identity is specified, the default identity for this controller will be used.
                 properties:
                   kind:
                     description: Kind of the identity.
@@ -110,43 +125,59 @@ spec:
                 - kind
                 - name
                 type: object
+              oidcProviderType:
+                default: Managed
+                description: OIDC provider type values are Managed or UnManaged. When
+                  set to UnManged OperatorRoleConfig OIDCID field must be provided.
+                enum:
+                - Managed
+                - UnManaged
+                type: string
               operatorRoleConfig:
                 description: OperatorRoleConfig defines cluster-specific operator
                   IAM roles based on your cluster configuration.
                 properties:
                   oidcID:
                     description: |-
                       OIDCID is the ID of the OIDC config that will be used to create the operator roles.
-                      A managed OIDC-provider will be created if the OIDCID not specified
+                      Cannot be set when OidcProviderType set to Managed
                     type: string
+                    x-kubernetes-validations:
+                    - message: oidcID is immutable
+                      rule: self == oldSelf
                   permissionsBoundaryARN:
                     description: The ARN of the policy that is used to set the permissions
                       boundary for the operator roles.
                     type: string
                   prefix:
                     description: ' User-defined prefix for generated AWS operator
-                      policies.'
+                      roles.'
                     maxLength: 4
+                    pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$
                     type: string
+                    x-kubernetes-validations:
+                    - message: prefix is immutable
+                      rule: self == oldSelf
                   sharedVPCConfig:
                     description: SharedVPCConfig is used to set up shared VPC.
                     properties:
                       routeRoleARN:
-                        description: ' Role ARN associated with the private hosted
-                          zone used for Hosted Control Plane cluster shared VPC, this
-                          role contains policies to be used with Route 53'
+                        description: Role ARN associated with the private hosted zone
+                          used for Hosted Control Plane cluster shared VPC, this role
+                          contains policies to be used with Route 53
                         type: string
                       vpcEndpointRoleArn:
-                        description: ' Role ARN associated with the shared VPC used
+                        description: Role ARN associated with the shared VPC used
                           for Hosted Control Plane clusters, this role contains policies
-                          to be used with the VPC endpoint'
+                          to be used with the VPC endpoint
                         type: string
                     type: object
                 required:
                 - prefix
                 type: object
             required:
             - accountRoleConfig
+            - oidcProviderType
             - operatorRoleConfig
             type: object
           status:
@@ -170,8 +201,7 @@ spec:
                     type: string
                 type: object
               conditions:
-                description: Conditions provide observations of the operational state
-                  of a Cluster API resource.
+                description: Conditions specifies the ROSARoleConfig conditions
                 items:
                   description: Condition defines an observation of a Cluster API resource
                     operational state.

config/crd/kustomization.yaml

@@ -42,7 +42,6 @@ patchesStrategicMerge:
 - patches/webhook_in_awsmanagedcontrolplanetemplates.yaml
 - patches/webhook_in_eksconfigs.yaml
 - patches/webhook_in_eksconfigtemplates.yaml
-- patches/webhook_in_rosaroleconfigs.yaml
   # +kubebuilder:scaffold:crdkustomizewebhookpatch
 
 # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix.

config/crd/patches/webhook_in_rosaroleconfigs.yaml

@@ -135,12 +135,6 @@ rules:
   - patch
   - update
   - watch
-- apiGroups:
-  - controlplane.cluster.x-k8s.io
-  resources:
-  - rosacontrolplanes/finalizers
-  verbs:
-  - update
 - apiGroups:
   - infrastructure.cluster.x-k8s.io
   resources:
@@ -169,7 +163,6 @@ rules:
   - awsmanagedclusters
   - awsmanagedmachinepools
   - rosaclusters
-  - rosamachinepools
   verbs:
   - delete
   - get
@@ -183,7 +176,6 @@ rules:
   - awsclusters/status
   - awsfargateprofiles/status
   - rosaclusters/status
-  - rosamachinepools/status
   - rosaroleconfigs/status
   verbs:
   - get
@@ -207,6 +199,7 @@ rules:
   resources:
   - awsmachines
   - rosamachinepools
+  - rosaroleconfigs
   verbs:
   - create
   - delete
@@ -222,3 +215,14 @@ rules:
   - rosaroleconfigs/finalizers
   verbs:
   - update
+- apiGroups:
+  - infrastructure.cluster.x-k8s.io
+  resources:
+  - rosamachinepools/status
+  verbs:
+  - create
+  - get
+  - list
+  - patch
+  - update
+  - watch

config/rbac/role.yaml

@@ -95,10 +95,19 @@ func TestRosaClusterReconcile(t *testing.T) {
 					PodCIDR:     "10.128.0.0/14",
 					ServiceCIDR: "172.30.0.0/16",
 				},
-				Region:           "us-east-1",
-				Version:          "4.19.20",
-				ChannelGroup:     "stable",
-				RolesRef:         rosacontrolplanev1.AWSRolesRef{},
+				Region:       "us-east-1",
+				Version:      "4.19.20",
+				ChannelGroup: "stable",
+				RolesRef: rosacontrolplanev1.AWSRolesRef{
+					IngressARN:              "ingress-arn",
+					ImageRegistryARN:        "image-arn",
+					StorageARN:              "storage-arn",
+					NetworkARN:              "net-arn",
+					KubeCloudControllerARN:  "kube-arn",
+					NodePoolManagementARN:   "node-arn",
+					ControlPlaneOperatorARN: "control-arn",
+					KMSProviderARN:          "kms-arn",
+				},
 				OIDCID:           "oidcid1",
 				InstallerRoleARN: "arn1",
 				WorkerRoleARN:    "arn2",

controllers/rosacluster_controller_test.go

@@ -126,8 +126,8 @@ type RosaControlPlaneSpec struct { //nolint: maligned
 	// +kubebuilder:default=WaitForAcknowledge
 	VersionGate VersionGateAckType `json:"versionGate"`
 
-	// RosaRoleConfigRef is a reference to a RosaRoleConfig resource that contains account and operator roles and OIDC configuration.
-	// If specified, the roles and OIDC configuration will be taken from the referenced RosaRoleConfig instead of the direct fields.
+	// RosaRoleConfigRef is a reference to a RosaRoleConfig resource that contains account roles, operator roles and OIDC configuration.
+	// RosaRoleConfigRef and role fields such as installerRoleARN, supportRoleARN, workerRoleARN, rolesRef and oidcID are mutually exclusive.
 	//
 	// +optional
 	RosaRoleConfigRef *corev1.LocalObjectReference `json:"rosaRoleConfigRef,omitempty"`

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go

@@ -31,9 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-// log is for logging in this package.
-var rosacpLog = ctrl.Log.WithName("rosacontrolplane-resource")
-
 // SetupWebhookWithManager will setup the webhooks for the ROSAControlPlane.
 func (r *ROSAControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	w := new(rosaControlPlaneWebhook)
@@ -124,6 +121,10 @@ func (*rosaControlPlaneWebhook) ValidateUpdate(_ context.Context, oldObj, newObj
 		allErrs = append(allErrs, err)
 	}
 
+	if err := r.validateRosaRoleConfig(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	allErrs = append(allErrs, r.validateNetwork()...)
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 
@@ -203,14 +204,14 @@ func (r *ROSAControlPlane) validateExternalAuthProviders() *field.Error {
 }
 
 func (r *ROSAControlPlane) validateRosaRoleConfig() *field.Error {
-	hasAnyDirectRoleFields := r.Spec.OIDCID != "" || r.Spec.InstallerRoleARN != "" || r.Spec.SupportRoleARN != "" || r.Spec.WorkerRoleARN != "" ||
+	hasRoleFields := r.Spec.OIDCID != "" || r.Spec.InstallerRoleARN != "" || r.Spec.SupportRoleARN != "" || r.Spec.WorkerRoleARN != "" ||
 		r.Spec.RolesRef.IngressARN != "" || r.Spec.RolesRef.ImageRegistryARN != "" || r.Spec.RolesRef.StorageARN != "" ||
 		r.Spec.RolesRef.NetworkARN != "" || r.Spec.RolesRef.KubeCloudControllerARN != "" || r.Spec.RolesRef.NodePoolManagementARN != "" ||
 		r.Spec.RolesRef.ControlPlaneOperatorARN != "" || r.Spec.RolesRef.KMSProviderARN != ""
 
 	if r.Spec.RosaRoleConfigRef != nil {
-		if hasAnyDirectRoleFields {
-			rosacpLog.Info("rosaRoleConfigRef and direct role fields (oidcID, installerRoleARN, supportRoleARN, workerRoleARN, rolesRef) are mutually exclusive")
+		if hasRoleFields {
+			return field.Invalid(field.NewPath("spec.rosaRoleConfigRef"), r.Spec.RosaRoleConfigRef, "RosaRoleConfigRef and role fields such as installerRoleARN, supportRoleARN, workerRoleARN, rolesRef and oidcID are mutually exclusive")
 		}
 		return nil
 	}