Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using .addFargateProfile from Icluster #13153

Closed
ismferd opened this issue Feb 19, 2021 · 20 comments
Closed

Using .addFargateProfile from Icluster #13153

ismferd opened this issue Feb 19, 2021 · 20 comments
Assignees
Labels
guidance Question that needs advice or information. investigating This issue is being investigated and/or work is in progress to resolve the issue.

Comments

@ismferd
Copy link

ismferd commented Feb 19, 2021

Hi everyone,

I would like to know how I can create a fargateProfile using CDK without having the object Cluster.
So, we are building a lib where users can create their k8s infrastructure just adding a name in their repo. This library can create resources talking with EKS trough IAM/k8s cli.
Nevertheless, fargateProfile expect an object Cluster instead of iCluster. I have seen that eksctl or awscli are expecting just the name of the cluste.

Do you know how we could create a fargaterProfile having ICluster object instead of Cluster?

@ismferd ismferd added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Feb 19, 2021
@peterwoodworth
Copy link
Contributor

Hey @ismferd, sorry for the delayed response.

The short answer is that you can't create a FargateProfile without a Cluster. However there should be a way to do what you're trying to accomplish. Why do you require the use of an ICluster rather than a Cluster?

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2021
@ismferd
Copy link
Author

ismferd commented Mar 23, 2021

Basically, security reasons. So, If If use Cluster every user using my library could modify Cluster properties and that doesn't make sense, also we could have a rase conditions between users, I mean, one user changing something the EKS stack in UPDATE_IN_PROGRESS which doesn't allow do changes to the other clients.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 24, 2021
@peterwoodworth peterwoodworth added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Apr 6, 2021
@vgribok
Copy link

vgribok commented Apr 8, 2021

I suggest this issue is re-classified as a bug since with current state it seems impossible to add Fargate profile to an existing EKS ICluster returned by Cluster.FromClusterAttributes();

@peterwoodworth
Copy link
Contributor

Just to clarify for all here, by design imported resources cannot be modified.

I haven't forgotten about this issue, so apologies if it seems like I have. I will try to get a full response by the end of next week

@vgribok
Copy link

vgribok commented Apr 9, 2021

Thanks, @peterwoodworth. To supply use case blocked by current implementation:
K8s namespaces are ephemeral, they get created and deleted easily, and that can be done via CDK on an imported cluster. Fargate profiles often need to be created and deleted at the same time as K8s namespaces, which cannot be done via CDK on an imported cluster.

@peterwoodworth
Copy link
Contributor

Sorry for the long response time,

Unfortunately there's no recommended way to go about this. For the CDK to support this, the FargateProfile class would need to be redesigned to be able to accept an ICluster instead of a Cluster, which I don't think is something we're looking to do. Apologies for the poor experience here

@peterwoodworth peterwoodworth added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Jun 24, 2021
@vgribok
Copy link

vgribok commented Jun 24, 2021

Is there a general delineation in CDK construct design between non-volatile and volatile construct properties?
Fargate profile to EKS cluster is what an S3 object/file to an S3 bucket. The fact we have no avenue of solving this points at a design flaw that needs to be identified and resolved.
Anyway, please do not close the issue as it seems to be staying unresolved and blocking a real-world use case.

@peterwoodworth
Copy link
Contributor

What do you mean by volatility?

@peterwoodworth peterwoodworth removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 24, 2021
@vgribok
Copy link

vgribok commented Jun 24, 2021

I mean properties that have values expected to change a lot during construct's lifetime - EKS profiles, count of objects in an S3 bucket, and alike. Versus non-volatile or low-volatility properties, like S3 bucket name, or EKS cluster name.

@peterwoodworth
Copy link
Contributor

I'm sorry I don't think I'm following, the cdk synthesizes cloudformation templates with the information you've given it, and unless you explicitly tell it to with context methods it won't communicate with AWS to be able to know additional information like how many objects an S3 bucket has.

I understand this is blocking a use case, do you have any ideas as to how the CDK can achieve this? Currently the FargateProfile construct makes use of custom resources rather than the CfnFargateProfile introduced last September

const resource = new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
resourceType: FARGATE_PROFILE_RESOURCE_TYPE,
properties: {
AssumeRoleArn: props.cluster.adminRole.roleArn,
Config: {
clusterName: props.cluster.clusterName,
fargateProfileName: props.fargateProfileName,
podExecutionRoleArn: this.podExecutionRole.roleArn,
selectors: props.selectors,
subnets,
tags: Lazy.any({ produce: () => this.tags.renderTags() }),
},
},
});

After some research I wasn't able to figure out if CloudFormation will allow you to add Fargate Profiles to existing clusters. I'll have to figure out if that's possible. If it's not, it's pretty unlikely that the CDK will support this feature

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 20, 2021
@peterwoodworth
Copy link
Contributor

Would it work if you create a CfnFargateProfile instead? It looks like all it requires is the name of the cluster.

@peterwoodworth peterwoodworth added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jul 29, 2021
@vgribok
Copy link

vgribok commented Jul 30, 2021

Looking at the spec, it may. I am not actively working on this project for now. Will revisit when I am back on this task.

@peterwoodworth
Copy link
Contributor

I can check to see if this works myself sometime in the next week

@peterwoodworth peterwoodworth added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jul 30, 2021
@peterwoodworth
Copy link
Contributor

@ismferd @vgribok I can confirm that you can attach a fargate profile to a Cluster without needing the cluster object by using CfnFargateProfile. I just created a stack with only the Fargate Profile L1 construct and attached it to an existing cluster by providing the cluster name. Ping me if you need more help with this!

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@vgribok
Copy link

vgribok commented Aug 27, 2021

@peterwoodworth Peter, CfnFargateProfileProps requires podExecutionRoleArn, which in FargateProfile class is set by a fair amount of boilerplate code. Could you please provide an example for using CfnFargateProfile?

@peterwoodworth
Copy link
Contributor

All you need to do is create a pod execution role. This is provided as an aws managed policy

const podExecutionRole = new iam.Role(this, 'PodExecutionRole', {
  assumedBy: new iam.ServicePrincipal('eks-fargate-pods.amazonaws.com'),
  managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSFargatePodExecutionRolePolicy')],
});

const fargateProfile = new CfnFargateProfile(this, "FargateProfile", {
  clusterName: 'myClusterName',
  podExecutionRoleArn: podExecutionRole.roleArn
}

@ismferd
Copy link
Author

ismferd commented Sep 16, 2021

An example in kotlin:

        val selector = CfnFargateProfile.SelectorProperty.builder().namespace("foo").build()
        val fargate = CfnFargateProfile(this, "FargateProfile", CfnFargateProfileProps
                .builder()
                .clusterName("foo-cluster")
                .fargateProfileName("foo")
                .selectors(mutableListOf(selector))
                .podExecutionRoleArn("arn:aws:iam::1234567890:role/devEksPodExecutionRole")
                .build())

@IsaacLeeWebDev
Copy link

IsaacLeeWebDev commented Mar 5, 2022

For anyone else looking at these GitHub issues looking for a way to define a Fargate Profile in a CDK app separate from the one the cluster is defined in, heads-up:

Because of #19218, if you take the recommended approach in this issue, you're at risk of your externally-defined PodExecutionRole being removed from the aws-auth ConfigMap the next time someone updates the stack that defines the cluster using the CDK, because CDK apps that define a Cluster replace that whole ConfigMap every time they synth.

To work around this, you'll need to make sure the cluster somehow gets a reference to the role ARN of that externally-defined PodExecutionRole, and explicitly add the role's ARN that to the mappedRoles of the ConfigMap like so:

import * as eks from '@aws-cdk/aws-eks'; // or import { aws_eks as eks } from 'aws-cdk-lib';

/* begin pseudocode */

const myCluster = new eks.SomeKindOfEksCluster(/* ... */);

// maybe uses CfnOutputs, maybe SSM, idk. Just be mindful of creating circular dependencies.
const externalPodExecutionRoleArn = await getTheRoleArnSomehow();

/* end pseudocode */

const myPodExecRole = iam.Role.fromRoleArn(this, `${somethingUnique}-arn-ref`, externalPodExecutionRoleArn);

myCluster.awsAuth.addRoleMapping(
  myPodExecRole,
  {
    username: 'system:node:{{SessionName}}',
    groups: ['system:bootstrappers', 'system:nodes', 'system:node-proxier'],
  },
);

You have to pass in the externally defined IAM role and those other props to your Cluster's .awsAuth.addRoleMapping method whenever that Cluster synths with a diff, or that role will be removed from the aws-auth ConfigMap any time the Cluster's stack changes, and you won't be able to get Nodes for your Pods.

@oliverdavidnoguerasanmartin

I have a issue similar but I want to modify clusterRole of ICluster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guidance Question that needs advice or information. investigating This issue is being investigated and/or work is in progress to resolve the issue.
Projects
None yet
Development

No branches or pull requests

5 participants