Skip to content

Commit

Permalink
fix(eks-v2-alpha): can't delete fargate cluster (#33573)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #33347.

### Reason for this change

The fargate cluster deletion issue is because the cluster admin access entry is deleted before deleting `KubernetesPatch`. 

Since deleting `KubernetesPatch` requires applying a restore patch to the cluster, it will still need the cluster access. In this case, because the access entry is deleted, kubectl provider won't be able to apply the patch to the cluster anymore.

### Description of changes

add an explicit dependency from patch to the access entry so the patch will only be deleted after the access entry

### Description of how you validated changes

unit tests/integration tests

### Checklist
- [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
xazhao authored Feb 28, 2025
1 parent 528e5df commit 4ada313
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 946 deletions.
42 changes: 36 additions & 6 deletions packages/@aws-cdk/aws-eks-v2-alpha/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,8 @@ export class Cluster extends ClusterBase {

private readonly _kubectlProvider?: IKubectlProvider;

private readonly _clusterAdminAccess?: AccessEntry;

/**
* Initiates an EKS Cluster with the supplied arguments
*
Expand Down Expand Up @@ -1279,11 +1281,7 @@ export class Cluster extends ClusterBase {

// give the handler role admin access to the cluster
// so it can deploy/query any resource.
this.grantAccess('ClusterAdminRoleAccess', this._kubectlProvider?.role!.roleArn, [
AccessPolicy.fromAccessPolicyName('AmazonEKSClusterAdminPolicy', {
accessScopeType: AccessScopeType.CLUSTER,
}),
]);
this._clusterAdminAccess = this.grantClusterAdmin('ClusterAdminRoleAccess', this._kubectlProvider?.role!.roleArn);
}

// do not create a masters role if one is not provided. Trusting the accountRootPrincipal() is too permissive.
Expand Down Expand Up @@ -1351,6 +1349,32 @@ export class Cluster extends ClusterBase {
this.addToAccessEntry(id, principal, accessPolicies);
}

/**
* Grants the specified IAM principal cluster admin access to the EKS cluster.
*
* This method creates an `AccessEntry` construct that grants the specified IAM principal the cluster admin
* access permissions. This allows the IAM principal to perform the actions permitted
* by the cluster admin acces.
*
* @param id - The ID of the `AccessEntry` construct to be created.
* @param principal - The IAM principal (role or user) to be granted access to the EKS cluster.
* @returns the access entry construct
*/
@MethodMetadata()
public grantClusterAdmin(id: string, principal: string): AccessEntry {
const newEntry = new AccessEntry(this, id, {
principal,
cluster: this,
accessPolicies: [
AccessPolicy.fromAccessPolicyName('AmazonEKSClusterAdminPolicy', {
accessScopeType: AccessScopeType.CLUSTER,
}),
],
});
this.accessEntries.set(principal, newEntry);
return newEntry;
}

/**
* Fetch the load balancer address of a service of type 'LoadBalancer'.
*
Expand Down Expand Up @@ -1730,13 +1754,19 @@ export class Cluster extends ClusterBase {
},
});

new KubernetesPatch(this, 'CoreDnsComputeTypePatch', {
const k8sPatch = new KubernetesPatch(this, 'CoreDnsComputeTypePatch', {
cluster: this,
resourceName: 'deployment/coredns',
resourceNamespace: 'kube-system',
applyPatch: renderPatch(CoreDnsComputeType.FARGATE),
restorePatch: renderPatch(CoreDnsComputeType.EC2),
});

// In Patch deletion, it needs to apply the restore patch to the cluster
// So the cluster admin access can only be deleted after the patch
if (this._clusterAdminAccess) {
k8sPatch.node.addDependency(this._clusterAdminAccess);
}
}
}

Expand Down

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,7 @@
"PatchType": "strategic"
},
"DependsOn": [
"FargateTestClusterClusterAdminRoleAccess9EFE9888",
"FargateTestClusterKubectlReadyBarrier724731D5"
],
"UpdateReplacePolicy": "Delete",
Expand Down
Loading

0 comments on commit 4ada313

Please sign in to comment.