Skip to content

Commit

Permalink
Merge branch 'master' into es-domainEndpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Dec 21, 2021
2 parents e589e2a + a6dde1e commit f1bc543
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 25 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codedeploy/lib/server/load-balancer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export abstract class LoadBalancer {
*
* @param albTargetGroup an ALB Target Group
*/
public static application(albTargetGroup: elbv2.ApplicationTargetGroup): LoadBalancer {
public static application(albTargetGroup: elbv2.IApplicationTargetGroup): LoadBalancer {
class AlbLoadBalancer extends LoadBalancer {
public readonly generation = LoadBalancerGeneration.SECOND;
public readonly name = albTargetGroup.targetGroupName;
Expand All @@ -55,7 +55,7 @@ export abstract class LoadBalancer {
*
* @param nlbTargetGroup an NLB Target Group
*/
public static network(nlbTargetGroup: elbv2.NetworkTargetGroup): LoadBalancer {
public static network(nlbTargetGroup: elbv2.INetworkTargetGroup): LoadBalancer {
class NlbLoadBalancer extends LoadBalancer {
public readonly generation = LoadBalancerGeneration.SECOND;
public readonly name = nlbTargetGroup.targetGroupName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,4 +412,30 @@ describe('CodeDeploy Server Deployment Group', () => {

expect(() => SynthUtils.toCloudFormation(stack)).toThrow(/deploymentInAlarm/);
});

test('can be used with an imported ALB Target Group as the load balancer', () => {
const stack = new cdk.Stack();

new codedeploy.ServerDeploymentGroup(stack, 'DeploymentGroup', {
loadBalancer: codedeploy.LoadBalancer.application(
lbv2.ApplicationTargetGroup.fromTargetGroupAttributes(stack, 'importedAlbTg', {
targetGroupArn: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/myAlbTargetGroup/73e2d6bc24d8a067',
}),
),
});

expect(stack).toHaveResourceLike('AWS::CodeDeploy::DeploymentGroup', {
'LoadBalancerInfo': {
'TargetGroupInfoList': [
{
'Name': 'myAlbTargetGroup',
},
],
},
'DeploymentStyle': {
'DeploymentOption': 'WITH_TRAFFIC_CONTROL',
},
});
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,11 @@ export interface TargetGroupImportProps extends TargetGroupAttributes {
* A target group
*/
export interface ITargetGroup extends cdk.IConstruct {
/**
* The name of the target group
*/
readonly targetGroupName: string;

/**
* ARN of the target group
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ export abstract class ImportedTargetGroupBase extends CoreConstruct implements I
*/
public readonly targetGroupArn: string;

/**
* The name of the target group
*/
public readonly targetGroupName: string;

/**
* A token representing a list of ARNs of the load balancers that route traffic to this target group
*/
Expand All @@ -29,6 +34,7 @@ export abstract class ImportedTargetGroupBase extends CoreConstruct implements I
super(scope, id);

this.targetGroupArn = props.targetGroupArn;
this.targetGroupName = cdk.Stack.of(scope).splitArn(props.targetGroupArn, cdk.ArnFormat.SLASH_RESOURCE_SLASH_RESOURCE_NAME).resourceName!.split('/')[0];
this.loadBalancerArns = props.loadBalancerArns || cdk.Aws.NO_VALUE;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { ApplicationProtocol, Protocol } from './enums';

export type Attributes = {[key: string]: string | undefined};
export type Attributes = { [key: string]: string | undefined };

/**
* Render an attribute dict to a list of { key, value } pairs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('tests', () => {

// WHEN
const tg = elbv2.ApplicationTargetGroup.fromTargetGroupAttributes(stack, 'TG', {
targetGroupArn: 'arn',
targetGroupArn: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/myAlbTargetGroup/73e2d6bc24d8a067',
});
tg.addTarget(new FakeSelfRegisteringTarget(stack, 'Target', vpc));
});
Expand All @@ -65,7 +65,7 @@ describe('tests', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const tg = elbv2.ApplicationTargetGroup.fromTargetGroupAttributes(stack, 'TG', {
targetGroupArn: 'arn',
targetGroupArn: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/myAlbTargetGroup/73e2d6bc24d8a067',
});

// WHEN
Expand Down Expand Up @@ -497,4 +497,18 @@ describe('tests', () => {
app.synth();
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 2 minutes');
});

test('imported targetGroup has targetGroupName', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');

// WHEN
const importedTg = elbv2.ApplicationTargetGroup.fromTargetGroupAttributes(stack, 'importedTg', {
targetGroupArn: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/myAlbTargetGroup/73e2d6bc24d8a067',
});

// THEN
expect(importedTg.targetGroupName).toEqual('myAlbTargetGroup');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -499,4 +499,18 @@ describe('tests', () => {
expect(() => targetGroup.metricHealthyHostCount()).toThrow(/The TargetGroup needs to be attached to a LoadBalancer/);
expect(() => targetGroup.metricUnHealthyHostCount()).toThrow(/The TargetGroup needs to be attached to a LoadBalancer/);
});

test('imported targetGroup has targetGroupName', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');

// WHEN
const importedTg = elbv2.NetworkTargetGroup.fromTargetGroupAttributes(stack, 'importedTg', {
targetGroupArn: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/myNlbTargetGroup/73e2d6bc24d8a067',
});

// THEN
expect(importedTg.targetGroupName).toEqual('myNlbTargetGroup');
});
});
48 changes: 28 additions & 20 deletions tools/@aws-cdk/prlint/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import * as GitHub from 'github-api';
import { breakingModules } from './parser';
import { findModulePath, moduleStability } from './module';

const OWNER = "aws"
const REPO = "aws-cdk"
const EXEMPT_README = 'pr-linter/exempt-readme'
const EXEMPT_TEST = 'pr-linter/exempt-test'
const OWNER = 'aws';
const REPO = 'aws-cdk';
const EXEMPT_README = 'pr-linter/exempt-readme';
const EXEMPT_TEST = 'pr-linter/exempt-test';
const EXEMPT_BREAKING_CHANGE = 'pr-linter/exempt-breaking-change';

class LinterError extends Error {
constructor(message: string) {
Expand All @@ -18,9 +19,9 @@ function createGitHubClient() {
const token = process.env.GITHUB_TOKEN;

if (token) {
console.log("Creating authenticated GitHub Client")
console.log("Creating authenticated GitHub Client");
} else {
console.log("Creating un-authenticated GitHub Client")
console.log("Creating un-authenticated GitHub Client");
}

return new GitHub({'token': token});
Expand Down Expand Up @@ -49,19 +50,19 @@ function readmeChanged(files: any[]) {
function featureContainsReadme(issue: any, files: any[]) {
if (isFeature(issue) && !readmeChanged(files) && !isPkgCfnspec(issue)) {
throw new LinterError("Features must contain a change to a README file");
};
}
};

function featureContainsTest(issue: any, files: any[]) {
if (isFeature(issue) && !testChanged(files)) {
throw new LinterError("Features must contain a change to a test file");
};
}
};

function fixContainsTest(issue: any, files: any[]) {
if (isFix(issue) && !testChanged(files)) {
throw new LinterError("Fixes must contain a change to a test file");
};
}
};

function shouldExemptReadme(issue: any) {
Expand All @@ -72,6 +73,10 @@ function shouldExemptTest(issue: any) {
return hasLabel(issue, EXEMPT_TEST);
}

function shouldExemptBreakingChange(issue: any) {
return hasLabel(issue, EXEMPT_BREAKING_CHANGE);
}

function hasLabel(issue: any, labelName: string) {
return issue.labels.some(function (l: any) {
return l.name === labelName;
Expand All @@ -93,7 +98,7 @@ function validateBreakingChangeFormat(title: string, body: string) {
throw new LinterError(`Breaking changes should be indicated by starting a line with 'BREAKING CHANGE: ', variations are not allowed. (found: '${m[0]}')`);
}
if (m[0].substr('BREAKING CHANGE:'.length).trim().length === 0) {
throw new LinterError("The description of the first breaking change should immediately follow the 'BREAKING CHANGE: ' clause")
throw new LinterError("The description of the first breaking change should immediately follow the 'BREAKING CHANGE: ' clause");
}
const titleRe = /^[a-z]+\([0-9a-z-_]+\)/;
if (!titleRe.exec(title)) {
Expand All @@ -104,7 +109,7 @@ function validateBreakingChangeFormat(title: string, body: string) {

function assertStability(title: string, body: string) {
const breakingStable = breakingModules(title, body)
.filter(mod => 'stable' === moduleStability(findModulePath(mod)))
.filter(mod => 'stable' === moduleStability(findModulePath(mod)));

if (breakingStable.length > 0) {
throw new Error(`Breaking changes in stable modules [${breakingStable.join(', ')}] is disallowed.`);
Expand All @@ -114,40 +119,43 @@ function assertStability(title: string, body: string) {
export async function validatePr(number: number) {

if (!number) {
throw new Error('Must provide a PR number')
throw new Error('Must provide a PR number');
}

const gh = createGitHubClient();

const issues = gh.getIssues(OWNER, REPO);
const repo = gh.getRepo(OWNER, REPO);

console.log(`⌛ Fetching PR number ${number}`)
console.log(`⌛ Fetching PR number ${number}`);
const issue = (await issues.getIssue(number)).data;

console.log(`⌛ Fetching files for PR number ${number}`)
console.log(`⌛ Fetching files for PR number ${number}`);
const files = (await repo.listPullRequestFiles(number)).data;

console.log("⌛ Validating...");

if (shouldExemptReadme(issue)) {
console.log(`Not validating README changes since the PR is labeled with '${EXEMPT_README}'`)
console.log(`Not validating README changes since the PR is labeled with '${EXEMPT_README}'`);
} else {
featureContainsReadme(issue, files);
}

if (shouldExemptTest(issue)) {
console.log(`Not validating test changes since the PR is labeled with '${EXEMPT_TEST}'`)
console.log(`Not validating test changes since the PR is labeled with '${EXEMPT_TEST}'`);
} else {
featureContainsTest(issue, files);
fixContainsTest(issue, files);
}

validateBreakingChangeFormat(issue.title, issue.body);
if (shouldExemptBreakingChange(issue)) {
console.log(`Not validating breaking changes since the PR is labeled with '${EXEMPT_BREAKING_CHANGE}'`);
} else {
assertStability(issue.title, issue.body);
}

assertStability(issue.title, issue.body)

console.log("✅ Success")
console.log("✅ Success");

}

Expand Down
13 changes: 13 additions & 0 deletions tools/@aws-cdk/prlint/test/lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ describe('ban breaking changes in stable modules', () => {
await expect(linter.validatePr(1000)).rejects.toThrow('Breaking changes in stable modules [lambda, ecs] is disallowed.');
});

test('unless exempt-breaking-change label added', async () => {
const issue = {
title: 'chore(lambda): some title',
body: `
BREAKING CHANGE: this breaking change
continued message
`,
labels: [{ name: 'pr-linter/exempt-breaking-change' }],
};
configureMock(issue, undefined);
await expect(linter.validatePr(1000)).resolves; // not throw
});

test('with additional "closes" footer', async () => {
const issue = {
title: 'chore(s3): some title',
Expand Down

0 comments on commit f1bc543

Please sign in to comment.