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.

Sign up for GitHub

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

Jump to bottom

feat(stack): GuStack accepts Stack value as prop #243

Merged
merged 5 commits into from
Feb 16, 2021
Merged

feat(stack): GuStack accepts Stack value as prop #243

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3196d28
feat(stack): allow GuStack to accept Stack and Stage values as props
Feb 11, 2021
8a0ee36
test(stack): add test for only passing one of stack or stage
Feb 11, 2021
6ebe629
Rework to only require stack
Feb 12, 2021
cf0fde3
feat(stack): enforce stack value at declaration
Feb 12, 2021
7719c76
Remove unnecessary extra test expectation
Feb 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions src/constructs/cloudwatch/__snapshots__/lambda-alarms.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
exports[`The GuLambdaErrorPercentageAlarm pattern should create the correct alarm resource with minimal config 1`] = `
Object {
"Parameters": Object {
"Stack": Object {
"Default": "deploy",
"Description": "Name of this stack",
"Type": "String",
},
"Stage": Object {
"AllowedValues": Array [
"CODE",
Expand Down Expand Up @@ -49,9 +44,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down Expand Up @@ -103,9 +96,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down
29 changes: 19 additions & 10 deletions src/constructs/core/stack.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,28 @@ import { TrackingTag } from "../../constants/library-info";
import { GuStack } from "./stack";

describe("The GuStack construct", () => {
it("should have stack and stage parameters", () => {
const stack = simpleGuStackForTesting();
it("requires passing the stack value as props", function () {
const stack = simpleGuStackForTesting({ stack: "some-stack" });
expect(stack.stack).toEqual("some-stack");

const json = SynthUtils.toCloudFormation(stack) as SynthedStack;

expect(json.Parameters).toEqual({
Copy link
Contributor

@jacobwinch jacobwinch Feb 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the expect(json.Parameters) assertion here, as it's tested as part of "should have a stage parameter"

Stack: {
Stage: {
Type: "String",
Description: "Name of this stack",
Default: "deploy",
Description: "Stage name",
AllowedValues: Stages,
Default: Stage.CODE,
},
});
});

it("should have a stage parameter", () => {
const stack = simpleGuStackForTesting({ stack: "test" });

const json = SynthUtils.toCloudFormation(stack) as SynthedStack;

expect(json.Parameters).toEqual({
Stage: {
Type: "String",
Description: "Stage name",
Expand All @@ -31,7 +42,7 @@ describe("The GuStack construct", () => {
});

it("should apply the stack, stage and app tags to resources added to it", () => {
const stack = new GuStack(new App(), "Test", { app: "MyApp" });
const stack = new GuStack(new App(), "Test", { app: "MyApp", stack: "test" });

new Role(stack, "MyRole", {
assumedBy: new ServicePrincipal("ec2.amazonaws.com"),
Expand All @@ -45,9 +56,7 @@ describe("The GuStack construct", () => {
},
{
Key: "Stack",
Value: {
Ref: "Stack",
},
Value: "test",
},
{
Key: "Stage",
Expand All @@ -61,7 +70,7 @@ describe("The GuStack construct", () => {
});

it("should return the correct app value when app is set", () => {
const stack = new GuStack(new App(), "Test", { app: "MyApp" });
const stack = new GuStack(new App(), "Test", { app: "MyApp", stack: "test" });

expect(stack.app).toBe("MyApp");
});
Expand Down
9 changes: 5 additions & 4 deletions src/constructs/core/stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { App, StackProps } from "@aws-cdk/core";
import { Stack, Tags } from "@aws-cdk/core";
import { TrackingTag } from "../../constants/library-info";
import { GuStackParameter, GuStageParameter } from "./parameters";
import { GuStageParameter } from "./parameters";

export interface GuStackProps extends StackProps {
// This limits GuStack to supporting a single app.
// In the future, support for stacks with multiple apps may be required
app: string;
stack: string;
migratedFromCloudFormation?: boolean;
}

Expand Down Expand Up @@ -39,7 +40,7 @@ export interface GuStackProps extends StackProps {
*/
export class GuStack extends Stack {
private readonly _stage: GuStageParameter;
private readonly _stack: GuStackParameter;
private readonly _stack: string;
private readonly _app: string;

public readonly migratedFromCloudFormation: boolean;
Expand All @@ -49,7 +50,7 @@ export class GuStack extends Stack {
}

get stack(): string {
return this._stack.valueAsString;
return this._stack;
}

get app(): string {
Expand Down Expand Up @@ -79,7 +80,7 @@ export class GuStack extends Stack {
this.migratedFromCloudFormation = !!props.migratedFromCloudFormation;

this._stage = new GuStageParameter(this);
this._stack = new GuStackParameter(this);
this._stack = props.stack;
this._app = props.app;

this.addTag(TrackingTag.Key, TrackingTag.Value);
Expand Down
9 changes: 1 addition & 8 deletions src/constructs/iam/policies/__snapshots__/ses.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ Object {
"ConstraintDescription": "Must be an @theguardian.com email address",
"Type": "String",
},
"Stack": Object {
"Default": "deploy",
"Description": "Name of this stack",
"Type": "String",
},
"Stage": Object {
"AllowedValues": Array [
"CODE",
Expand Down Expand Up @@ -98,9 +93,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down
8 changes: 2 additions & 6 deletions src/constructs/iam/policies/parameter-store-read.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { GuParameterStoreReadPolicy } from "./parameter-store-read";

describe("ParameterStoreReadPolicy", () => {
it("should constrain the policy to the patch of a stack's identity", () => {
const stack = new GuStack(new App(), "my-app", { app: "MyApp" });
const stack = new GuStack(new App(), "my-app", { app: "MyApp", stack: "test-stack" });

const policy = new GuParameterStoreReadPolicy(stack);

Expand Down Expand Up @@ -36,11 +36,7 @@ describe("ParameterStoreReadPolicy", () => {
{
Ref: "Stage",
},
"/",
{
Ref: "Stack",
},
"/MyApp",
"/test-stack/MyApp",
],
],
},
Expand Down
2 changes: 1 addition & 1 deletion src/constructs/iam/policies/s3-get-object.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe("The GuGetDistributablePolicy construct", () => {
const json = SynthUtils.toCloudFormation(stack) as SynthedStack;

const parameterKeys = Object.keys(json.Parameters);
const expectedKeys = ["Stage", "Stack", "DistributionBucketName"];
const expectedKeys = ["Stage", "DistributionBucketName"];
expect(parameterKeys).toEqual(expectedKeys);

expect(json.Parameters.DistributionBucketName).toEqual({
Expand Down
45 changes: 6 additions & 39 deletions src/constructs/iam/roles/__snapshots__/instance-role.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ Object {
"Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
"Type": "AWS::SSM::Parameter::Value<String>",
},
"Stack": Object {
"Default": "deploy",
"Description": "Name of this stack",
"Type": "String",
},
"Stage": Object {
"AllowedValues": Array [
"CODE",
Expand Down Expand Up @@ -139,9 +134,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down Expand Up @@ -176,11 +169,7 @@ Object {
Object {
"Ref": "Stage",
},
"/",
Object {
"Ref": "Stack",
},
"/testing",
"/test-stack/testing",
],
],
},
Expand Down Expand Up @@ -250,11 +239,6 @@ Object {
"Description": "SSM parameter containing the Name (not ARN) on the kinesis stream",
"Type": "AWS::SSM::Parameter::Value<String>",
},
"Stack": Object {
"Default": "deploy",
"Description": "Name of this stack",
"Type": "String",
},
"Stage": Object {
"AllowedValues": Array [
"CODE",
Expand Down Expand Up @@ -402,9 +386,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down Expand Up @@ -439,11 +421,7 @@ Object {
Object {
"Ref": "Stage",
},
"/",
Object {
"Ref": "Stack",
},
"/testing",
"/test-stack/testing",
],
],
},
Expand Down Expand Up @@ -508,11 +486,6 @@ Object {
"Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
"Type": "AWS::SSM::Parameter::Value<String>",
},
"Stack": Object {
"Default": "deploy",
"Description": "Name of this stack",
"Type": "String",
},
"Stage": Object {
"AllowedValues": Array [
"CODE",
Expand Down Expand Up @@ -618,9 +591,7 @@ Object {
},
Object {
"Key": "Stack",
"Value": Object {
"Ref": "Stack",
},
"Value": "test-stack",
},
Object {
"Key": "Stage",
Expand Down Expand Up @@ -655,11 +626,7 @@ Object {
Object {
"Ref": "Stage",
},
"/",
Object {
"Ref": "Stack",
},
"/testing",
"/test-stack/testing",
],
],
},
Expand Down
Loading