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

fix(eks): incorrect nodegroupName(under feature flag) #29794

Merged
merged 18 commits into from
Apr 19, 2024
Merged
11 changes: 9 additions & 2 deletions packages/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { Cluster, ICluster, IpFamily } from './cluster';
import { CfnNodegroup } from './eks.generated';
import { InstanceType, ISecurityGroup, SubnetSelection, InstanceArchitecture, InstanceClass, InstanceSize } from '../../aws-ec2';
import { IRole, ManagedPolicy, PolicyStatement, Role, ServicePrincipal } from '../../aws-iam';
import { IResource, Resource, Annotations, withResolved } from '../../core';
import { IResource, Resource, Annotations, withResolved, FeatureFlags } from '../../core';
import * as cxapi from '../../cx-api';


/**
* NodeGroup interface
Expand Down Expand Up @@ -537,7 +539,12 @@ export class Nodegroup extends Resource implements INodegroup {
resource: 'nodegroup',
resourceName: this.physicalName,
});
this.nodegroupName = this.getResourceNameAttribute(resource.ref);

if (FeatureFlags.of(this).isEnabled(cxapi.EKS_NODEGROUP_NAME)) {
this.nodegroupName = this.getResourceNameAttribute(resource.attrNodegroupName);
} else {
this.nodegroupName = this.getResourceNameAttribute(resource.ref);
}
}

private validateUpdateConfig(maxUnavailable?: number, maxUnavailablePercentage?: number) {
Expand Down
22 changes: 22 additions & 0 deletions packages/aws-cdk-lib/aws-eks/test/nodegroup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Template } from '../../assertions';
import * as ec2 from '../../aws-ec2';
import * as iam from '../../aws-iam';
import * as cdk from '../../core';
import * as cxapi from '../../cx-api';
import * as eks from '../lib';
import { NodegroupAmiType, TaintEffect } from '../lib';

Expand Down Expand Up @@ -1641,6 +1642,27 @@ describe('node group', () => {
});
});

test('EKS_NODEGROUP_NAME feature flag should return correct nodegroupName', () => {
// GIVEN
const app = new cdk.App();
const stackWithFlag = new cdk.Stack(app, 'StackWithFlag', {
env: { account: '1234', region: 'testregion' },
});

// WHEN
stackWithFlag.node.setContext(cxapi.EKS_NODEGROUP_NAME, true);
const cluster = new eks.Cluster(stackWithFlag, 'Cluster', {
defaultCapacity: 0,
version: CLUSTER_VERSION,
});
const ng = new eks.Nodegroup(stackWithFlag, 'Nodegroup', {
cluster,
});

// THEN
expect(ng.nodegroupName).not.toEqual((ng.node.defaultChild as eks.CfnNodegroup).ref);
});

test('throws when maxUnavailable and maxUnavailablePercentage are set', () => {
// GIVEN
const { stack, vpc } = testFixture();
Expand Down
12 changes: 12 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-clou
export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
export const EKS_NODEGROUP_NAME = '@aws-cdk/aws-eks:nodegroupNameAttribute';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1034,6 +1035,17 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: '2.134.0' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[EKS_NODEGROUP_NAME]: {
type: FlagType.BugFix,
summary: 'When enabled, nodegroupName attribute of the provisioned EKS NodeGroup does not have the cluster name prefix',
detailsMd: `
When this feature flag is enabled, the nodegroupName attribute will be exactly the name of the nodegroup only.
`,
introducedIn: { v2: '2.138.0' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down
Loading