Revert "CNTRLPLANE-2205: Auto-detect shared role use and enable for e2e"

cmd/cluster/aws/destroy.go

@@ -105,6 +105,7 @@ func destroyPlatformSpecifics(ctx context.Context, o *core.DestroyOptions) error
 			CredentialsSecretData:        secretData,
 			VPCOwnerCredentialsOpts:      o.AWSPlatform.VPCOwnerCredentials,
 			PrivateZonesInClusterAccount: o.AWSPlatform.PrivateZonesInClusterAccount,
+			SharedRole:                   o.AWSPlatform.SharedRole,
 		}
 		if err := destroyOpts.Run(ctx); err != nil {
 			return fmt.Errorf("failed to destroy IAM: %w", err)

cmd/infra/aws/destroy_iam.go

@@ -29,6 +29,7 @@ type DestroyIAMOptions struct {
 
 	VPCOwnerCredentialsOpts      awsutil.AWSCredentialsOptions
 	PrivateZonesInClusterAccount bool
+	SharedRole                   bool
 
 	CredentialsSecretData *util.CredentialsSecretData
 }
@@ -49,6 +50,7 @@ func NewDestroyIAMCommand() *cobra.Command {
 	cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, "Infrastructure ID to use for AWS resources.")
 	cmd.Flags().StringVar(&opts.Region, "region", opts.Region, "Region where cluster infra lives")
 	cmd.Flags().BoolVar(&opts.PrivateZonesInClusterAccount, "private-zones-in-cluster-account", opts.PrivateZonesInClusterAccount, "In shared VPC infrastructure, delete roles for private hosted zones from cluster account")
+	cmd.Flags().BoolVar(&opts.SharedRole, "shared-role", opts.SharedRole, "Delete the shared role instead of individual component roles")
 
 	opts.AWSCredentialsOpts.BindFlags(cmd.Flags())
 	opts.VPCOwnerCredentialsOpts.BindVPCOwnerFlags(cmd.Flags())
@@ -146,67 +148,64 @@ func (o *DestroyIAMOptions) DestroyOIDCResources(ctx context.Context, iamClient
 		}
 	}
 
-	// Delete the shared role
-	removed := false
-	if removed, err = o.DestroyOIDCRole(iamClient, "shared-role"); err != nil {
-		return err
-	}
-	if removed {
-		// The cluster was created with a single shared role, so we are done.
-		// Save on additional API calls and just return here.
-		return nil
-	}
-	// Delete individual component roles
-	if _, err = o.DestroyOIDCRole(iamClient, "openshift-ingress"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "openshift-image-registry"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "aws-ebs-csi-driver-controller"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "cloud-controller"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "node-pool"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "control-plane-operator"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "cloud-network-config-controller"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "kms-provider"); err != nil {
-		return err
-	}
-	if _, err = o.DestroyOIDCRole(iamClient, "karpenter"); err != nil {
-		return err
+	if o.SharedRole {
+		// Delete the shared role
+		if err = o.DestroyOIDCRole(iamClient, "shared-role", false); err != nil {
+			return err
+		}
+	} else {
+		// Delete individual component roles
+		if err = o.DestroyOIDCRole(iamClient, "openshift-ingress", true); err != nil {
+			return err
+		}
+		if err = o.DestroyOIDCRole(iamClient, "openshift-image-registry", false); err != nil {
+			return err
+		}
+		if err = o.DestroyOIDCRole(iamClient, "aws-ebs-csi-driver-controller", false); err != nil {
+			return err
+		}
+		if err = o.DestroyOIDCRole(iamClient, "cloud-controller", false); err != nil {
+			return err
+		}
+		if err = o.DestroyOIDCRole(iamClient, "node-pool", false); err != nil {
+			return err
+		}
+		if err = o.DestroyOIDCRole(iamClient, "control-plane-operator", true); err != nil {
+			return err
+		}
+		if err := o.DestroyOIDCRole(iamClient, "cloud-network-config-controller", false); err != nil {
+			return err
+		}
+		if err := o.DestroyOIDCRole(iamClient, "kms-provider", false); err != nil {
+			return err
+		}
+		if err := o.DestroyOIDCRole(iamClient, "karpenter", false); err != nil {
+			return err
+		}
 	}
 
 	return nil
 }
 
 // DestroyOIDCRole deletes an IAM Role with all its policies
-func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string) (removed bool, reterr error) {
+func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string, includeAssumePolicy bool) error {
 	roleName := fmt.Sprintf("%s-%s", o.InfraID, name)
 	role, err := existingRole(client, roleName)
 	if err != nil {
-		return false, fmt.Errorf("cannot check for existing role: %w", err)
+		return fmt.Errorf("cannot check for existing role: %w", err)
 	}
 
 	if role == nil {
 		o.Log.Info("Role already deleted!", "role", roleName)
-		return false, nil
+		return nil
 	}
 
 	// Detach managed policies
 	attachedPolicies, err := client.ListAttachedRolePolicies(&iam.ListAttachedRolePoliciesInput{
 		RoleName: aws.String(roleName),
 	})
 	if err != nil {
-		return false, fmt.Errorf("failed to list attached policies for role %s: %w", roleName, err)
+		return fmt.Errorf("failed to list attached policies for role %s: %w", roleName, err)
 	}
 
 	for _, policy := range attachedPolicies.AttachedPolicies {
@@ -215,7 +214,7 @@ func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string)
 			RoleName:  aws.String(roleName),
 		})
 		if err != nil {
-			return false, fmt.Errorf("failed to detach policy %s from role %s: %w", *policy.PolicyArn, roleName, err)
+			return fmt.Errorf("failed to detach policy %s from role %s: %w", *policy.PolicyArn, roleName, err)
 		}
 		o.Log.Info("Detached role policy", "role", roleName, "policy", *policy.PolicyArn)
 	}
@@ -225,7 +224,7 @@ func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string)
 		RoleName: aws.String(roleName),
 	})
 	if err != nil {
-		return false, fmt.Errorf("failed to list inline policies for role %s: %w", roleName, err)
+		return fmt.Errorf("failed to list inline policies for role %s: %w", roleName, err)
 	}
 
 	for _, policyName := range listPoliciesOutput.PolicyNames {
@@ -237,11 +236,11 @@ func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string)
 			if aerr, ok := err.(awserr.Error); ok {
 				if aerr.Code() != iam.ErrCodeNoSuchEntityException {
 					o.Log.Error(aerr, "Error deleting role policy", "role", roleName, "policy", *policyName)
-					return false, aerr
+					return aerr
 				}
 			} else {
 				o.Log.Error(err, "Error deleting role policy", "role", roleName, "policy", *policyName)
-				return false, err
+				return err
 			}
 		} else {
 			o.Log.Info("Deleted role policy", "role", roleName, "policy", *policyName)
@@ -253,11 +252,11 @@ func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string)
 		RoleName: aws.String(roleName),
 	})
 	if err != nil {
-		return false, fmt.Errorf("failed to delete role %s: %w", roleName, err)
+		return fmt.Errorf("failed to delete role %s: %w", roleName, err)
 	}
 	o.Log.Info("Deleted role", "role", roleName)
 
-	return true, nil
+	return nil
 }
 
 func (o *DestroyIAMOptions) DestroyWorkerInstanceProfile(client iamiface.IAMAPI) error {
@@ -358,10 +357,10 @@ func (o *DestroyIAMOptions) DestroySharedVPCRoles(ctx context.Context, iamClient
 	if o.PrivateZonesInClusterAccount {
 		ingressRoleClient = iamClient
 	}
-	if _, err = o.DestroyOIDCRole(ingressRoleClient, "shared-vpc-ingress"); err != nil {
+	if err = o.DestroyOIDCRole(ingressRoleClient, "shared-vpc-ingress", true); err != nil {
 		return err
 	}
-	if _, err = o.DestroyOIDCRole(vpcOwnerIAMClient, "shared-vpc-control-plane"); err != nil {
+	if err = o.DestroyOIDCRole(vpcOwnerIAMClient, "shared-vpc-control-plane", true); err != nil {
 		return err
 	}
 	return nil

cmd/infra/aws/iam.go

@@ -1029,31 +1029,18 @@ func (o *CreateIAMOptions) CreateSharedOIDCRole(ctx context.Context, client iami
 		return "", fmt.Errorf("failed to create shared role: %w", err)
 	}
 
-	// Add all policies to the shared role
+	// Add all policies to the shared role as inline policies
 	for _, binding := range bindings {
-		if o.UseROSAManagedPolicies && binding.rosaManagedPolicyARN != "" {
-			// Attach ROSA managed policy
-			_, err = client.AttachRolePolicyWithContext(ctx, &iam.AttachRolePolicyInput{
-				PolicyArn: aws.String(binding.rosaManagedPolicyARN),
-				RoleName:  aws.String(roleName),
-			})
-			if err != nil {
-				return "", fmt.Errorf("failed to attach managed policy %q to shared role: %w", binding.rosaManagedPolicyARN, err)
-			}
-			logger.Info("Attached managed policy to shared role", "policy", binding.rosaManagedPolicyARN, "role", roleName)
-		} else {
-			// Add inline policy
-			policyName := fmt.Sprintf("%s-%s", roleName, binding.name)
-			_, err = client.PutRolePolicyWithContext(ctx, &iam.PutRolePolicyInput{
-				PolicyName:     aws.String(policyName),
-				PolicyDocument: aws.String(binding.policy),
-				RoleName:       aws.String(roleName),
-			})
-			if err != nil {
-				return "", fmt.Errorf("failed to add policy %q to shared role: %w", binding.name, err)
-			}
-			logger.Info("Added inline policy to shared role", "policy", binding.name, "role", roleName)
+		policyName := fmt.Sprintf("%s-%s", roleName, binding.name)
+		_, err = client.PutRolePolicyWithContext(ctx, &iam.PutRolePolicyInput{
+			PolicyName:     aws.String(policyName),
+			PolicyDocument: aws.String(binding.policy),
+			RoleName:       aws.String(roleName),
+		})
+		if err != nil {
+			return "", fmt.Errorf("failed to add policy %q to shared role: %w", binding.name, err)
 		}
+		logger.Info("Added policy to shared role", "policy", binding.name, "role", roleName)
 	}
 
 	// Add assume role policy if needed

test/e2e/create_cluster_test.go

@@ -2313,10 +2313,6 @@ func TestCreateCluster(t *testing.T) {
 	if !e2eutil.IsLessThan(e2eutil.Version418) {
 		clusterOpts.FeatureSet = string(configv1.TechPreviewNoUpgrade)
 	}
-	if globalOpts.Platform == hyperv1.AWSPlatform {
-		// Use this test cluster to validate per-component roles
-		clusterOpts.AWSPlatform.SharedRole = false
-	}
 
 	clusterOpts.PodsLabels = map[string]string{
 		"hypershift-e2e-test-label": "test",

test/e2e/util/options.go

@@ -284,7 +284,6 @@ func (o *Options) DefaultAWSOptions() hypershiftaws.RawCreateOptions {
 		MultiArch:              o.ConfigurableClusterOptions.AWSMultiArch,
 		PublicOnly:             true,
 		UseROSAManagedPolicies: true,
-		SharedRole:             true,
 	}
 	if IsLessThan(semver.MustParse("4.16.0")) {
 		opts.PublicOnly = false