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

Add ShowPlanFileRaw #83

Merged
merged 2 commits into from
Sep 15, 2020
Merged

Add ShowPlanFileRaw #83

merged 2 commits into from
Sep 15, 2020

Conversation

paddycarver
Copy link
Contributor

The SDK used a non-JSON version of ShowPlanFile to return the
human-friendly, opaque-to-machines plan output during testing. With the
switch to tfexec, this is no longer possible, as all terraform show
commands have the -json flag applied, meaning all output will be JSON
output.

This adds a new function, ShowRawPlanFile, which returns a string of
the output of terraform show without the -json flag, showing the
raw contents of the plan file. A new function was chosen to avoid
overloading the existing ShowPlanFile, and because the existing
ShowPlanFile returns a tfjson.Plan type, which we can't shoehorn a
non-JSON return type into easily. Rather than complicate the return
type, it felt like a new function was a better solution.

The SDK used a non-JSON version of ShowPlanFile to return the
human-friendly, opaque-to-machines plan output during testing. With the
switch to tfexec, this is no longer possible, as all `terraform show`
commands have the `-json` flag applied, meaning all output will be JSON
output.

This adds a new function, `ShowRawPlanFile`, which returns a string of
the output of `terraform show` _without_ the `-json` flag, showing the
raw contents of the plan file. A new function was chosen to avoid
overloading the existing ShowPlanFile, and because the existing
ShowPlanFile returns a `tfjson.Plan` type, which we can't shoehorn a
non-JSON return type into easily. Rather than complicate the return
type, it felt like a new function was a better solution.
@paddycarver paddycarver added the enhancement New feature or request label Sep 12, 2020
@paddycarver paddycarver requested a review from a team September 12, 2020 11:08
@paultyng suggested `ShowPlanFileRaw`, so we'll run with that, instead,
because I'm ambivalent.
@paddycarver
Copy link
Contributor Author

Updated to ShowPlanFileRaw, at @paultyng's suggestion, because he has opinions and I do not. :D

showCmd := tf.showCmd(ctx, false, mergeEnv, planPath)

var ret bytes.Buffer
showCmd.Stdout = &ret
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to use mergewriters just in case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mergeWriters is already called by tf.runTerraformCmd, though? In fact, that's the only command that calls it.

Copy link
Member

@kmoe kmoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Note name of function ShowPlanFileRaw (looks like it was changed at some point) if you rebase and merge, as it uses the PR title.

@paddycarver paddycarver changed the title Add ShowRawPlanFile Add ShowPlanFileRaw Sep 15, 2020
@paddycarver paddycarver merged commit a88ac03 into master Sep 15, 2020
@paddycarver paddycarver deleted the paddy_raw_plan_output branch September 15, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants