feat(codebuild): allow taking the artifact name from the buildspec #7384

shearn89 · 2020-04-16T11:51:32Z

Commit Message

feat(codebuild): allow taking the artifact name from the buildspec

This commit adds support for CodeBuild Artifacts to have names
controlled by a buildspec file, allowing the use of shell scripting to
sensibly name artifacts in CodeBuild projects generated from the CDK.
The previosuly required field name in S3ArtifactsProps is now optional,
and not providing it will set the overrideArtifactName on the underlying L1 to true.

Fixes #5955

End Commit Message

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

This commit adds support for CodeBuild Artifacts to have names controlled by a buildspec file, allowing the use of shell scripting to sensibly name artifacts in CodeBuild projects generated from the CDK. Fixes #5955

aws-cdk-automation · 2020-04-16T12:00:20Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: eb965d8
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-16T12:05:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: d524887
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-16T12:37:04Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c1f6757
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…o-s3

aws-cdk-automation · 2020-04-19T19:39:06Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: cf6d658
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…o-s3

aws-cdk-automation · 2020-04-21T08:20:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 7c171e7
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…o-s3

aws-cdk-automation · 2020-04-21T15:41:12Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ecef1e7
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…o-s3

shearn89 · 2020-04-26T10:33:35Z

@skinny85 - Hi, any chance this can get reviewed in the next week or so? It's a feature that would unblock me at work! Hopefully it's a small enough change, despite the various merges from master.

aws-cdk-automation · 2020-04-26T10:43:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: cc930e6
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-26T12:59:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 395d53f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

skinny85 · 2020-04-27T15:22:26Z

I'll review it today - apologies for the delay @shearn89 !

skinny85

Thanks a lot for the contribution @shearn89 ! I have a few small comments, and one idea I wanted to run by you.

So, when useBuildSpecName is true, we ignore the name property completely. I wonder whether a slightly different design might work here.

Currently, name is required in S3ArtifactsProps. I wonder if, instead of adding the new useBuildSpecName property, we could make name optional. If name was not provided, we will just set overrideArtifactName in the implementation to true, and pass either undefined, or some random value (like project.node.uniqueId) for name (it's optional in the CFN resource, but we need to check whether its presence is not validated by the CodeBuild backend APIs).

What do you think?

packages/@aws-cdk/aws-codebuild/README.md

packages/@aws-cdk/aws-codebuild/lib/artifacts.ts

Pull request has been modified.

shearn89 · 2020-04-28T14:12:45Z

Hey @skinny85 - thanks for the review! I'm just about to push a commit with the proposed design change. I think it makes sense, and we shouldn't be changing any existing behaviour as the parameter is going from mandatory to optional. Given that this PR is the first time the overrideArtifactName parameter has been exposed I can't see that we'd be altering any existing behaviour!

(My TypeScript is not brilliant, so hopefully I've done it correctly! 🤞 )

aws-cdk-automation · 2020-04-28T14:21:55Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ed1c25b
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

This commit reworks the implementation to instead make `name` optional. If the `name` is not provided, `overrideArtifactName` will be set and the buildspec name will be used.

aws-cdk-automation · 2020-04-28T15:06:29Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3906e85
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…o-s3

aws-cdk-automation · 2020-04-28T15:59:29Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: a7f78b9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

skinny85

@shearn89 , this looks amazing. I have one last question before I merge this in.

Thanks so much!

skinny85 · 2020-04-28T21:09:35Z

packages/@aws-cdk/aws-codebuild/lib/artifacts.ts

        path: this.props.path,
        namespaceType: this.props.includeBuildId === false ? 'NONE' : 'BUILD_ID',
-        name: this.props.name,
+        name: this.props.name == null ? project.node.uniqueId : this.props.name,


Did you test that this is necessary - we can't just leave this as undefined? name is optional in CfnProject.ArtifactsProperty: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-codebuild.CfnProject.ArtifactsProperty.html#name

I didn't - I'll do so today and set it to undefined if that works.

Done, hopefully the integration tests pass!

Just to make sure we're on the same page - by default, the integration tests act like snapshot tests. You need to specifically run the npm integ target to have them deployed. More info here: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md#integration-tests

Ah, okay! I'll run those this evening and let you know the outcome...

Just ran the tests against my own account and all the aws-codebuild integration tests provision and tear down correctly, no errors visible in the CloudFormation console or build logs!

packages/@aws-cdk/aws-codebuild/README.md

Fixing typo. Co-Authored-By: Adam Ruka <[email protected]>

… node id

…o-s3

aws-cdk-automation · 2020-04-29T08:04:43Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 71ab99c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-29T08:22:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f5b99c3
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-29T08:29:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: dbf6778
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Pull request has been modified.

…o-s3

aws-cdk-automation · 2020-04-30T13:17:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8c76e8d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

skinny85

This is fantastic. I'm really happy with where we ended up with this feature. Thanks so much for the awesome contribution @shearn89 !

mergify · 2020-04-30T18:26:23Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…o-s3

aws-cdk-automation · 2020-04-30T19:09:10Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 12c9654
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-04-30T19:09:32Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

shearn89 and others added 2 commits April 16, 2020 12:49

feat(codebuild): Support OverrideArtifactName on S3BuildProps

eb965d8

This commit adds support for CodeBuild Artifacts to have names controlled by a buildspec file, allowing the use of shell scripting to sensibly name artifacts in CodeBuild projects generated from the CDK. Fixes #5955

Updating README for package

d524887

Fixing trailing whitespace

c1f6757

shearn89 changed the title ~~feat(codebuild): Support OverrideArtifactName on S3BuildProps~~ feat(codebuild): Expose the overrideArtifactName property on S3ArtifactsProps Apr 16, 2020

SomayaB assigned skinny85 Apr 16, 2020

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

cf6d658

…o-s3

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

7c171e7

…o-s3

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

ecef1e7

…o-s3

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

cc930e6

…o-s3

Fixing lint errors

395d53f

skinny85 previously requested changes Apr 27, 2020

View reviewed changes

packages/@aws-cdk/aws-codebuild/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-codebuild/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-codebuild/lib/artifacts.ts Outdated Show resolved Hide resolved

Updated with review comments from @skinny85

ed1c25b

Updating PR with design comments

3906e85

This commit reworks the implementation to instead make `name` optional. If the `name` is not provided, `overrideArtifactName` will be set and the buildspec name will be used.

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

a7f78b9

…o-s3

skinny85 previously requested changes Apr 28, 2020

View reviewed changes

shearn89 and others added 3 commits April 29, 2020 08:20

Update packages/@aws-cdk/aws-codebuild/README.md

71ab99c

Fixing typo. Co-Authored-By: Adam Ruka <[email protected]>

Setting name property to undefined rather than specifying the project…

f5b99c3

… node id

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

dbf6778

…o-s3

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

8c76e8d

…o-s3

skinny85 approved these changes Apr 30, 2020

View reviewed changes

Merge branch 'master' into shearn89/expose-buildspec-artifact-names-t…

12c9654

…o-s3

skinny85 changed the title ~~feat(codebuild): Expose the overrideArtifactName property on S3ArtifactsProps~~ feat(codebuild): allow taking the artifact name from the buildspec Apr 30, 2020

mergify bot merged commit f7d3cd6 into aws:master Apr 30, 2020

feat(codebuild): allow taking the artifact name from the buildspec #7384

feat(codebuild): allow taking the artifact name from the buildspec #7384

Uh oh!

Conversation

shearn89 commented Apr 16, 2020 • edited by skinny85 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commit Message

End Commit Message

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 19, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 21, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 21, 2020

AWS CodeBuild CI Report

Uh oh!

shearn89 commented Apr 26, 2020

Uh oh!

aws-cdk-automation commented Apr 26, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 26, 2020

AWS CodeBuild CI Report

Uh oh!

skinny85 commented Apr 27, 2020

Uh oh!

skinny85 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shearn89 commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation commented Apr 28, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 28, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 28, 2020

AWS CodeBuild CI Report

Uh oh!

skinny85 left a comment

Choose a reason for hiding this comment

Uh oh!

skinny85 Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

shearn89 Apr 29, 2020

Choose a reason for hiding this comment

Uh oh!

shearn89 Apr 29, 2020

Choose a reason for hiding this comment

Uh oh!

skinny85 Apr 29, 2020

Choose a reason for hiding this comment

Uh oh!

shearn89 Apr 29, 2020

Choose a reason for hiding this comment

Uh oh!

shearn89 Apr 30, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aws-cdk-automation commented Apr 29, 2020

AWS CodeBuild CI Report

Uh oh!

shearn89 commented Apr 16, 2020 •

edited by skinny85

Loading

shearn89 commented Apr 28, 2020 •

edited

Loading