Skip to content

Commit

Permalink
fix(cli): cdk ls returns stack id instead of stack display name (#2…
Browse files Browse the repository at this point in the history
…9447)

### Issue # (if applicable)

Closes #29420 

### Reason for this change

The `cdk list` functionality displays the stacks .

For instance
```
> cdk ls
producer
consumer
```
With the latest changes for list stack dependencies we did add a new flag `-d` to show the dependencies.

The dependencies between stacks can be established in 2 ways:
1. Using the resource defined from one stack in another.
2. Using `addDependency()` to add dependency among stacks.

Current we are fetching the dependency details through the `CloudStackArtifact`.

* Establishing the dependency between stacks through the first method would have the same `displayName` and `id` for the stacks.
Using the `-d` flag to display dependencies -

```
❯ cdk list --show-dependencies
- id: producer
  dependencies: []
- id: consumer
  dependencies:
    - id: producer
      dependencies: []
```
* Establishing the dependency through `addDependency()` will create a different `displayName` and `id`.
In such a case when a user runs `cdk ls` we would want to show the `displayName` and if not present then use the `id`

For instance:
```
> cdk ls
producer
producer/consumer
```
With the `-d` flag we would want to display something like:
```
> cdk ls -d
- id: producer
  dependencies: []
- id: producer/consumer
  dependencies:
    - id: producer
      dependencies: []
```
With our previous change we didn't consider `displayName` and just fetched `id`s which changes the previous functionality and caused a regression.

### Description of changes

With the new changes we are looking out for `displayName` first and if it does not exist we fetch the `id`.

### Description of how you validated changes

Added a new unit test and updated integ tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
SankyRed authored Mar 12, 2024
1 parent bd41b9f commit 77189be
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This patch brings the [fix](https://github.com/aws/aws-cdk/issues/29420) into the regression suite.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Skipping the test to fix issue https://github.com/aws/aws-cdk/issues/29420.
# cli-integ tests failing for the old tests with the new cli changes for list stacks.

cdk ls --show-dependencies --json
Original file line number Diff line number Diff line change
Expand Up @@ -886,10 +886,10 @@ integTest('cdk ls --show-dependencies --json', withDefaultFixture(async (fixture
id: 'list-stacks',
dependencies: [
{
id: 'liststacksDependentStack',
id: 'list-stacks/DependentStack',
dependencies: [
{
id: 'liststacksDependentStackInnerDependentStack',
id: 'list-stacks/DependentStack/InnerDependentStack',
dependencies: [],
},
],
Expand All @@ -900,11 +900,11 @@ integTest('cdk ls --show-dependencies --json', withDefaultFixture(async (fixture
id: 'list-multiple-dependent-stacks',
dependencies: [
{
id: 'listmultipledependentstacksDependentStack1',
id: 'list-multiple-dependent-stacks/DependentStack1',
dependencies: [],
},
{
id: 'listmultipledependentstacksDependentStack2',
id: 'list-multiple-dependent-stacks/DependentStack2',
dependencies: [],
},
],
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/list-stacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export async function listStacks(toolkit: CdkToolkit, options: ListStacksOptions

for (const stack of collectionOfStacks.stackArtifacts) {
const data: StackDetails = {
id: stack.id,
id: stack.displayName ?? stack.id,
name: stack.stackName,
environment: stack.environment,
dependencies: [],
Expand All @@ -82,7 +82,7 @@ export async function listStacks(toolkit: CdkToolkit, options: ListStacksOptions
}
} else {
data.dependencies.push({
id: depStack.stackArtifacts[0].id,
id: depStack.stackArtifacts[0].displayName ?? depStack.stackArtifacts[0].id,
dependencies: [],
});
}
Expand Down
92 changes: 90 additions & 2 deletions packages/aws-cdk/test/list-stacks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('list', () => {
dependencies: [],
},
{
id: 'Test-Stack-B',
id: 'Test-Stack-A/Test-Stack-B',
name: 'Test-Stack-B',
environment: {
account: '123456789012',
Expand All @@ -185,7 +185,7 @@ describe('list', () => {
}]));
});

test('stacks with nested dependencies', async () => {
test('stacks with display names and have nested dependencies', async () => {
let cloudExecutable = new MockCloudExecutable({
stacks: [
MockStack.MOCK_STACK_A,
Expand All @@ -201,9 +201,84 @@ describe('list', () => {
],
},
depends: ['Test-Stack-A'],
displayName: 'Test-Stack-A/Test-Stack-B',
},
{
stackName: 'Test-Stack-C',
template: { Resources: { TemplateName: 'Test-Stack-C' } },
env: 'aws://123456789012/bermuda-triangle-1',
metadata: {
'/Test-Stack-C': [
{
type: cxschema.ArtifactMetadataEntryType.STACK_TAGS,
},
],
},
depends: ['Test-Stack-B'],
displayName: 'Test-Stack-A/Test-Stack-B/Test-Stack-C',
},
],
});

// GIVEN
const toolkit = new CdkToolkit({
cloudExecutable,
configuration: cloudExecutable.configuration,
sdkProvider: cloudExecutable.sdkProvider,
deployments: cloudFormation,
});

// WHEN
const workflow = await listStacks( toolkit, { selectors: ['Test-Stack-A', 'Test-Stack-A/Test-Stack-B', 'Test-Stack-A/Test-Stack-B/Test-Stack-C'] });

// THEN
expect(JSON.stringify(workflow)).toEqual(JSON.stringify([{
id: 'Test-Stack-A',
name: 'Test-Stack-A',
environment: {
account: '123456789012',
region: 'bermuda-triangle-1',
name: 'aws://123456789012/bermuda-triangle-1',
},
dependencies: [],
},
{
id: 'Test-Stack-A/Test-Stack-B',
name: 'Test-Stack-B',
environment: {
account: '123456789012',
region: 'bermuda-triangle-1',
name: 'aws://123456789012/bermuda-triangle-1',
},
dependencies: [{
id: 'Test-Stack-A',
dependencies: [],
}],
},
{
id: 'Test-Stack-A/Test-Stack-B/Test-Stack-C',
name: 'Test-Stack-C',
environment: {
account: '123456789012',
region: 'bermuda-triangle-1',
name: 'aws://123456789012/bermuda-triangle-1',
},
dependencies: [{
id: 'Test-Stack-A/Test-Stack-B',
dependencies: [{
id: 'Test-Stack-A',
dependencies: [],
}],
}],
}]));
});

test('stacks with nested dependencies', async () => {
let cloudExecutable = new MockCloudExecutable({
stacks: [
MockStack.MOCK_STACK_A,
{
stackName: 'Test-Stack-B',
template: { Resources: { TemplateName: 'Test-Stack-B' } },
env: 'aws://123456789012/bermuda-triangle-1',
metadata: {
Expand All @@ -213,6 +288,19 @@ describe('list', () => {
},
],
},
depends: ['Test-Stack-A'],
},
{
stackName: 'Test-Stack-C',
template: { Resources: { TemplateName: 'Test-Stack-C' } },
env: 'aws://123456789012/bermuda-triangle-1',
metadata: {
'/Test-Stack-C': [
{
type: cxschema.ArtifactMetadataEntryType.STACK_TAGS,
},
],
},
depends: ['Test-Stack-B'],
},
],
Expand Down

0 comments on commit 77189be

Please sign in to comment.