Skip to content

Commit 76ac417

Browse files
authored
Merge pull request #1242 from awslabs/bugfix/s3mountpoint-fails-rbac
Bugfix/s3mountpoint fails rbac
2 parents e90df20 + bc61c0b commit 76ac417

File tree

2 files changed

+21
-23
lines changed

2 files changed

+21
-23
lines changed

lib/addons/s3-csi-driver/index.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const defaultProps: HelmAddOnUserProps & S3CSIDriverAddOnProps = {
3737
name: S3_CSI_DRIVER,
3838
namespace: "kube-system",
3939
release: S3_CSI_DRIVER_RELEASE,
40-
version: "2.4.0",
40+
version: "2.4.1",
4141
repository: "https://awslabs.github.io/mountpoint-s3-csi-driver",
4242
createNamespace: false,
4343
bucketNames: [],
@@ -55,40 +55,38 @@ export class S3CSIDriverAddOn extends HelmAddOn {
5555
}
5656

5757
deploy(clusterInfo: ClusterInfo): Promise<Construct> {
58-
// Create service account and policy
5958
const cluster = clusterInfo.cluster;
59+
60+
// Create namespace
61+
if (this.options.createNamespace) {
62+
createNamespace(this.options.namespace!, cluster, true);
63+
}
64+
65+
// Let Helm create the node SA with RBAC bindings
66+
const chartValues = populateValues(this.options);
67+
const s3CsiDriverChart = this.addHelmChart(clusterInfo, chartValues, true, true);
68+
69+
// Overwrite the Helm-created SA with IRSA annotation (fires after chart)
6070
const serviceAccount = cluster.addServiceAccount(S3_CSI_DRIVER_SA, {
6171
name: S3_CSI_DRIVER_SA,
6272
namespace: this.options.namespace,
73+
overwriteServiceAccount: true
6374
});
6475

6576
const s3BucketPolicy = new iam.Policy(cluster, S3_DRIVER_POLICY, {
6677
statements:
6778
getS3DriverPolicyStatements(this.options.bucketNames, this.options.kmsArns ?? [])
6879
});
6980
serviceAccount.role.attachInlinePolicy(s3BucketPolicy);
70-
71-
// Create namespace
72-
if (this.options.createNamespace) {
73-
const ns = createNamespace(this.options.namespace!, cluster, true);
74-
serviceAccount.node.addDependency(ns);
75-
}
76-
77-
// setup value for helm chart
78-
const chartValues = populateValues(this.options, serviceAccount.serviceAccountName);
7981

80-
const s3CsiDriverChart = this.addHelmChart(clusterInfo, chartValues, true, true);
81-
s3CsiDriverChart.node.addDependency(serviceAccount);
82+
serviceAccount.node.addDependency(s3CsiDriverChart);
8283
return Promise.resolve(s3CsiDriverChart);
8384
}
8485
}
8586

86-
function populateValues(helmOptions: S3CSIDriverAddOnProps, serviceAccountName: string): any {
87+
function populateValues(helmOptions: S3CSIDriverAddOnProps): any {
8788
const values = helmOptions.values ?? {};
88-
// Only configure the node service account (which needs S3 access)
89-
setPath(values, 'node.serviceAccount.create', false);
90-
setPath(values, 'node.serviceAccount.name', serviceAccountName);
91-
// Let Helm create the controller service account (no S3 access needed)
89+
setPath(values, 'node.serviceAccount.create', true);
9290
setPath(values, 'controller.serviceAccount.create', true);
9391
setPath(values, 'node.tolerateAllTaints', true);
9492
return values;

lib/addons/vpc-cni/index.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as eks from "aws-cdk-lib/aws-eks";
33
import * as iam from "aws-cdk-lib/aws-iam";
44
import { Construct } from 'constructs';
55
import { ClusterInfo, Values } from "../../spi";
6-
import { loadYaml, readYamlDocument, ReplaceServiceAccount, supportsALL, conflictsWithAutoMode, AutoModeConflictType} from "../../utils";
6+
import { loadYaml, readYamlDocument, supportsALL, conflictsWithAutoMode, AutoModeConflictType} from "../../utils";
77
import { CoreAddOn, CoreAddOnProps } from "../core-addon";
88
import { KubectlProvider, ManifestDeployment } from "../helm-addon/kubectl-provider";
99
import { KubernetesVersion } from "aws-cdk-lib/aws-eks";
@@ -426,14 +426,14 @@ export class VpcCniAddOn extends CoreAddOn {
426426
* @returns
427427
*/
428428
createServiceAccount(clusterInfo: ClusterInfo, saNamespace: string, policies: iam.IManagedPolicy[]): eks.ServiceAccount {
429-
const sa = new ReplaceServiceAccount(clusterInfo.cluster, `${this.coreAddOnProps.saName}-sa`, {
430-
cluster: clusterInfo.cluster,
429+
const sa = clusterInfo.cluster.addServiceAccount(`${this.coreAddOnProps.saName}-sa`, {
431430
name: this.coreAddOnProps.saName,
432-
namespace: saNamespace
431+
namespace: saNamespace,
432+
overwriteServiceAccount: true
433433
});
434434

435435
policies.forEach(p => sa.role.addManagedPolicy(p));
436-
return sa as any as eks.ServiceAccount;
436+
return sa;
437437
}
438438
}
439439

0 commit comments

Comments
 (0)