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

Allow disabling name hashes on the chart level #1037

Closed
iliapolo opened this issue Feb 6, 2023 · 2 comments · Fixed by #1079
Closed

Allow disabling name hashes on the chart level #1037

iliapolo opened this issue Feb 6, 2023 · 2 comments · Fixed by #1079
Assignees
Labels
effort/small 1 day tops feature-request New/Enhanced functionality wanted priority/p1 Should be on near term plans

Comments

@iliapolo
Copy link
Member

iliapolo commented Feb 6, 2023

Currently the hash of the construct path is always suffixed to the autogenerated resource name. We should add an option on the chart to opt-out of this.

For example, the following code:

import { App, Chart } from 'cdk8s';
import * as kplus from 'cdk8s-plus-25';

const app = new App();
const chart = new MyChart(app, 'my-chart');

new kplus.Deployment(chart, 'Deployment', {
  containers: [{ image: 'nginx' }]
});

app.synth();

Will generate a Deployment resource with the following name: my-chart-deployment-c8c354dd.

See https://docs.aws.amazon.com/cdk/v2/guide/identifiers.html#identifiers_unique_ids for reasoning why we add the hash.

In some cases the hash is undesirable and currently users are forced to override the Chart.generateObjectName method to drop the hashes. Lets make this easier by adding a chart property that allows dropping the hashes.

See cdk8s-team/cdk8s#232

@iliapolo iliapolo added feature-request New/Enhanced functionality wanted effort/small 1 day tops priority/p1 Should be on near term plans labels Feb 6, 2023
@sumupitchayan
Copy link
Contributor

sumupitchayan commented Feb 24, 2023

Created a PR - can close this issue once it's reviewed and merged!

@sumupitchayan
Copy link
Contributor

Looks like this issue overlaps with this one cdk8s-team/cdk8s#232

@mergify mergify bot closed this as completed in #1079 Mar 6, 2023
@mergify mergify bot closed this as completed in d1c8cde Mar 6, 2023
iliapolo pushed a commit that referenced this issue Mar 6, 2023
Fixes #1037

Added an `enableResourceNameHashes` property to `ChartProps` which is set to `true` by default. If set to false, then all resources inside of a chart will no longer have the hash value suffixed on their resource names.

Created a test called "disabling resource name hashes at chart level" in test/chart.test.ts to verify these changes are working.

This fix should also apply to [this issue](cdk8s-team/cdk8s#232) requesting documentation about name generation.

(cherry picked from commit d1c8cde)
Signed-off-by: Sumu Pitchayan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small 1 day tops feature-request New/Enhanced functionality wanted priority/p1 Should be on near term plans
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants