Review fixes

Author: PanSpagetka

controlplane/rosa/api/v1beta2/rosacontrolplane_webhook.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package v1beta2
 
 import (

exp/api/v1beta2/rosaroleconfig_types.go

@@ -1,5 +1,5 @@
 /*
-Copyright The Kubernetes Authors.
+Copyright 2025 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -25,9 +25,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
-// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
-
 // ROSARoleConfigSpec defines the desired state of ROSARoleConfig
 type ROSARoleConfigSpec struct {
 	AccountRoleConfig  AccountRoleConfig             `json:"accountRoleConfig"`
@@ -158,6 +155,25 @@ const (
 	RosaRoleConfigCreatedReason = "Created"
 )
 
+const (
+	// IngressOperatorARNSuffix is the suffix for the ingress operator role.
+	IngressOperatorARNSuffix = "openshift-ingress-operator-cloud-credentials"
+	// ImageRegistryARNSuffix is the suffix for the image registry operator role.
+	ImageRegistryARNSuffix = "openshift-image-registry-installer-cloud-credentials"
+	// StorageARNSuffix is the suffix for the storage operator role.
+	StorageARNSuffix = "openshift-cluster-csi-drivers-ebs-cloud-credentials"
+	// NetworkARNSuffix is the suffix for the network operator role.
+	NetworkARNSuffix = "openshift-cloud-network-config-controller-cloud-credentials"
+	// KubeCloudControllerARNSuffix is the suffix for the kube cloud controller role.
+	KubeCloudControllerARNSuffix = "kube-system-kube-controller-manager"
+	// NodePoolManagementARNSuffix is the suffix for the node pool management role.
+	NodePoolManagementARNSuffix = "kube-system-capa-controller-manager"
+	// ControlPlaneOperatorARNSuffix is the suffix for the control plane operator role.
+	ControlPlaneOperatorARNSuffix = "kube-system-control-plane-operator"
+	// KMSProviderARNSuffix is the suffix for the kms provider role.
+	KMSProviderARNSuffix = "kube-system-kms-provider"
+)
+
 // SetConditions sets the conditions of the ROSARoleConfig.
 func (r *ROSARoleConfig) SetConditions(conditions clusterv1.Conditions) {
 	r.Status.Conditions = conditions
@@ -168,6 +184,11 @@ func (r *ROSARoleConfig) GetConditions() clusterv1.Conditions {
 	return r.Status.Conditions
 }
 
+// IsSharedVPC checks if the shared VPC config is set.
+func (s SharedVPCConfig) IsSharedVPC() bool {
+	return s.VPCEndpointRoleARN != "" && s.RouteRoleARN != ""
+}
+
 func init() {
 	SchemeBuilder.Register(&ROSARoleConfig{}, &ROSARoleConfigList{})
 }

exp/controllers/rosamachinepool_controller.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controllers
 
 import (

exp/controllers/rosaroleconfig_controller.go

@@ -1,5 +1,5 @@
 /*
-Copyright The Kubernetes Authors.
+Copyright 2025 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -93,6 +93,7 @@ func (r *ROSARoleConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		if apierrors.IsNotFound(err) {
 			return ctrl.Result{}, nil
 		}
+		log.Error(err, "Failed to get ROSARoleConfig")
 		return ctrl.Result{Requeue: true}, nil
 	}
 
@@ -113,7 +114,7 @@ func (r *ROSARoleConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	defer func() {
 		conditions.SetSummary(scope.RosaRoleConfig, conditions.WithConditions(expinfrav1.RosaRoleConfigReadyCondition), conditions.WithStepCounter())
 
-		if err := scope.Close(); err != nil {
+		if err := scope.PatchObject(); err != nil {
 			reterr = errors.Join(reterr, err)
 		}
 	}()
@@ -134,9 +135,7 @@ func (r *ROSARoleConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	}
 
 	if controllerutil.AddFinalizer(scope.RosaRoleConfig, expinfrav1.RosaRoleConfigFinalizer) {
-		if err := scope.PatchObject(); err != nil {
-			return ctrl.Result{}, err
-		}
+		return ctrl.Result{}, err
 	}
 
 	err = r.createAccountRoles(ctx, roleConfig, scope, ocmClient)
@@ -260,7 +259,6 @@ func (r *ROSARoleConfigReconciler) createOperatorRoles(ctx context.Context, role
 	version := roleConfig.Spec.AccountRoleConfig.Version
 	hostedCp := true
 	forcePolicyCreation := true
-	isSharedVpc := config.SharedVPCConfig.VPCEndpointRoleARN != "" && config.SharedVPCConfig.RouteRoleARN != ""
 
 	operatorRoles, err := runtime.AWSClient.ListOperatorRoles(version, "", config.Prefix)
 
@@ -270,28 +268,35 @@ func (r *ROSARoleConfigReconciler) createOperatorRoles(ctx context.Context, role
 
 	for _, roles := range operatorRoles {
 		for _, role := range roles {
-			if role.RoleName == fmt.Sprintf("%s-openshift-ingress-operator-cloud-credentials", config.Prefix) {
+			roleSuffix := strings.TrimPrefix(role.RoleName, config.Prefix)
+			if roleSuffix == role.RoleName {
+				continue
+			}
+			switch roleSuffix {
+			case expinfrav1.IngressOperatorARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.IngressARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-openshift-image-registry-installer-cloud-credentials", config.Prefix) {
+			case expinfrav1.ImageRegistryARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.ImageRegistryARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-openshift-cluster-csi-drivers-ebs-cloud-credentials", config.Prefix) {
+			case expinfrav1.StorageARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.StorageARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-openshift-cloud-network-config-controller-cloud-credentials", config.Prefix) {
+			case expinfrav1.NetworkARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.NetworkARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-kube-system-kube-controller-manager", config.Prefix) {
+			case expinfrav1.KubeCloudControllerARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.KubeCloudControllerARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-kube-system-capa-controller-manager", config.Prefix) {
+			case expinfrav1.NodePoolManagementARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.NodePoolManagementARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-kube-system-control-plane-operator", config.Prefix) {
+			case expinfrav1.ControlPlaneOperatorARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.ControlPlaneOperatorARN = role.RoleARN
-			} else if role.RoleName == fmt.Sprintf("%s-kube-system-kms-provider", config.Prefix) {
+			case expinfrav1.KMSProviderARNSuffix:
 				scope.RosaRoleConfig.Status.OperatorRolesRef.KMSProviderARN = role.RoleARN
+			default:
+				return fmt.Errorf("unknown role suffix: when listing operator roles %s", roleSuffix)
 			}
 		}
 	}
 
 	if !r.operatorRolesReady(&scope.RosaRoleConfig.Status.OperatorRolesRef) {
-		err = operatorroles.CreateOperatorRoles(runtime, ocm.Production, config.PermissionsBoundaryARN, interactive.ModeAuto, policies, version, isSharedVpc, config.Prefix, hostedCp, installerRoleArn, forcePolicyCreation,
+		err = operatorroles.CreateOperatorRoles(runtime, ocm.Production, config.PermissionsBoundaryARN, interactive.ModeAuto, policies, version, config.SharedVPCConfig.IsSharedVPC(), config.Prefix, hostedCp, installerRoleArn, forcePolicyCreation,
 			oidcConfigID, config.SharedVPCConfig.RouteRoleARN, ocm.DefaultChannelGroup, config.SharedVPCConfig.VPCEndpointRoleARN)
 		return err
 	}
@@ -423,8 +428,7 @@ func (r *ROSARoleConfigReconciler) createAccountRoles(ctx context.Context, roleC
 		}
 
 		managedPolicies := true
-		isSharedVpc := config.SharedVPCConfig.VPCEndpointRoleARN != "" && config.SharedVPCConfig.RouteRoleARN != ""
-		err := accountroles.CreateHCPRoles(runtime, config.Prefix, managedPolicies, config.PermissionsBoundaryARN, ocm.Production, policies, config.Version, config.Path, isSharedVpc, config.SharedVPCConfig.RouteRoleARN, config.SharedVPCConfig.VPCEndpointRoleARN)
+		err := accountroles.CreateHCPRoles(runtime, config.Prefix, managedPolicies, config.PermissionsBoundaryARN, ocm.Production, policies, config.Version, config.Path, config.SharedVPCConfig.IsSharedVPC(), config.SharedVPCConfig.RouteRoleARN, config.SharedVPCConfig.VPCEndpointRoleARN)
 		return err
 	}
 
@@ -471,23 +475,16 @@ func (r *ROSARoleConfigReconciler) deleteAccountRoles(ocmClient *ocm.Client, aws
 		return err
 	}
 
-	var err2, err3 error
 	if canDeleteRole(clusters, roles.InstallerRoleARN) {
-		err = awsClient.DeleteAccountRole(strings.Split(roles.InstallerRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies)
+		err = errors.Join(err, awsClient.DeleteAccountRole(strings.Split(roles.InstallerRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies))
 	}
 	if canDeleteRole(clusters, roles.WorkerRoleARN) {
-		err2 = awsClient.DeleteAccountRole(strings.Split(roles.WorkerRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies)
+		err = errors.Join(err, awsClient.DeleteAccountRole(strings.Split(roles.WorkerRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies))
 	}
 	if canDeleteRole(clusters, roles.SupportRoleARN) {
-		err3 = awsClient.DeleteAccountRole(strings.Split(roles.SupportRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies)
-	}
-	if err != nil {
-		return err
-	}
-	if err2 != nil {
-		return err2
+		err = errors.Join(err, awsClient.DeleteAccountRole(strings.Split(roles.SupportRoleARN, "/")[1], config.Prefix, true, deleteHcpSharedVpcPolicies))
 	}
-	return err3
+	return err
 }
 
 func (r *ROSARoleConfigReconciler) deleteOIDCProvider(ocmClient *ocm.Client, awsClient aws.Client, oidcConfigID string) error {

pkg/cloud/scope/rosaroleconfig.go

@@ -1,5 +1,5 @@
 /*
- Copyright The Kubernetes Authors.
+ Copyright 2025 The Kubernetes Authors.
 
  Licensed under the Apache License, Version 2.0 (the "License");
  you may not use this file except in compliance with the License.
@@ -19,8 +19,8 @@ package scope
 import (
 	"context"
 
-	awsv2 "github.com/aws/aws-sdk-go-v2/aws"
-	iamv2 "github.com/aws/aws-sdk-go-v2/service/iam"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/iam"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -52,8 +52,8 @@ type RosaRoleConfigScope struct {
 	patchHelper     *patch.Helper
 	RosaRoleConfig  *expinfrav1.ROSARoleConfig
 	serviceLimiters throttle.ServiceLimiters
-	session         awsv2.Config
-	iamClient       *iamv2.Client
+	session         aws.Config
+	iamClient       *iam.Client
 }
 
 // NewRosaRoleConfigScope creates a new RosaRoleConfigScope from the supplied parameters.
@@ -71,21 +71,21 @@ func NewRosaRoleConfigScope(params RosaRoleConfigScopeParams) (*RosaRoleConfigSc
 		RosaRoleConfig: params.RosaRoleConfig,
 	}
 
-	sessionv2, serviceLimitersv2, err := sessionForClusterWithRegionV2(params.Client, RosaRoleConfigScope, "", params.Endpoints, params.Logger)
+	session, serviceLimiters, err := sessionForClusterWithRegionV2(params.Client, RosaRoleConfigScope, "", params.Endpoints, params.Logger)
 	if err != nil {
 		return nil, errors.Errorf("failed to create aws V2 session: %v", err)
 	}
 
-	iamClient := iamv2.NewFromConfig(*sessionv2)
+	iamClient := iam.NewFromConfig(*session)
 
 	patchHelper, err := patch.NewHelper(params.RosaRoleConfig, params.Client)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to init patch helper")
 	}
 
 	RosaRoleConfigScope.patchHelper = patchHelper
-	RosaRoleConfigScope.session = *sessionv2
-	RosaRoleConfigScope.serviceLimiters = serviceLimitersv2
+	RosaRoleConfigScope.session = *session
+	RosaRoleConfigScope.serviceLimiters = serviceLimiters
 	RosaRoleConfigScope.iamClient = iamClient
 
 	return RosaRoleConfigScope, nil
@@ -97,7 +97,7 @@ func (s *RosaRoleConfigScope) IdentityRef() *infrav1.AWSIdentityReference {
 }
 
 // Session returns the AWS SDK V2 session. Used for creating clients.
-func (s *RosaRoleConfigScope) Session() awsv2.Config {
+func (s *RosaRoleConfigScope) Session() aws.Config {
 	return s.session
 }
 
@@ -140,15 +140,10 @@ func (s *RosaRoleConfigScope) GetClient() client.Client {
 // PatchObject persists the RosaRoleConfig configuration and status.
 func (s *RosaRoleConfigScope) PatchObject() error {
 	return s.patchHelper.Patch(
-		context.TODO(),
+		context.Background(),
 		s.RosaRoleConfig)
 }
 
-// Close closes the current scope persisting the RosaRoleConfig configuration and status.
-func (s *RosaRoleConfigScope) Close() error {
-	return s.PatchObject()
-}
-
 // CredentialsSecret returns the CredentialsSecret object.
 func (s *RosaRoleConfigScope) CredentialsSecret() *corev1.Secret {
 	secretRef := s.RosaRoleConfig.Spec.CredentialsSecretRef
@@ -165,6 +160,6 @@ func (s *RosaRoleConfigScope) CredentialsSecret() *corev1.Secret {
 }
 
 // IAMClient returns the IAM client.
-func (s *RosaRoleConfigScope) IAMClient() *iamv2.Client {
+func (s *RosaRoleConfigScope) IAMClient() *iam.Client {
 	return s.iamClient
 }

pkg/rosa/client.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 // Package rosa provides a way to interact with the Red Hat OpenShift Service on AWS (ROSA) API.
 package rosa
 

pkg/rosa/externalauthproviders.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package rosa
 
 import (

pkg/rosa/helpers.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package rosa
 
 import (

pkg/rosa/idps.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package rosa
 
 import (