theonestack / cfhighlander

Cloudformation DSL and component library
MIT License
25 stars 23 forks source link

Feature request: get diff output with new component versions #102

Open rererecursive opened 5 years ago

rererecursive commented 5 years ago

When changing a component's version, we have no mechanism to verify that the compiled output will also be changed. For small changes to components, it's usually easy for a developer to verify that the new code is safe to deploy, simply by visiting GitHub. But for large changes, verifying can involve more lengthy processes, such as using diff, updates to CloudFormation, etc, which can be inefficient and error prone.

It would be nice to have an option to cfcompile that would allow us to see the diff if we were to use a new component's version in the project. For example, a command like cfhighlander cfcompile --diff vpc1.0.0=vpc1.0.1.

Note that this is separate from showing the actual diff of the components themselves -- this covers only the generated output.

This could also serve as a testing tool, as we can verify that a config can work across multiple versions of a particular component, and vice versa.

Guslington commented 5 years ago

@rererecursive good idea, just thinking it might be better to have diff as a seperate command outside on cfcompile. But i agree it should be diffing the generated cloudformation output, either json or yaml.

Something like cfhighlander diff --compare vpc@1.0.1 ecs@1.7.0 bastion@1.2.0 and would diff each generated component and display the output under a heading of each template.

Any thoughts @toshke @aaronwalker ?

toshke commented 5 years ago

@Guslington @rererecursive my two cents: this behaviour can be implemented outside of highlander, either by just invoking diff on the highlander output of two different configurations. I feel like adding this functionality may make the cli bit bloated and steeping bit aside from unix philosophy of single tool for single job. Perhaps having standardised Makefile to work with components to wrap such functionality would me more suitable - @aaronwalker thoughts?

aaronwalker commented 5 years ago

@toshke kinda agree... Although I had this exact situation though and my workflow was to compile my stack with the old version copy the complied yaml then upgrade the component and compile again then run a diff against the old version. Was pretty clunky.

I think if we go down the path of having a version lock file we could include some sort of diff functionality as part of that process as it would make it easier to determine what to actually diff

Guslington commented 5 years ago

@toshke i get where you're coming from but i agree with @aaronwalker that if we're versioning components there should be some provided functionality to check for changes. Maybe it's feature for kurgan, something like an update command which would display the diffed yaml/json

kurgan update vpc --version 1.0.1 --diff
toshke commented 5 years ago

@Guslington - i like idea of such functionality being part of kurgan . I've also had few more thing in mind for kurgan, e.g. generating default config, starting a new component that extends another one

Guslington commented 5 years ago

@toshke cool, i'll move this request to kurgan

https://github.com/theonestack/kurgan/issues/2

rererecursive commented 5 years ago

Actually, I've realised that the best way to achieve a diff of the output is to simply track the generated out/ folder in git. We make our changes, run cfcompile, and then do a git diff to check the differences.

Which leads me to think: might it be beneficial to actually track the generated JSON in git? After all, it is the information that actually gets uploaded and represents the state of the infrastructure. Is there a reason why we don't commit the output (JSON) to the repository? Is it because we want Highlander/Cfndsl to represent the infrastructure instead? Ideally this would be the case, but ultimately we rely on the JSON being correct, and we need a method to ensure that a change in any Highlander is correctly translated into the expected JSON.