Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/cluster/aws/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.")
}
Expand Down Expand Up @@ -581,6 +583,7 @@ func CreateIAMOptions(awsOpts *ValidatedCreateOptions, infra *awsinfra.CreateInf
PrivateZonesInClusterAccount: awsOpts.PrivateZonesInClusterAccount,
CreateKarpenterRoleARN: awsOpts.AutoNode,
UseROSAManagedPolicies: awsOpts.UseROSAManagedPolicies,
SharedRole: awsOpts.SharedRole,
}
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/cluster/aws/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions cmd/cluster/core/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type AWSPlatformDestroyOptions struct {
AwsInfraGracePeriod time.Duration
VPCOwnerCredentials awsutil.AWSCredentialsOptions
PrivateZonesInClusterAccount bool
SharedRole bool
}

type AzurePlatformDestroyOptions struct {
Expand Down
2 changes: 2 additions & 0 deletions cmd/infra/aws/create_iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type CreateIAMOptions struct {
additionalIAMTags []*iam.Tag
CreateKarpenterRoleARN bool
UseROSAManagedPolicies bool
SharedRole bool
}

type CreateIAMOutput struct {
Expand Down Expand Up @@ -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())
Expand Down
90 changes: 52 additions & 38 deletions cmd/infra/aws/destroy_iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type DestroyIAMOptions struct {

VPCOwnerCredentialsOpts awsutil.AWSCredentialsOptions
PrivateZonesInClusterAccount bool
SharedRole bool

CredentialsSecretData *util.CredentialsSecretData
}
Expand All @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
})
Expand Down
94 changes: 89 additions & 5 deletions cmd/infra/aws/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = `{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down