feat(appconfig): constrain environments to a single deployment at a time #29500

M-Hawkins · 2024-03-15T01:25:49Z

Issue # (if applicable)

Reason for this change

The current L2 AppConfig constructs do not have any guardrails
that prevent simultaneous Deployments to a single Environment.
This is not allowed, and will result in Cfn deploy-time conflicts.

Description of changes

This commit adds a pair of new public methods to IEnvironment that
enable the addition of a new Deployment for a given IConfiguration.

It then updates the creation of new Deployments in ConfigurationBase
to utilize these new methods instead of the current resource creation.

These new methods interact with an internal queue.
This queue creates a chain of Cfn dependencies between Deployments
in order to enforce that only a single Deployment can be in progress
for the Environment at any given time.

Description of how you validated changes

Added new unit and integ test coverage.
Deployed the new integ test without these changes and confirmed that Cfn failed at deployment time.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

lpizzinidev

Looks good overall 👍
Left notes for some adjustments

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

lpizzinidev · 2024-03-16T16:39:26Z

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

@@ -154,6 +179,7 @@ export class Environment extends EnvironmentBase {
      public readonly applicationId = applicationId;
      public readonly environmentId = environmentId;
      public readonly environmentArn = environmentArn;
+      public readonly name?: string | undefined;


Suggested change

public readonly name?: string | undefined;

public readonly name?: string;

I'm not sure this will be set so probably for environments imported with fromEnvironmentArn the this.name! in addDeployment will throw.
Can you please verify?

Yes, it will throw in that scenario and when the name is not provided to fromEnvironmentAttributes.
My understanding is that behavior is the same for the current implementation.

packages/aws-cdk-lib/aws-appconfig/test/environment.test.ts

packages/aws-cdk-lib/aws-appconfig/lib/configuration.ts

packages/aws-cdk-lib/aws-appconfig/README.md

lpizzinidev

Thanks 👍

lpizzinidev · 2024-03-17T18:01:13Z

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

@@ -154,6 +183,7 @@ export class Environment extends EnvironmentBase {
      public readonly applicationId = applicationId;
      public readonly environmentId = environmentId;
      public readonly environmentArn = environmentArn;
+      public readonly name? = undefined;


Suggested change

public readonly name? = undefined;

public readonly name = undefined;

I think having name as an optional property here is ok as it's only required when using the new methods.

lpizzinidev · 2024-03-17T18:05:03Z

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

+
+  public addDeployment(configuration: IConfiguration): void {
+    if (this.name === undefined) {
+      throw new Error('Environment name must be known to add a Deployment');


Just a note.
We can probably change this behavior by using the environmentId to build the CfnDeployment id.
However, it would require a feature flag and it's out of scope for this PR.

paulhcsun

Thanks for the contribution @M-Hawkins and thank you for your patience in waiting for a review! The changes looks reasonable to me, I've left a few comments on the descriptions and one question on the login in addDeployment().

packages/aws-cdk-lib/aws-appconfig/README.md

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

paulhcsun · 2024-04-18T00:00:19Z

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

@@ -154,6 +183,7 @@ export class Environment extends EnvironmentBase {
      public readonly applicationId = applicationId;
      public readonly environmentId = environmentId;
      public readonly environmentArn = environmentArn;
+      public readonly name? = undefined;


I think having name as an optional property here is ok as it's only required when using the new methods.

packages/aws-cdk-lib/aws-appconfig/lib/environment.ts

M-Hawkins · 2024-04-18T01:26:00Z

Thanks for taking a look @paulhcsun !
I've pushed a new commit based on your feedback.

paulhcsun · 2024-04-18T23:09:38Z

@Mergifyio update

paulhcsun · 2024-04-18T23:10:05Z

Thanks again for this contribution @M-Hawkins!! And thanks as always for the review @lpizzinidev!

mergify · 2024-04-18T23:10:12Z

update

✅ Branch has been successfully updated

mergify · 2024-04-18T23:10:14Z

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

paulhcsun · 2024-04-18T23:29:49Z

https://github.com/Mergifyio update

mergify · 2024-04-18T23:29:56Z

update

☑️ Nothing to do

#commits-behind>0 [📌 update requirement]
-closed [📌 update requirement]
-conflict [📌 update requirement]
queue-position=-1 [📌 update requirement]

aws-cdk-automation · 2024-04-19T19:57:13Z

@Mergifyio update

mergify · 2024-04-19T19:57:27Z

update

✅ Branch has been successfully updated

mergify · 2024-04-19T20:26:33Z

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

aws-cdk-automation · 2024-04-19T21:57:41Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 871d095
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2024-04-19T21:58:03Z

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

github-actions bot added feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Mar 15, 2024

aws-cdk-automation requested a review from a team March 15, 2024 01:26

aws-cdk-automation previously requested changes Mar 15, 2024

View reviewed changes

M-Hawkins changed the title ~~feat(appconfig): Constrain environments to a single deployment at a time~~ feat(appconfig): constrain environments to a single deployment at a time Mar 15, 2024

M-Hawkins force-pushed the 29345-appconfig branch from a6b974d to 39218f8 Compare March 15, 2024 02:27

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Mar 15, 2024

M-Hawkins force-pushed the 29345-appconfig branch 5 times, most recently from 17211d0 to e64d69d Compare March 15, 2024 23:57

shikha372 assigned shikha372 and unassigned shikha372 Mar 16, 2024

M-Hawkins force-pushed the 29345-appconfig branch from e64d69d to 5f7015f Compare March 16, 2024 03:09

lpizzinidev suggested changes Mar 16, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Mar 16, 2024

lpizzinidev approved these changes Mar 17, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 17, 2024

github-actions bot mentioned this pull request Mar 18, 2024

Weekly PR metrics report #29527

Closed

M-Hawkins force-pushed the 29345-appconfig branch from 72448c3 to 02fb3f2 Compare March 20, 2024 04:12

M-Hawkins force-pushed the 29345-appconfig branch from 02fb3f2 to 1121054 Compare March 30, 2024 02:06

github-actions bot mentioned this pull request Apr 1, 2024

Monthly PR metrics report #29668

Closed

M-Hawkins force-pushed the 29345-appconfig branch from 1121054 to 9673773 Compare April 13, 2024 03:41

paulhcsun self-assigned this Apr 17, 2024

paulhcsun reviewed Apr 18, 2024

View reviewed changes

M-Hawkins force-pushed the 29345-appconfig branch from 9673773 to 54b8341 Compare April 18, 2024 00:27

Merge branch 'main' into 29345-appconfig

b444a58

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Apr 18, 2024

paulhcsun changed the title ~~feat(appconfig): constrain environments to a single deployment at a time~~ feat(appconfig): constrain environments to a single deployment at a time. Apr 18, 2024

paulhcsun changed the title ~~feat(appconfig): constrain environments to a single deployment at a time.~~ feat(appconfig): constrain environments to a single deployment at a time Apr 18, 2024

Merge branch 'main' into 29345-appconfig

49737ee

Merge branch 'main' into 29345-appconfig

871d095

mergify bot merged commit 3dd834d into aws:main Apr 19, 2024
9 checks passed

This was referenced May 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#98

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#99

Open

rwlxxvii mentioned this pull request May 27, 2024

[Snyk] Upgrade aws-cdk-lib from 2.136.1 to 2.140.0 rwlxxvii/containers#124

Merged

NOUIY mentioned this pull request May 30, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#101

Open

NOUIY mentioned this pull request Jun 7, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#102

Open

niravparikh05 mentioned this pull request Jun 13, 2024

[Snyk] Upgrade aws-cdk from 2.65.0 to 2.142.1 paralus/eks-blueprints-addon#3

Open

NOUIY mentioned this pull request Jun 14, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#103

Open

snyk-io bot mentioned this pull request Jun 20, 2024

[Snyk] Upgrade aws-cdk-lib from 2.138.0 to 2.143.1 Opetushallitus/heratepalvelu#280

Open

NOUIY mentioned this pull request Jun 20, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#104

Open

snyk-io bot mentioned this pull request Jun 21, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.143.1 Opetushallitus/ovara#36

Open

rwlxxvii mentioned this pull request Jun 22, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.144.0 rwlxxvii/containers#140

Open

NOUIY mentioned this pull request Jun 22, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#105

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(appconfig): constrain environments to a single deployment at a time #29500

feat(appconfig): constrain environments to a single deployment at a time #29500

M-Hawkins commented Mar 15, 2024 •

edited

Loading

aws-cdk-automation left a comment

lpizzinidev left a comment

lpizzinidev Mar 16, 2024

M-Hawkins Mar 16, 2024

lpizzinidev left a comment

lpizzinidev Mar 17, 2024

paulhcsun Apr 18, 2024

lpizzinidev Mar 17, 2024

paulhcsun left a comment

paulhcsun Apr 18, 2024

M-Hawkins commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

mergify bot commented Apr 18, 2024

mergify bot commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

mergify bot commented Apr 18, 2024

aws-cdk-automation commented Apr 19, 2024

mergify bot commented Apr 19, 2024

mergify bot commented Apr 19, 2024

aws-cdk-automation commented Apr 19, 2024

mergify bot commented Apr 19, 2024

	public readonly name?: string \| undefined;
	public readonly name?: string;

	public readonly name? = undefined;
	public readonly name = undefined;

feat(appconfig): constrain environments to a single deployment at a time #29500

feat(appconfig): constrain environments to a single deployment at a time #29500

Conversation

M-Hawkins commented Mar 15, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Mar 16, 2024

Choose a reason for hiding this comment

M-Hawkins Mar 16, 2024

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Mar 17, 2024

Choose a reason for hiding this comment

paulhcsun Apr 18, 2024

Choose a reason for hiding this comment

lpizzinidev Mar 17, 2024

Choose a reason for hiding this comment

paulhcsun left a comment

Choose a reason for hiding this comment

paulhcsun Apr 18, 2024

Choose a reason for hiding this comment

M-Hawkins commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

mergify bot commented Apr 18, 2024

✅ Branch has been successfully updated

mergify bot commented Apr 18, 2024

paulhcsun commented Apr 18, 2024

mergify bot commented Apr 18, 2024

☑️ Nothing to do

aws-cdk-automation commented Apr 19, 2024

mergify bot commented Apr 19, 2024

✅ Branch has been successfully updated

mergify bot commented Apr 19, 2024

aws-cdk-automation commented Apr 19, 2024

AWS CodeBuild CI Report

mergify bot commented Apr 19, 2024

M-Hawkins commented Mar 15, 2024 •

edited

Loading