Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(pipelines): versionReporting and --no-version-reporting not respected for support stacks #18933

Closed
gshpychka opened this issue Feb 11, 2022 · 3 comments · Fixed by #19010
Closed
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. p2

Comments

@gshpychka
Copy link
Contributor

What is the problem?

The pipeline is adding AWS::CDK::Metadata resource to the support stacks regardless of the versionReporting value and even if the synth command is cdk --no-version-reporting synth.

Reproduction Steps

  1. Have "versionReporting": false in cdk.json
  2. Alternatively, specify the synth command as cdk --no-version-reporting synth in the Synth step
  3. Deploy the pipeline. Verify that the support stacks do not have the AWS::CDK::Metadata resource.
  4. Wait for the pipeline to self-mutate the first time

What did you expect to happen?

No changes to the support stacks.

What actually happened?

The self-mutate action adds the AWS::CDK::Metadata to the support stacks.

If I do a cdk deploy from my local machine again, the resource will be deleted.

CDK CLI Version

2.12.0

Framework Version

2.12.0

Node.js Version

17.3.0

OS

ArcLinux

Language

Python

Language Version

3.9.10

Other information

No response

@gshpychka gshpychka added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 11, 2022
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Feb 11, 2022
@gshpychka gshpychka changed the title (pipelines): 'versionReporting and --no-version-reporting` not respected for support stacks (pipelines): versionReporting and --no-version-reporting not respected for support stacks Feb 11, 2022
@NGL321 NGL321 added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 14, 2022
@gshpychka
Copy link
Contributor Author

@NGL321 Why is nobody assigned this? Shouldn't the inability to disable telemetry be P1?

rix0rrr added a commit that referenced this issue Feb 17, 2022
When we moved analytics generation from the CLI to the framework, we
kept a fallback in the CLI for backwards compatibility: if there were no
analytics in the templates but analytics were requested, they would
still be added in the CLI.

This breaks in the case of CDK Pipelines self-mutate step, if analytics
have been disabled: there are no analytics in the template, and the
default setting for analytics is `true`, since there is no `cdk.json`
to opt out, so they will always be added back in.

A better solution to check whether the CLI needs to do the fallback
is to go off of the cx schema version number: the CLI can know that
it never needs to do anything for cloud assemblies generated starting
at a certain version.

Fixes #18933.
@mergify mergify bot closed this as completed in #19010 Feb 17, 2022
mergify bot pushed a commit that referenced this issue Feb 17, 2022
When we moved analytics generation from the CLI to the framework, we
kept a fallback in the CLI for backwards compatibility: if there were no
analytics in the templates but analytics were requested, they would
still be added in the CLI.

This breaks in the case of CDK Pipelines self-mutate step, if analytics
have been disabled: there are no analytics in the template, and the
default setting for analytics is `true`, since there is no `cdk.json`
to opt out, so they will always be added back in.

A better solution to check whether the CLI needs to do the fallback
is to go off of the cx schema version number: the CLI can know that
it never needs to do anything for cloud assemblies generated starting
at a certain version.

Fixes #18933.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️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.

@NGL321
Copy link
Contributor

NGL321 commented Feb 17, 2022

@gshpychka thank you for bringing this to our attention!
That was a good observation, I definitely under-prioritized this.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
When we moved analytics generation from the CLI to the framework, we
kept a fallback in the CLI for backwards compatibility: if there were no
analytics in the templates but analytics were requested, they would
still be added in the CLI.

This breaks in the case of CDK Pipelines self-mutate step, if analytics
have been disabled: there are no analytics in the template, and the
default setting for analytics is `true`, since there is no `cdk.json`
to opt out, so they will always be added back in.

A better solution to check whether the CLI needs to do the fallback
is to go off of the cx schema version number: the CLI can know that
it never needs to do anything for cloud assemblies generated starting
at a certain version.

Fixes aws#18933.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants