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

Add prop for setting service account name #136

Closed
plumdog opened this issue Oct 3, 2023 · 3 comments
Closed

Add prop for setting service account name #136

plumdog opened this issue Oct 3, 2023 · 3 comments

Comments

@plumdog
Copy link
Contributor

plumdog commented Oct 3, 2023

This is unset so defaults to the id of the resource, per https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_eks.ServiceAccount.html#name

This can get very long (I'm using CDK pipelines) and Kubernetes caps the length of the label at 63 characters.

Should add an option to allow me to set this.

I'm investigating whether I can handle this with CDK escape hatches.

@plumdog
Copy link
Contributor Author

plumdog commented Oct 5, 2023

My disgraceful but effective workaround:

const orig = myCluster.addServiceAccount.bind(myCluster);
const patched = (id: string, props: eks.ServiceAccountProps) => {
    const patchedProps = id === 'karpenter' ? { ...props, name: 'karpenter' } : props;
    return orig(id, patchedProps);
};
// @ts-ignore
myCluster.addServiceAccount = patched;

This allows me to make

this.serviceAccount = this.cluster.addServiceAccount('karpenter', {
namespace: this.namespace,
});
do what I want, which is actually:

this.serviceAccount = this.cluster.addServiceAccount('karpenter', {
    namespace: this.namespace,
    name: 'karpenter',
});

@andskli
Copy link
Contributor

andskli commented Oct 24, 2023

@plumdog thanks for the report! Would it make sense to just default the serivceAccountName to karpenter and provide an option to the construct to specify your own name? In hindsight I don't really see the benefits of using the CDK generated name and can't really see how changing this behavior could break existing deployments either; the name of the serviceAccount would change but so would the reference to it in the Helm chart values.

@plumdog
Copy link
Contributor Author

plumdog commented Oct 24, 2023

@andskli Yes, I think just defaulting to karpenter would be perfectly sensible.

And I agree it makes sense to treat that as an implementation detail, rather than a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants