diff --git a/cmd/cluster/aws/create.go b/cmd/cluster/aws/create.go index 1ba3266101ba..cf7211db82f4 100644 --- a/cmd/cluster/aws/create.go +++ b/cmd/cluster/aws/create.go @@ -60,6 +60,7 @@ type RawCreateOptions struct { PublicOnly bool AutoNode bool UseROSAManagedPolicies bool + SharedRole bool } // validatedCreateOptions is a private wrapper that enforces a call of Validate() before Complete() can be invoked. @@ -502,6 +503,7 @@ func bindCoreOptions(opts *RawCreateOptions, flags *flag.FlagSet) { flags.BoolVar(&opts.PrivateZonesInClusterAccount, "private-zones-in-cluster-account", opts.PrivateZonesInClusterAccount, "In shared VPC infrastructure, create private hosted zones in cluster account") flags.BoolVar(&opts.PublicOnly, "public-only", opts.PublicOnly, "If true, creates a cluster that does not have private subnets or NAT gateway and assigns public IPs to all instances.") flags.BoolVar(&opts.UseROSAManagedPolicies, "use-rosa-managed-policies", opts.UseROSAManagedPolicies, "Use ROSA managed policies for the operator roles and worker instance profile") + flags.BoolVar(&opts.SharedRole, "shared-role", opts.SharedRole, "Create a single shared role with all role policies instead of individual component roles") _ = flags.MarkDeprecated("multi-arch", "Multi-arch validation is now performed automatically based on the release image and signaled in the HostedCluster.Status.PayloadArch.") } @@ -581,6 +583,7 @@ func CreateIAMOptions(awsOpts *ValidatedCreateOptions, infra *awsinfra.CreateInf PrivateZonesInClusterAccount: awsOpts.PrivateZonesInClusterAccount, CreateKarpenterRoleARN: awsOpts.AutoNode, UseROSAManagedPolicies: awsOpts.UseROSAManagedPolicies, + SharedRole: awsOpts.SharedRole, } } diff --git a/cmd/cluster/aws/destroy.go b/cmd/cluster/aws/destroy.go index 602d9f1f26ce..c7f1dc51370b 100644 --- a/cmd/cluster/aws/destroy.go +++ b/cmd/cluster/aws/destroy.go @@ -34,6 +34,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { cmd.Flags().StringVar(&opts.CredentialSecretName, "secret-creds", opts.CredentialSecretName, "A Kubernetes secret with a platform credential, pull-secret and base-domain. The secret must exist in the supplied \"--namespace\"") cmd.Flags().DurationVar(&opts.AWSPlatform.AwsInfraGracePeriod, "aws-infra-grace-period", opts.AWSPlatform.AwsInfraGracePeriod, "Timeout for destroying infrastructure in minutes") cmd.Flags().BoolVar(&opts.AWSPlatform.PrivateZonesInClusterAccount, "private-zones-in-cluster-account", opts.AWSPlatform.PrivateZonesInClusterAccount, "In shared VPC infrastructure, delete private hosted zones in cluster account") + cmd.Flags().BoolVar(&opts.AWSPlatform.SharedRole, "shared-role", opts.AWSPlatform.SharedRole, "Delete the shared role instead of individual component roles") opts.AWSPlatform.Credentials.BindFlags(cmd.Flags()) opts.AWSPlatform.VPCOwnerCredentials.BindVPCOwnerFlags(cmd.Flags()) @@ -104,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) diff --git a/cmd/cluster/core/destroy.go b/cmd/cluster/core/destroy.go index d37059abec1f..21bcc1cb979f 100644 --- a/cmd/cluster/core/destroy.go +++ b/cmd/cluster/core/destroy.go @@ -56,6 +56,7 @@ type AWSPlatformDestroyOptions struct { AwsInfraGracePeriod time.Duration VPCOwnerCredentials awsutil.AWSCredentialsOptions PrivateZonesInClusterAccount bool + SharedRole bool } type AzurePlatformDestroyOptions struct { diff --git a/cmd/infra/aws/create_iam.go b/cmd/infra/aws/create_iam.go index 4c553f10437a..7ea9662a66ac 100644 --- a/cmd/infra/aws/create_iam.go +++ b/cmd/infra/aws/create_iam.go @@ -47,6 +47,7 @@ type CreateIAMOptions struct { additionalIAMTags []*iam.Tag CreateKarpenterRoleARN bool UseROSAManagedPolicies bool + SharedRole bool } type CreateIAMOutput struct { @@ -90,6 +91,7 @@ func NewCreateIAMCommand() *cobra.Command { cmd.Flags().BoolVar(&opts.PrivateZonesInClusterAccount, "private-zones-in-cluster-account", opts.PrivateZonesInClusterAccount, "In shared VPC infrastructure, create private hosted zones in cluster account") cmd.Flags().BoolVar(&opts.UseROSAManagedPolicies, "use-rosa-managed-policies", opts.UseROSAManagedPolicies, "Use ROSA managed policies for the operator roles and worker instance profile") cmd.Flags().StringVar(&opts.BaseDomain, "base-domain", opts.BaseDomain, "The ingress base domain for the cluster") + cmd.Flags().BoolVar(&opts.SharedRole, "shared-role", opts.SharedRole, "Create a single shared role with all role policies instead of individual component roles") opts.AWSCredentialsOpts.BindFlags(cmd.Flags()) opts.VPCOwnerCredentialsOpts.BindVPCOwnerFlags(cmd.Flags()) diff --git a/cmd/infra/aws/destroy_iam.go b/cmd/infra/aws/destroy_iam.go index 19d948d1b151..c536247c3e55 100644 --- a/cmd/infra/aws/destroy_iam.go +++ b/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()) @@ -145,38 +147,47 @@ func (o *DestroyIAMOptions) DestroyOIDCResources(ctx context.Context, iamClient break } } - 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 + + 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 } -// CreateOIDCRole create an IAM Role with a trust policy for the OIDC provider +// DestroyOIDCRole deletes an IAM Role with all its policies 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) @@ -208,32 +219,35 @@ func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string, o.Log.Info("Detached role policy", "role", roleName, "policy", *policy.PolicyArn) } - // Delete inline policies - policyNames := []string{ - roleName, - } - if includeAssumePolicy { - policyNames = append(policyNames, fmt.Sprintf("%s-assume", roleName)) + // List and delete all inline policies + listPoliciesOutput, err := client.ListRolePolicies(&iam.ListRolePoliciesInput{ + RoleName: aws.String(roleName), + }) + if err != nil { + return fmt.Errorf("failed to list inline policies for role %s: %w", roleName, err) } - for _, policyName := range policyNames { + + for _, policyName := range listPoliciesOutput.PolicyNames { _, err := client.DeleteRolePolicy(&iam.DeleteRolePolicyInput{ - PolicyName: aws.String(policyName), + PolicyName: policyName, RoleName: aws.String(roleName), }) if err != nil { if aerr, ok := err.(awserr.Error); ok { if aerr.Code() != iam.ErrCodeNoSuchEntityException { - o.Log.Error(aerr, "Error deleting role policy", "role", roleName, "policy", policyName) + o.Log.Error(aerr, "Error deleting role policy", "role", roleName, "policy", *policyName) return aerr } } else { - o.Log.Error(err, "Error deleting role policy", "role", roleName, "policy", policyName) + o.Log.Error(err, "Error deleting role policy", "role", roleName, "policy", *policyName) return err } } else { - o.Log.Info("Deleted role policy", "role", roleName, "policy", policyName) + o.Log.Info("Deleted role policy", "role", roleName, "policy", *policyName) } } + + // Delete the role _, err = client.DeleteRole(&iam.DeleteRoleInput{ RoleName: aws.String(roleName), }) diff --git a/cmd/infra/aws/iam.go b/cmd/infra/aws/iam.go index df146344fdaa..3873f03f645e 100644 --- a/cmd/infra/aws/iam.go +++ b/cmd/infra/aws/iam.go @@ -869,13 +869,26 @@ func (o *CreateIAMOptions) CreateOIDCResources(ctx context.Context, iamClient ia bindings[&output.KMSProviderRoleARN] = kmsProviderPolicy(o.KMSKeyARN) } - for into, binding := range bindings { - trustPolicy := oidcTrustPolicy(providerARN, providerName, binding.serviceAccounts...) - arn, err := o.CreateOIDCRole(ctx, iamClient, binding, trustPolicy, logger) + if o.SharedRole { + // Create a single shared role with all policies + sharedRoleARN, err := o.CreateSharedOIDCRole(ctx, iamClient, bindings, providerARN, providerName, logger) if err != nil { - return nil, fmt.Errorf("failed to create OIDC Role %q: with trust policy %s and permission policy %s: %v", binding.name, trustPolicy, binding.policy, err) + return nil, fmt.Errorf("failed to create shared OIDC role: %v", err) + } + // Set all role ARNs to the shared role ARN + for into := range bindings { + *into = sharedRoleARN + } + } else { + // Create individual roles for each component + for into, binding := range bindings { + trustPolicy := oidcTrustPolicy(providerARN, providerName, binding.serviceAccounts...) + arn, err := o.CreateOIDCRole(ctx, iamClient, binding, trustPolicy, logger) + if err != nil { + return nil, fmt.Errorf("failed to create OIDC Role %q: with trust policy %s and permission policy %s: %v", binding.name, trustPolicy, binding.policy, err) + } + *into = arn } - *into = arn } if o.UseROSAManagedPolicies { @@ -977,6 +990,77 @@ func (o *CreateIAMOptions) CreateOIDCRole(ctx context.Context, client iamiface.I return createIAMRoleOpts.CreateRoleWithInlinePolicy(ctx, client, logger) } +// CreateSharedOIDCRole creates a single IAM Role with all policies from the bindings +func (o *CreateIAMOptions) CreateSharedOIDCRole(ctx context.Context, client iamiface.IAMAPI, bindings map[*string]policyBinding, providerARN, providerName string, logger logr.Logger) (string, error) { + // Collect all service accounts from all bindings + var allServiceAccounts []string + serviceAccountsMap := make(map[string]bool) + for _, binding := range bindings { + for _, sa := range binding.serviceAccounts { + if !serviceAccountsMap[sa] { + serviceAccountsMap[sa] = true + allServiceAccounts = append(allServiceAccounts, sa) + } + } + } + + // Create a combined trust policy with all service accounts + trustPolicy := oidcTrustPolicy(providerARN, providerName, allServiceAccounts...) + + // Create the shared role + roleName := fmt.Sprintf("%s-shared-role", o.InfraID) + createIAMRoleOpts := CreateIAMRoleOptions{ + RoleName: roleName, + TrustPolicy: trustPolicy, + additionalIAMTags: o.additionalIAMTags, + AllowAssume: false, + } + + // Check if any binding has allowAssumeRole set + for _, binding := range bindings { + if binding.allowAssumeRole { + createIAMRoleOpts.AllowAssume = true + break + } + } + + arn, err := createIAMRoleOpts.CreateRole(ctx, client, logger) + if err != nil { + return "", fmt.Errorf("failed to create shared role: %w", err) + } + + // Add all policies to the shared role as inline policies + for _, binding := range bindings { + 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 + if createIAMRoleOpts.AllowAssume { + assumePolicyName := fmt.Sprintf("%s-assume", roleName) + _, err = client.PutRolePolicyWithContext(ctx, &iam.PutRolePolicyInput{ + PolicyName: aws.String(assumePolicyName), + PolicyDocument: aws.String(allowAssumeRolePolicy), + RoleName: aws.String(roleName), + }) + if err != nil { + return "", fmt.Errorf("failed to add assume role policy to shared role: %w", err) + } + logger.Info("Added assume role policy to shared role", "role", roleName) + } + + logger.Info("Created shared role", "name", roleName, "arn", arn) + return arn, nil +} + func (o *CreateIAMOptions) CreateWorkerInstanceProfile(client iamiface.IAMAPI, profileName string, logger logr.Logger) error { const ( assumeRolePolicy = `{ diff --git a/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go b/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go index 3ea1a5d5ed09..47ba9a8b6a53 100644 --- a/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go +++ b/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go @@ -298,14 +298,19 @@ func (p AWS) ReconcileCredentials(ctx context.Context, c client.Client, createOr } return nil } - for arn, secret := range map[string]*corev1.Secret{ - hcluster.Spec.Platform.AWS.RolesRef.KubeCloudControllerARN: KubeCloudControllerCredsSecret(controlPlaneNamespace), - hcluster.Spec.Platform.AWS.RolesRef.NodePoolManagementARN: NodePoolManagementCredsSecret(controlPlaneNamespace), - hcluster.Spec.Platform.AWS.RolesRef.ControlPlaneOperatorARN: ControlPlaneOperatorCredsSecret(controlPlaneNamespace), - hcluster.Spec.Platform.AWS.RolesRef.NetworkARN: CloudNetworkConfigControllerCredsSecret(controlPlaneNamespace), - hcluster.Spec.Platform.AWS.RolesRef.StorageARN: AWSEBSCSIDriverCredsSecret(controlPlaneNamespace), - } { - if err := syncSecret(secret, arn); err != nil { + // Use a slice instead of a map to support shared roles where multiple secrets may have the same ARN + secretsToSync := []struct { + arn string + secret *corev1.Secret + }{ + {hcluster.Spec.Platform.AWS.RolesRef.KubeCloudControllerARN, KubeCloudControllerCredsSecret(controlPlaneNamespace)}, + {hcluster.Spec.Platform.AWS.RolesRef.NodePoolManagementARN, NodePoolManagementCredsSecret(controlPlaneNamespace)}, + {hcluster.Spec.Platform.AWS.RolesRef.ControlPlaneOperatorARN, ControlPlaneOperatorCredsSecret(controlPlaneNamespace)}, + {hcluster.Spec.Platform.AWS.RolesRef.NetworkARN, CloudNetworkConfigControllerCredsSecret(controlPlaneNamespace)}, + {hcluster.Spec.Platform.AWS.RolesRef.StorageARN, AWSEBSCSIDriverCredsSecret(controlPlaneNamespace)}, + } + for _, item := range secretsToSync { + if err := syncSecret(item.secret, item.arn); err != nil { errs = append(errs, err) } }