aws-quickstart / cdk-eks-blueprints

AWS Quick Start Team
Apache License 2.0
454 stars 205 forks source link

Allow passing in `NodeRole` when creating managed node groups #540

Closed praveenperera closed 1 year ago

praveenperera commented 1 year ago

Describe the feature

Allow passing in a NodeRole or policies for the NodeRole for managed groups.

Use Case

I need to give the nodes some extra access, and to do that I need to add some permissions to the nodeRole.

However, when I try to pass in a nodeRole with the managedNodeGroups:

    const nodeRole = new aws_iam.Role(this, "NodeRole", {
      assumedBy: new aws_iam.ServicePrincipal("ec2.amazonaws.com"),
     ....
    });

    const clusterProvider = new blueprints.GenericClusterProvider({
      clusterName: clusterName,
      version: KubernetesVersion.V1_23,
      managedNodeGroups: {... nodeRole: nodeRole}
    });

I get an error:

<PARENT_STACK>/NodeRole should be defined in the scope of the <BLUEPRINTS_CLUSTER_STACK> stack to prevent circular dependencies

Proposed Solution

I think what needs to be done is add the ability to pass in permissions to the nodeRole, or give me access to the nodeRole afterwards so I can add permissions after its created.

Other Information

No response

Acknowledgements

CDK version used

2.50.0 (build 4c11af6)

EKS Blueprints Version

1.4.1

Node.js Version

v16.17.1

Environment details (OS name and version, etc.)

MacOS 12.6.1 (21G217)

shapirov103 commented 1 year ago

Similar to #435, please see comments there. The approach used can be similar to https://docs.aws.amazon.com/cdk/api/v1/docs/aws-s3-readme.html#sharing-buckets-between-stacks

praveenperera commented 1 year ago

Thanks, if we could make the blueprints a NestedStack instead of Stack would this problem be solved?

shapirov103 commented 1 year ago

You don't have to make it a nested stack. Just create a separate stack where you either provision or look up the role you need. create a variable in the stack and then pass it over to the blueprints in Mng or generic cluster provider as nodeRole: myOtherStack.nodeRole. Similar to the example with S3 above.

However, I have a separate issue to make this configuration passing much easier. I expect it to be part of the solution soon.

praveenperera commented 1 year ago

@shapirov103 I believe that's what I did above. But I get the error:

<PARENT_STACK>/NodeRole should be defined in the scope of the <BLUEPRINTS_CLUSTER_STACK> stack to prevent circular dependencies

But I think the solution in the issue you linked should work. I'm looking forward to the new config passing solution.

shapirov103 commented 1 year ago

Yeah, please don't use the approach like:

class MyCdkBlueprint extends Stack {
    constructor(...) {
        const myRole = ...;
        blueprints.builder.build();
    }
}

The above will produce a redundant outer stack and another stack for blueprints.

Instead you can use:

const app = new cdk.App();

class MyRoleStack extends Stack {
    public nodeRole;
    constructor()
}

const roleStack = new MyRoleStack(app, ...);

const clusterProvider = blueprints.MngClusterProvider( { nodeRole: roleStack.nodeRole}); 
blueprints.EksBlueprint.builder().clusterProvider(clusterProvider)...build()...
praveenperera commented 1 year ago

Ah I think doing it that way caused me a lot of issues. I did it this way because I needed to control the VPC better (less AZs for some clusters). But I think I'll find a different way to do that, I guess by creating a separate stack for VPCs.

namjug-kim commented 1 year ago

@shapirov103

Instead you can use:

const app = new cdk.App();

class MyRoleStack extends Stack {
    public nodeRole;
    constructor()
}

const roleStack = new MyRoleStack(app, ...);

const clusterProvider = blueprints.MngClusterProvider( { nodeRole: roleStack.nodeRole}); 
blueprints.EksBlueprint.builder().clusterProvider(clusterProvider)...build()...

Thanks for your comment. I tried it with the sample code below.


class NodeGroupRoleStack extends cdk.Stack {
    public readonly nodeRole: iam.IRole

    constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
        super(scope, id, props);
        this.nodeRole = iam.Role.fromRoleName(this, "123", "123")
    }
}
const roleStack = new NodeGroupRoleStack(app, ...);
const clusterProvider = blueprints.MngClusterProvider( { nodeRole: roleStack.nodeRole}); 
blueprints.EksBlueprint.builder().clusterProvider(clusterProvider)...build()...

[NodeGroupRoleStackId] should be defined in the scope of the [EksBluprintStackId] stack to prevent circular dependencies An error occurred as a result of execution. Is there something wrong?

there's no error when use shared nodeRole to blueprints.MngClusterProvider( { role: roleStack.nodeRole});

shapirov103 commented 1 year ago

@namjug-kim I will need to run the example to tell for sure, however, based on what I observed, CDK is very sensitive to the envs being identical between the stacks. You can see more details here. Have you tried setting account/region on both?

builder.account("").region("") and env for the NodeGroupRoleStack.

namjug-kim commented 1 year ago

@shapirov103

#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import * as blueprints from '@aws-quickstart/eks-blueprints';
import {GlobalResources, PlatformTeam, VpcProvider} from '@aws-quickstart/eks-blueprints';
import * as ec2 from "aws-cdk-lib/aws-ec2"
import * as iam from "aws-cdk-lib/aws-iam"
import {CapacityType, KubernetesVersion, NodegroupAmiType} from "aws-cdk-lib/aws-eks";

const app = new cdk.App();
const account = /*accont*/;
const region = 'ap-northeast-2';
let id = 'eks';

export class NodeGroupRoleStack extends cdk.Stack {
    public readonly nodeRole: iam.IRole

    constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
        super(scope, id, props);

        this.nodeRole = new iam.Role(this, `${id}-NodeGroupRole`, {
            assumedBy: new iam.ServicePrincipal('sns.amazonaws.com'),
            roleName: `${id}-NodeGroupRole`,
            managedPolicies: [
                iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonEKSWorkerNodePolicy"),
                iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonEC2ContainerRegistryReadOnly"),
                iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonEKS_CNI_Policy"),
                iam.ManagedPolicy.fromAwsManagedPolicyName("ElasticLoadBalancingFullAccess"),
                iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonSQSFullAccess"),
                iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonS3FullAccess"),
            ]
        });
    }
}

const nodeGroupRoleStack = new NodeGroupRoleStack(app, id + '-node-group-role', {
    env: {
        account: account,
        region: region
    }
});

let mngClusterProvider = new blueprints.MngClusterProvider({
    nodegroupName: 'test-node-group2',
    minSize: 1,
    maxSize: 1,
    desiredSize: 1,
    instanceTypes: [ec2.InstanceType.of(ec2.InstanceClass.T4G, ec2.InstanceSize.MEDIUM)],
    amiType: NodegroupAmiType.AL2_ARM_64,
    nodeGroupCapacityType: CapacityType.ON_DEMAND,
    nodeRole: nodeGroupRoleStack.nodeRole,
    version: KubernetesVersion.V1_24,
    privateCluster: false,
    vpcSubnets: [{availabilityZones: ["ap-northeast-2a", "ap-northeast-2c"]}]
});

const stack = blueprints.EksBlueprint.builder()
    .version(KubernetesVersion.V1_24)
    .clusterProvider(mngClusterProvider)
    .account(account)
    .region(region)
    .useDefaultSecretEncryption(false)
    .build(app, `${id}-eksBlueprint`);

sorry for bothering you :( I added account, region to producer and consumer, but the same error occurred. error was reproduced by above code.

namjug-kim commented 1 year ago

Im not sure this is right purpose. but under code works for me :)

const stack = blueprints.EksBlueprint.builder() ...
    .build(app, id)

stack.getClusterInfo().nodeGroups?.at(0)?.role?.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonSQSFullAccess"));
praveenperera commented 1 year ago

Im not sure this is right purpose. but under code works for me :)

const stack = blueprints.EksBlueprint.builder() ...
    .build(app, id)

stack.getClusterInfo().nodeGroups?.at(0)?.role?.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonSQSFullAccess"));

Oh that’s very helpful and very easy. I’m going to do that.