appconfig: Constrain environments to a single deployment at a time

[M-Hawkins]

Describe the feature

Summary

The current L2 constructs make it easy to create Cfn deploy-time conflicts. They do not expose any mechanisms to resolve concurrent deployment without leveraging escape hatches or ignoring the L2 methods in favor of manual L1 deployments.

Context

AppConfig requires that its resources are deployed in a particular order:

• Create an application
• Create an environment
• Create a configuration profile
• Create a deployment strategy
• Deploy the configuration

The last of these is modeled as CfnDeployment, and consists of:

• The Environment where the deployment will occur
• The Configuration that will be deployed
• The Strategy that will be utilized for this deployment

The Problem

With the newly added L2 constructs, the creation of deployments is owned by the Configuration (both Hosted and Shared use the same base class implementation).

This creates a problem, as no individual Configuration is aware of another.
If more than one Configuration intends to deploy to the same Environment, they will attempt to do so at the same time, resulting in a Cfn failure.
Per the AppConfig docs:

Note
You can only deploy one configuration at a time to an environment. However, you can deploy one configuration each to different environments at the same time.

In other words, this extension of the basic use case will fail to deploy:

const app = new appconfig.Application(this, 'MyApp');
const env = new appconfig.Environment(this, 'MyEnv', {
  application: app,
});

new appconfig.HostedConfiguration(this, 'MyHostedConfig', {
  application: app,
  deployTo: [env],
  content: appconfig.ConfigurationContent.fromInlineText('This is my configuration content.'),
});

new appconfig.HostedConfiguration(this, 'MyHostedConfig2', {
  application: app,
  deployTo: [env],
  content: appconfig.ConfigurationContent.fromInlineText('This is my second configuration content.'),
});

Use Case

It is common and expected for an AppConfig Environment to host more than one Configuration.
The current behavior makes it impossible to rely solely on the L2 constructs and impacts our adoption of them.

Proposed Solution

Migrate the ownership and creation of CfnDeployment from Configuration to Environment.
Then, Cfn dependencies can be created whenever a new CfnDeployment is added, such that each deployment depends on the previous one.
This will ensure that only one CfnDeployment is in progress at any given point in time, avoiding deploy-time conflicts.

Prototype solution

In environment.ts, first expose new functionality for IEnvironment.

export interface IEnvironment extends IResource {
 
 ...

  /**
   * Creates a deployment of the supplied configuration to this environment.
   * Note that you can only deploy one configuration at a time to an environment.
   * However, you can deploy one configuration each to different environments at the same time.
   * If more than one deployment is requested for this environment, they will occur in the same order they were provided.
   *
   * @param configuration The configuration that will be deployed to this environment.
   */
  addDeployment(configuration: IConfiguration): void;

  /**
   * Creates a deployment for each of the supplied configurations to this environment.
   *
   * @param configurations The configurations that will be deployed to this environment.
   */
  addDeployments(...configurations: Array<IConfiguration>): void;

}

Then, implement this new functionality as a part of EnvironmentBase.

abstract class EnvironmentBase extends Resource implements IEnvironment, IExtensible {
  public abstract applicationId: string;
  public abstract environmentId: string;
  public abstract environmentArn: string;
  protected extensible!: ExtensibleBase;
  protected deploymentQueue: Array<CfnDeployment> = [];

  public addDeployment(configuration: IConfiguration): void {
    const queueSize = this.deploymentQueue.push(new CfnDeployment(this, `Deployment${getHash(this.environmentArn)}`, {
      applicationId: configuration.application.applicationId,
      configurationProfileId: configuration.configurationProfileId,
      deploymentStrategyId: configuration.deploymentStrategy!.deploymentStrategyId,
      environmentId: this.environmentId,
      configurationVersion: configuration.versionNumber!,
      description: configuration.description,
      kmsKeyIdentifier: configuration.deploymentKey?.keyArn,
    }));

    if (queueSize > 1) {
      this.deploymentQueue[queueSize - 1].addDependency(this.deploymentQueue[queueSize - 2]);
    }
  }

  public addDeployments(...configurations: IConfiguration[]): void {
    configurations.forEach(this.addDeployment);
  }

  ...

}

Then, change the existing internal helper in configuration.ts to utilize this function instead.

  protected deployConfigToEnvironments() {
    if (!this.deployTo || !this.versionNumber) {
      return;
    }

    this.application.environments().forEach((environment) => {
      if ((this.deployTo && !this.deployTo.includes(environment))) {
        return;
      }
      environment.addDeployment(this);
    });
  }

Other Information

No response

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

2.130.0

Environment details (OS name and version, etc.)

Various Linux

[pahud]

Thank you for your feedback. This LGTM but we'll discuss with the team this week.

[M-Hawkins]

Thanks!

In the meantime, here's the simplified (single-env) workaround we were looking at when considering the migration to L2 constructs.

// The L2 constructs currently create every deployment in isolation.
// This means that deploying >1 config to the same env will fail, as simultaneous deployments are not allowed.
// To avoid this, we locate the underlying deployment object and create a dependency on the prior one.
// This approach is simplified for the single-env use case, and as such only uses a single queue.
const deploymentQueue: Array<CfnDeployment> = [];
hostedConfigurations.forEach((namedConfigurationOptions) =>
    mainApp
        .addHostedConfiguration(
            `HC-${namedConfigurationOptions.name}`,
            {
                deployTo: [mainEnv],
                deploymentStrategy,
                ...namedConfigurationOptions,
            }
        )
        .node.children.forEach((child) => {
            if (child instanceof CfnDeployment) {
                deploymentQueue.push(child);
            }
        })
);
for (let i = 1; i < deploymentQueue.length; i += 1) {
    deploymentQueue[i].addDependency(deploymentQueue[i - 1]);
}

[chenjane-dev]

Hi @M-Hawkins, have you tried adding dependencies on the configs so only one is created at a time?

const firstConfig = new appconfig.HostedConfiguration(this, 'MyHostedConfig', {
  application: app,
  deployTo: [env],
  content: appconfig.ConfigurationContent.fromInlineText('This is my configuration content.'),
});

const secondConfig = new appconfig.HostedConfiguration(this, 'MyHostedConfig2', {
  application: app,
  deployTo: [env],
  content: appconfig.ConfigurationContent.fromInlineText('This is my second configuration content.'),
});

secondConfig.node.addDependency(firstConfig);

[M-Hawkins]

Hello, while that might* work for my simplified use case, it is not something I'd want to encourage in general.
Once you start working with Configurations that you deploy to more than one Environment you're creating unnecessary bottlenecks, resulting in periods where zero deployments are happening for a given Environment.

To put it another way - it is inaccurate to say that there is a dependency between the Configurations.
Only their Deployments (within the context of a shared Environment) need to be aware of each other.

---

*I would need to test your suggestion, it might not even work.
The Deployment is a proper Cfn resource that depends on the Configuration to exist, not the other way around.
If the only wait is for the Configuration to deploy, I think the two Deployments could still end up happening at the same time, depending on your deployment strategies.

[xazhao]

Hello @M-Hawkins thanks for raising this issue. Your proposed solution looks good to me in general. I just have a few questions:

1. Will it impact the current usage?
2. What kind of breaking changes it might incur?

[M-Hawkins]

Hello @xazhao
Unfortunately I'm not 100% certain, so I checked that box just to be sure.
We're moving the scope, which I am not familiar with the implications of.
At a minimum, we'll need to change the id pattern. My snippet above is also insufficient.

Today the CfnDeployment's id is generated from the environment's name.
Since the scope is now the Environment, the differentiation needs to be per-Configuration instead (as multiple Deployments can be created), perhaps by the configurationProfileId.
I'm not sure what the implications of changing said id are - if it impacts resource naming, I imagine that would be breaking.

[M-Hawkins]

If the current name! usage is acceptable, then I believe we should be able to do the same in EnvironmentBase.
Then we could keep the deployment scoped to the config, but still have the environment manage the sequencing of Deployments.
I believe that'd make this non-breaking.

  public abstract name: string | undefined;
  protected deploymentQueue: Array<CfnDeployment> = [];

  public addDeployment(configuration: IConfiguration): void {
    const queueSize = this.deploymentQueue.push(new CfnDeployment(configuration, `Deployment${getHash(this.name!)}`, {
	...
  }

[GavinZZ]

Hi @M-Hawkins, thanks for the detailed proposal and answering questions. It would be great if it doesn't incur a breaking change for current users. However, even if it does, we can work on wrapping everything inside feature flags.

I love the solution you provided so feel free to come up with a PR and we can get more detailed reviews on the PR.

[M-Hawkins]

Hello @GavinZZ , I've opened a pull request now, pending reviewer assignment.

[M-Hawkins]

Hello again, it's been a few weeks since my PR received community review and approval.
Is there anything missing for getting a maintainer to take a look?

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.