From aef795fc67ed5daec40d0616ad35d51549413eba Mon Sep 17 00:00:00 2001 From: Akash Askoolum Date: Fri, 12 Feb 2021 15:21:46 +0000 Subject: [PATCH 1/2] chore: add regex for ACM ARNs --- src/constants/regex-pattern.test.ts | 6 ++++++ src/constants/regex-pattern.ts | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/constants/regex-pattern.test.ts b/src/constants/regex-pattern.test.ts index 6dab0bc0a..dabea84a6 100644 --- a/src/constants/regex-pattern.test.ts +++ b/src/constants/regex-pattern.test.ts @@ -25,4 +25,10 @@ describe("the regex patterns", () => { expect(regex.test("another.@theguardian.com")).toBeFalsy(); expect(regex.test("another1@theguardian.com")).toBeFalsy(); }); + + it("should successfully regex against ACM ARNs", () => { + const regex = new RegExp(RegexPattern.ACM_ARN); + expect(regex.test("arn:aws:acm:eu-west-1:000000000000:certificate/123abc-0000-0000-0000-123abc")).toBeTruthy(); + expect(regex.test("arn:aws:acm:eu-west-1:000000000000:tls/123abc-0000-0000-0000-123abc")).toBeFalsy(); + }); }); diff --git a/src/constants/regex-pattern.ts b/src/constants/regex-pattern.ts index 8e682dd35..7bdd79cae 100644 --- a/src/constants/regex-pattern.ts +++ b/src/constants/regex-pattern.ts @@ -5,8 +5,12 @@ const s3ArnRegex = `arn:aws:s3:::${s3BucketRegex}*`; const emailRegex = "^[a-zA-Z]+(\\.[a-zA-Z]+)*@theguardian.com$"; +// TODO be more strict on region? +const acmRegex = "arn:aws:acm:[0-9a-z\\-]+:[0-9]{12}:certificate/[0-9a-z\\-]+"; + export const RegexPattern = { ARN: arnRegex, S3ARN: s3ArnRegex, GUARDIAN_EMAIL: emailRegex, + ACM_ARN: acmRegex, }; From bea9c61ed19aacdfb75dbfca66b563dc0df13df3 Mon Sep 17 00:00:00 2001 From: Akash Askoolum Date: Fri, 12 Feb 2021 15:47:03 +0000 Subject: [PATCH 2/2] feat: create GuCertificateArnParameter construct GuCertificateArnParameter has an allowed pattern of an ACM ARN string. Update GuHttpsApplicationListener to use it and validate the certificate prop too. --- src/constructs/core/parameters.ts | 10 ++++++++ src/constructs/loadbalancing/elb.test.ts | 32 +++++++++++++++++++----- src/constructs/loadbalancing/elb.ts | 15 ++++++++--- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/constructs/core/parameters.ts b/src/constructs/core/parameters.ts index 23a7be747..975efd2aa 100644 --- a/src/constructs/core/parameters.ts +++ b/src/constructs/core/parameters.ts @@ -110,3 +110,13 @@ export class GuGuardianEmailSenderParameter extends GuStringParameter { }); } } + +export class GuCertificateArnParameter extends GuStringParameter { + constructor(scope: GuStack, id: string = "TLSCertificate", props?: GuNoTypeParameterProps) { + super(scope, id, { + ...props, + allowedPattern: RegexPattern.ACM_ARN, + constraintDescription: "Must be an ACM ARN resource", + }); + } +} diff --git a/src/constructs/loadbalancing/elb.test.ts b/src/constructs/loadbalancing/elb.test.ts index 26059d778..5b9a03464 100644 --- a/src/constructs/loadbalancing/elb.test.ts +++ b/src/constructs/loadbalancing/elb.test.ts @@ -5,6 +5,7 @@ import { ApplicationProtocol, ListenerAction } from "@aws-cdk/aws-elasticloadbal import { Stack } from "@aws-cdk/core"; import { simpleGuStackForTesting } from "../../../test/utils/simple-gu-stack"; import type { SynthedStack } from "../../../test/utils/synthed-stack"; +import { RegexPattern } from "../../constants"; import { GuApplicationListener, GuApplicationLoadBalancer, @@ -331,10 +332,10 @@ describe("The GuHttpsApplicationListener class", () => { const json = SynthUtils.toCloudFormation(stack) as SynthedStack; - expect(json.Parameters.CertificateARN).toEqual({ + expect(json.Parameters.TLSCertificate).toEqual({ Type: "String", - AllowedPattern: "arn:aws:[a-z0-9]*:[a-z0-9\\-]*:[0-9]{12}:.*", - ConstraintDescription: "Must be a valid ARN, eg: arn:partition:service:region:account-id:resource-id", + AllowedPattern: RegexPattern.ACM_ARN, + ConstraintDescription: "Must be an ACM ARN resource", Description: "Certificate ARN for ApplicationListener", }); @@ -342,13 +343,32 @@ describe("The GuHttpsApplicationListener class", () => { Certificates: [ { CertificateArn: { - Ref: "CertificateARN", + Ref: "TLSCertificate", }, }, ], }); }); + test("passing in an invalid ACM ARN", () => { + const stack = simpleGuStackForTesting(); + + const loadBalancer = new GuApplicationLoadBalancer(stack, "ApplicationLoadBalancer", { vpc }); + const targetGroup = new GuApplicationTargetGroup(stack, "GrafanaInternalTargetGroup", { + vpc: vpc, + protocol: ApplicationProtocol.HTTP, + }); + + expect( + () => + new GuHttpsApplicationListener(stack, "ApplicationListener", { + loadBalancer, + targetGroup, + certificate: "test", + }) + ).toThrowError(new Error("test is not a valid ACM ARN")); + }); + test("does not create certificate prop if a value passed in", () => { const stack = simpleGuStackForTesting(); @@ -361,7 +381,7 @@ describe("The GuHttpsApplicationListener class", () => { new GuHttpsApplicationListener(stack, "ApplicationListener", { loadBalancer, targetGroup, - certificate: "test", + certificate: "arn:aws:acm:eu-west-1:000000000000:certificate/123abc-0000-0000-0000-123abc", }); const json = SynthUtils.toCloudFormation(stack) as SynthedStack; @@ -371,7 +391,7 @@ describe("The GuHttpsApplicationListener class", () => { expect(stack).toHaveResource("AWS::ElasticLoadBalancingV2::Listener", { Certificates: [ { - CertificateArn: "test", + CertificateArn: "arn:aws:acm:eu-west-1:000000000000:certificate/123abc-0000-0000-0000-123abc", }, ], }); diff --git a/src/constructs/loadbalancing/elb.ts b/src/constructs/loadbalancing/elb.ts index 9c5e30752..cb48eded4 100644 --- a/src/constructs/loadbalancing/elb.ts +++ b/src/constructs/loadbalancing/elb.ts @@ -15,8 +15,9 @@ import { Protocol, } from "@aws-cdk/aws-elasticloadbalancingv2"; import { Duration } from "@aws-cdk/core"; +import { RegexPattern } from "../../constants"; import type { GuStack } from "../core"; -import { GuArnParameter } from "../core"; +import { GuCertificateArnParameter } from "../core"; interface GuApplicationLoadBalancerProps extends ApplicationLoadBalancerProps { overrideId?: boolean; @@ -84,6 +85,13 @@ export interface GuHttpsApplicationListenerProps export class GuHttpsApplicationListener extends ApplicationListener { constructor(scope: GuStack, id: string, props: GuHttpsApplicationListenerProps) { + if (props.certificate) { + const isValid = new RegExp(RegexPattern.ACM_ARN).test(props.certificate); + if (!isValid) { + throw new Error(`${props.certificate} is not a valid ACM ARN`); + } + } + const mergedProps: GuApplicationListenerProps = { port: 443, protocol: ApplicationProtocol.HTTPS, @@ -92,8 +100,9 @@ export class GuHttpsApplicationListener extends ApplicationListener { { certificateArn: props.certificate ?? - new GuArnParameter(scope, "CertificateARN", { description: "Certificate ARN for ApplicationListener" }) - .valueAsString, + new GuCertificateArnParameter(scope, "TLSCertificate", { + description: `Certificate ARN for ${id}`, + }).valueAsString, }, ], defaultAction: ListenerAction.forward([props.targetGroup]),