[Resolve #1318] Implement dump template and write output files #1325

alex-harvey-z3q · 2023-04-15T03:19:02Z

Description

Proposed implementation of #1318

This introduces a dump template command that behaves similarly to generate but also writes output docs to files.

The behaviour of dump config is changed here not to print multi-doc output to screen similar to dump config.

The write helper function is modified to optionally write to a file_path.

Writes files in a naming convention for dump config and dump template respectively:

f".dump/{stack_name}/config.yaml"
f".dump/{stack_name}/template.yaml"

Usage example

Dump config

% sceptre --merge-vars --dir=emr --var-file=/app/sceptre-environment/nonprod/datalake-nonprod/common-env.yaml --var-file=/app/sceptre-environment/common-env.yaml --var-file=/app/sceptre-environment/nonprod/datalake-nonprod/emr/streaming-nonprod-6100/cluster.yaml dump config emr.yaml > /dev/null

Dump template

% sceptre --merge-vars --dir=emr --var-file=/app/sceptre-environment/nonprod/datalake-nonprod/common-env.yaml --var-file=/app/sceptre-environment/common-env.yaml --var-file=/app/sceptre-environment/nonprod/datalake-nonprod/emr/streaming-nonprod-6100/cluster.yaml dump template emr.yaml > /dev/null

File structure created

% tree .dump/
.dump/
├── streaming-nonprod-6100-efs-pvt
│   ├── config.yaml
│   └── template.yaml
├── streaming-nonprod-6100-emr
│   ├── config.yaml
│   └── template.yaml
├── streaming-nonprod-6100-emr-profile
│   ├── config.yaml
│   └── template.yaml
└── streaming-nonprod-6100-emr-sg-pvt
    ├── config.yaml
    └── template.yaml

4 directories, 8 files

File content

% head .dump/streaming-nonprod-6100-emr/config.yaml 
---
dependencies:
- emr-profile.yaml
hooks:
  after_create:
  - !!python/object:hook.stack_termination_protection.StackTerminationProtection
    _argument: enabled
    _argument_is_resolved: false
    logger: &id001 !!python/object/apply:logging.getLogger
    - hook.stack_termination_protection

% head .dump/streaming-nonprod-6100-emr/template.yaml 
---
AWSTemplateFormatVersion: '2010-09-09'

Parameters:
  MasterInstanceType:
    Type: String
  CoreInstantType:
    Type: String
  CoreInstantCount:
    Type: String

PR Checklist

Wrote a good commit message & description [see guide below].
Commit message starts with [Resolve #issue-number].
Added/Updated unit tests.
Added/Updated integration tests (if applicable).
All unit tests (make test) are passing.
Used the same coding conventions as the rest of the project.
The new code passes pre-commit validations (pre-commit run --all-files).
The PR relates to only one subject with a clear title.
and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

Before merge squash related commits.

Other Information

Guide to writing a good commit

In the event that the reader class is unable to parse the rendered config, this adds logic to write the rendered config to a file to assist the caller in understand what went wrong.

This adds a sceptre dump template command and deprecates sceptre generate.

This reverts commit 3005b14.

This reverts commit acb399b.

This reverts commit 4d1e5cc.

This reverts commit c1ca8ed.

sceptre/cli/dump.py

jfalkenstein · 2023-04-15T18:29:43Z

I feel like there must be a better way. Is there a way we can fix the generate command?

CONTRIBUTING.md

dboitnot · 2023-04-17T21:44:22Z

I feel like there must be a better way. Is there a way we can fix the generate command?

Maybe a flag like --dump or something to the generate sub-command?

alex-harvey-z3q · 2023-04-17T23:50:43Z

I feel like there must be a better way. Is there a way we can fix the generate command?

@jfalkenstein Well if we fixed the generate command (we could copy this functionality there instead), we are then left with:

A breaking change
A seriously ugly inconsistency in the naming and behaviour between dump config and generate.

I think this is actually quite clean and a big improvement myself. (Not that I really care either way.)

alex-harvey-z3q · 2023-04-18T00:33:26Z

I feel like there must be a better way. Is there a way we can fix the generate command?

Maybe a flag like --dump or something to the generate sub-command?

Yes, I assume we will probably make this behaviour optional via some sort of flag in the final version.

sceptre/cli/dump.py

jfalkenstein · 2023-04-18T01:49:31Z

sceptre/cli/dump.py

+        if template is None:
+            logger.warning(f"{stack.external_name} does not exist")
+        else:
+            output.append({stack.external_name: template})


Why a list of dicts and not just one big dict?

Again, for consistency with dump config. I can't remember to be honest why we did it this way.

That feels a bit odd to me

I think it may be to work around or hack the limitations of the write helper. If we need to change dump config, perhaps this should be looked at in a separate issue/PR?

My only concern is this: Let's say I need to pipe a template over to a linter or awscli for some reason because it has jinja and other stuff in it. I want to do something like: sceptre dump template my/sceptre/config.yaml | cfn-lint -. When you're outputting the templates inside a larger data structure like this, that's not a valid template.

I understand your concern of wanting to be able to associate the template with a stack, so there are a few options I can think of:

Option 1: Rather than this, you could output the stack name via a logger rather than echoing it. That will result it going to stderr rather than stdout. That way, if you pipe output to another command or use >, it won't actually get included. So you could alternate by logging the stack name and then writing the output.

Option 2: Have a --stack-names flag that optionally makes it output this way but by default it does not.

@jfalkenstein you are misunderstanding the implementation and behaviour. Here's a demo, showing a yaml stream that can be parsed by external tools is always emitted, even in the case of multiple stacks:

sceptre/plan/actions.py

venv.sh

jfalkenstein

I think I'm just very concerned about adding a completely new command that is ALMOST identical to the old command except it outputs in a slightly different format. I really don't think this is the right way to proceed.

alex-harvey-z3q · 2023-04-28T03:58:21Z

I know you won't like this, but generate has integration tests and I believe that we should have integration tests on these dump commands as well.

I've refactored the generate test to instead call dump_template. As for the other dump commands, they are on a different branch so will have to go in different PRs and this one needs to be merged first.

alex-harvey-z3q · 2023-05-02T09:23:20Z

@jfalkenstein have a look now. Here's what I changed:

implemented --to-file making the default behaviour not to create the dump files.
refactored the process files logic you didn't like moving it all back the dump commands.

changes done

jfalkenstein · 2023-05-02T14:28:11Z

sceptre/cli/dump.py

@@ -18,9 +22,12 @@ def dump_group():

 @dump_group.command(name="config")
 @click.argument("path")
+@click.option(
+    "--to-file", is_flag=True, help="If True, also dump the template to a local file."


Very minor thing, but this says "also", but it appears as if dumping is now either to file or to console, but not both.

Never mind. I see what we're doing now.

jfalkenstein · 2023-05-02T14:30:05Z

sceptre/cli/helpers.py

@@ -93,6 +102,13 @@ def write(var, output_format="json", no_colour=True):

    click.echo(output)


I find it odd that we still echo if a file_path is specified.

Ok how about now? (I'm not sure whether people want the text to go to both the screen and file if file_path is specified).

jfalkenstein · 2023-05-02T14:32:11Z

Running integration tests now.

zaro0508

generate is found in a few locations in the docs. if we are deprecating that command in this same PR then could we also update to dump in the docs as well?

I think it might be cleaner to deprecate the generate command in a follow on PR.

zaro0508 · 2023-05-05T18:14:26Z

sceptre/cli/dump.py

+
+@dump_group.command(name="all")
+@click.argument("path")
+@click.option(


This is missing the --to-file option. is this intentional?

not intentional, fixed.

alex-harvey-z3q · 2023-05-07T12:12:33Z

generate is found in a few locations in the docs. if we are deprecating that command in this same PR then could we also update to dump in the docs as well?

I think it might be cleaner to deprecate the generate command in a follow on PR.

A few hits on the word when I grep -rw generate, however those are not hits on the subcommand. Are you sure?

zaro0508 · 2023-05-08T15:32:15Z

A few hits on the word when I grep -rw generate, however those are not hits on the subcommand. Are you sure?

This is what i see

docs/_source/docs/hooks.rst:``before_generate`` or ``after_generate`` - run hook before or after generating stack template.
docs/_source/docs/resolvers.rst:  ``!stack_output other_stack.yaml::OutputName`` and you run the ``dump template`` command, the generated
docs/_source/docs/templates.rst:to generate CloudFormation Template as a `json` string.
docs/_source/docs/templates.rst:  To generate templates using Troposphere you must install the
docs/_source/docs/templates.rst:AWS CDK can be used to programmatically generate templates for your stacks and then Sceptre can

I think those references should be changed to the dump command

Also didn't we decide to alias the generate to the dump command? if so, it looks like it wasn't done in template.py

sceptre/cli/template.py:@click.command(name="generate", short_help="Prints the template.")
sceptre/cli/template.py:def generate_command(ctx, no_placeholders, path):

alex-harvey-z3q · 2023-05-09T03:06:38Z

This is what i see

docs/_source/docs/hooks.rst:``before_generate`` or ``after_generate`` - run hook before or after generating stack template.
docs/_source/docs/resolvers.rst:  ``!stack_output other_stack.yaml::OutputName`` and you run the ``dump template`` command, the generated
docs/_source/docs/templates.rst:to generate CloudFormation Template as a `json` string.
docs/_source/docs/templates.rst:  To generate templates using Troposphere you must install the
docs/_source/docs/templates.rst:AWS CDK can be used to programmatically generate templates for your stacks and then Sceptre can

I think those references should be changed to the dump command

@zaro0508 I didn't actually know about the before_generate and after_generate hooks but I don't see how those can be renamed. before_dump_template is both an ugly name and does not make sense in that context. It would have to stay as before_generate I think. Remember, it doesn't mean "before you run the generate command" ; it means "before Sceptre generates its template". Generating the template internally, and dumping it out for the caller to look at, are different things.

alex-harvey-z3q · 2023-05-09T08:49:49Z

Also didn't we decide to alias the generate to the dump command? if so, it looks like it wasn't done in template.py
sceptre/cli/template.py:@click.command(name="generate", short_help="Prints the template.")
sceptre/cli/template.py:def generate_command(ctx, no_placeholders, path):

Yes but it was aliased not in the CLI but in the API which is I think the way it should be.

zaro0508 · 2023-05-09T18:35:10Z

Yes but it was aliased not in the CLI but in the API which is I think the way it should be.

What is your reasoning for not aliasing it in the CLI as well?

alex-harvey-z3q · 2023-05-09T23:57:31Z

Yes but it was aliased not in the CLI but in the API which is I think the way it should be.

What is your reasoning for not aliasing it in the CLI as well?

Well, aliasing in both places will just result in unused code paths. If I point the generate CLI command at the dump_template API, why not just remove the generate API since it is no longer used?

In the end I don't really mind how it is done or necessarily see the point in aliasing in the first place. The implementation above does have the advantage that it seems to be the one that @jfalkenstein agreed to and not to mention that it's the one that I already implemented!

alex-harvey-z3q added 11 commits April 10, 2023 18:48

Write rendered template to a temp file

4d1e5cc

In the event that the reader class is unable to parse the rendered config, this adds logic to write the rendered config to a file to assist the caller in understand what went wrong.

[Resolves Sceptre#1318] Part 1

c9ebb5a

This adds a sceptre dump template command and deprecates sceptre generate.

more

97d680f

Merge branch 'alexharvey/add-some-debugging' into merge-branch

8ad7b63

version

3005b14

Revert "version"

acb399b

This reverts commit 3005b14.

Revert deprecation

a3bf4b7

Revert "Revert "version""

c1ca8ed

This reverts commit acb399b.

more

bfa4675

Revert "Write rendered template to a temp file"

fc2be81

This reverts commit 4d1e5cc.

Revert "Revert "Revert "version"""

8905933

This reverts commit c1ca8ed.

alex-harvey-z3q changed the title ~~Alexharvey/1318 part 1~~ [Resolves #1318] Part 1 Apr 15, 2023

alex-harvey-z3q changed the title ~~[Resolves #1318] Part 1~~ [Resolve #1318] Part 1 Apr 15, 2023

alex-harvey-z3q added 5 commits April 15, 2023 18:00

more

7c1d2a1

more

901c5cf

Merge branch 'alexharvey/1318-part-1' into alexharvey/1318-part-2

8d1bbdd

more

f9533bd

Merge branch 'alexharvey/1318-part-1' into alexharvey/1318-part-2

7919be0

jfalkenstein reviewed Apr 15, 2023

View reviewed changes

sceptre/cli/dump.py Outdated Show resolved Hide resolved

zaro0508 requested changes Apr 17, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

jfalkenstein reviewed Apr 18, 2023

View reviewed changes

sceptre/cli/dump.py Outdated Show resolved Hide resolved

jfalkenstein reviewed Apr 18, 2023

View reviewed changes

sceptre/plan/actions.py Show resolved Hide resolved

jfalkenstein reviewed Apr 18, 2023

View reviewed changes

venv.sh Outdated Show resolved Hide resolved

jfalkenstein reviewed Apr 18, 2023

View reviewed changes

Restore CONTRIBUTING.md

efb2090

dboitnot approved these changes Apr 27, 2023

View reviewed changes

more

119162d

Merge branch 'master' into alexharvey/1318-part-1

c1b3068

alex-harvey-z3q requested a review from jfalkenstein May 2, 2023 07:50

alex-harvey-z3q added 2 commits May 2, 2023 17:52

Correction to deprecation

94dcaf1

Add to-file

3e9682e

jfalkenstein reviewed May 2, 2023

View reviewed changes

alex-harvey-z3q and others added 2 commits May 4, 2023 08:58

Correction in steps

e5321c0

Merge branch 'master' into alexharvey/1318-part-1

64004ff

zaro0508 requested changes May 5, 2023

View reviewed changes

alex-harvey-z3q requested review from zaro0508 and jfalkenstein May 7, 2023 12:15

Respone to reviews

d08f091

zaro0508 approved these changes May 10, 2023

View reviewed changes

alex-harvey-z3q added 3 commits May 13, 2023 21:09

Fix integration tests

8a771e6

More renaming for dump name

08862a8

Linter

f51e267

zaro0508 merged commit dcae1a9 into Sceptre:master May 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Resolve #1318] Implement dump template and write output files #1325

[Resolve #1318] Implement dump template and write output files #1325

alex-harvey-z3q commented Apr 15, 2023 •

edited

Loading

jfalkenstein commented Apr 15, 2023

dboitnot commented Apr 17, 2023

alex-harvey-z3q commented Apr 17, 2023 •

edited

Loading

alex-harvey-z3q commented Apr 18, 2023

jfalkenstein Apr 18, 2023

alex-harvey-z3q Apr 18, 2023 •

edited

Loading

jfalkenstein Apr 20, 2023

alex-harvey-z3q Apr 20, 2023

jfalkenstein Apr 20, 2023

alex-harvey-z3q Apr 21, 2023

jfalkenstein left a comment

alex-harvey-z3q commented Apr 28, 2023

alex-harvey-z3q commented May 2, 2023

jfalkenstein May 2, 2023

jfalkenstein May 2, 2023

jfalkenstein May 2, 2023

alex-harvey-z3q May 7, 2023

jfalkenstein commented May 2, 2023

zaro0508 left a comment

zaro0508 May 5, 2023

alex-harvey-z3q May 7, 2023

alex-harvey-z3q commented May 7, 2023

zaro0508 commented May 8, 2023 •

edited

Loading

alex-harvey-z3q commented May 9, 2023

alex-harvey-z3q commented May 9, 2023

zaro0508 commented May 9, 2023

alex-harvey-z3q commented May 9, 2023 •

edited

Loading

		@@ -93,6 +102,13 @@ def write(var, output_format="json", no_colour=True):

		click.echo(output)

[Resolve #1318] Implement dump template and write output files #1325

[Resolve #1318] Implement dump template and write output files #1325

Conversation

alex-harvey-z3q commented Apr 15, 2023 • edited Loading

Description

Usage example

Dump config

Dump template

File structure created

File content

PR Checklist

Approver/Reviewer Checklist

Other Information

jfalkenstein commented Apr 15, 2023

dboitnot commented Apr 17, 2023

alex-harvey-z3q commented Apr 17, 2023 • edited Loading

alex-harvey-z3q commented Apr 18, 2023

Choose a reason for hiding this comment

alex-harvey-z3q Apr 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfalkenstein left a comment

Choose a reason for hiding this comment

alex-harvey-z3q commented Apr 28, 2023

alex-harvey-z3q commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfalkenstein commented May 2, 2023

zaro0508 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-harvey-z3q commented May 7, 2023

zaro0508 commented May 8, 2023 • edited Loading

alex-harvey-z3q commented May 9, 2023

alex-harvey-z3q commented May 9, 2023

zaro0508 commented May 9, 2023

alex-harvey-z3q commented May 9, 2023 • edited Loading

alex-harvey-z3q commented Apr 15, 2023 •

edited

Loading

alex-harvey-z3q commented Apr 17, 2023 •

edited

Loading

alex-harvey-z3q Apr 18, 2023 •

edited

Loading

zaro0508 commented May 8, 2023 •

edited

Loading

alex-harvey-z3q commented May 9, 2023 •

edited

Loading