Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
metral committed Feb 5, 2020
1 parent e6c0894 commit 6acf763
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const cluster = new eks.Cluster(`${projectName}`, {
deployDashboard: false,
// Modify the roleMappings to update the aws-auth configMap.
// This will not remove the managed node group's role from aws-auth since
// it's role is set in instanceRoles. Not setting instanceRoles in the
// its role is set in instanceRoles. Not setting instanceRoles in the
// cluster first for the nodegroup will error eks.createManagedNodeGroup().
roleMappings: [roleMapping0, roleMapping1],
instanceRoles: [roles[2]],
Expand Down
42 changes: 23 additions & 19 deletions nodejs/eks/nodegroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,19 @@ export type ManagedNodeGroupOptions = Omit<aws.eks.NodeGroupArgs, "clusterName"

/**
* Make nodeRoleArn optional, since users may prefer to provide the
* nodegroup role directly using nodeRole.
* nodegroup role directly using nodeRole.
*
* Note, nodeRoleArn and nodeRole are mutually exclusive, and a single option
* must be used.
*/
nodeRoleArn?: pulumi.Output<string>;
nodeRoleArn?: pulumi.Input<string>;

/**
* Make nodeRole an option in lieu of nodeGroupArn to create consistency
* with how roles are used in the cluster args.
* Make nodeRole optional, since users may prefer to provide the
* nodeRoleArn.
*
* Note, nodeRole and nodeRoleArn are mutually exclusive, and a single
* option must be used.
*/
nodeRole?: pulumi.Input<aws.iam.Role>;

Expand Down Expand Up @@ -699,31 +705,29 @@ export function createManagedNodeGroup(name: string, args: ManagedNodeGroupOptio
const core = isCoreData(args.cluster) ? args.cluster : args.cluster.core;
const parent = isCoreData(args.cluster) ? args.cluster.cluster : args.cluster;

// Compute the nodegroup role.
let roleArn: pulumi.Output<string>;
if (!args.nodeRole && !args.nodeRoleArn) {
throw new Error(`An IAM role, or role ARN must be provided to create a managed node group`);
}

if (args.nodeRole && args.nodeRoleArn) {
throw new Error("nodeRole and nodeRoleArn are mutually exclusive to create a managed node group. Choose a single approach");
}

// Compute the nodegroup role.
let roleArn: pulumi.Output<string> = pulumi.output("");
if (args.nodeRole !== undefined) {
} else if (args.nodeRole && args.nodeRoleArn) {
throw new Error("nodeRole and nodeRoleArn are mutually exclusive to create a managed node group.");
} else if (args.nodeRole !== undefined) {
roleArn = pulumi.output(args.nodeRole).apply(r => r.arn);
} else if (args.nodeRoleArn !== undefined) {
roleArn = args.nodeRoleArn;
roleArn = pulumi.output(args.nodeRoleArn);
} else {
roleArn = core.instanceRoles[0]?.arn;
}

// Check that the nodegroup role has been set on the cluster to
// ensure that the aws-auth configmap was properly formed.
const roleExistsInNodeAccess = pulumi.all([
const nodegroupRole = pulumi.all([
core.instanceRoles,
roleArn,
]).apply(([roles, rArn]) => {
// Map out the ARNs of all of the instanceRoles.
const roleArns = roles.map(role => {
return role.arn.apply(a => a);
return role.arn;
});
// Try finding the nodeRole in the ARNs array.
return pulumi.all([
Expand All @@ -734,9 +738,9 @@ export function createManagedNodeGroup(name: string, args: ManagedNodeGroupOptio
});
});

roleExistsInNodeAccess.apply(exist => {
if (!exist) {
throw new Error(`A managed node group cannot be created without first setting it's role in the cluster's instanceRoles`);
nodegroupRole.apply(role => {
if (!role) {
throw new Error(`A managed node group cannot be created without first setting its role in the cluster's instanceRoles`);
}
});

Expand Down

0 comments on commit 6acf763

Please sign in to comment.