From ae8f9ae2577ade513a8c18692a61e2b5933ff00e Mon Sep 17 00:00:00 2001 From: Mattia Barbon Date: Thu, 20 Sep 2018 11:11:04 +0200 Subject: [PATCH] Display policy changes as diffs Format policy changes using diff/colordiff, for better readability Displaying policy additions as diffs is questionable, but I prefer it for consistency. --- astro/cli/astro/cmd/display.go | 10 +- astro/terraform/policy_diff.go | 211 ++++++++++++++++++++++++++++ astro/terraform/policy_diff_test.go | 146 +++++++++++++++++++ 3 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 astro/terraform/policy_diff.go create mode 100644 astro/terraform/policy_diff_test.go diff --git a/astro/cli/astro/cmd/display.go b/astro/cli/astro/cmd/display.go index f98f814..34555db 100644 --- a/astro/cli/astro/cmd/display.go +++ b/astro/cli/astro/cmd/display.go @@ -96,7 +96,15 @@ func printExecStatus(status <-chan string, results <-chan *astro.Result) (errors // If this was a plan, print the plan if planResult != nil && planResult.HasChanges() { - fmt.Fprintf(out, "\n%s", planResult.Changes()) + planOutput := planResult.Changes() + if terraform.CanDisplayReadableTerraformPolicyChanges() { + var err error + planOutput, err = terraform.ReadableTerraformPolicyChanges(planOutput) + if err != nil { + fmt.Fprintf(out, "\n%s", err) + } + } + fmt.Fprintf(out, "\n%s", planOutput) } // If there is a stderr, print it diff --git a/astro/terraform/policy_diff.go b/astro/terraform/policy_diff.go new file mode 100644 index 0000000..6b5f6f7 --- /dev/null +++ b/astro/terraform/policy_diff.go @@ -0,0 +1,211 @@ +/* + * Copyright (c) 2018 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package terraform + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "os/exec" + "regexp" + "strings" + "syscall" + + multierror "github.com/hashicorp/go-multierror" +) + +var ( + // Full path to differ will be stored here on init + differPath string + // $PATH will be searched for these tools on init + differTools = []string{ + "colordiff", + "diff", + } + newline = []byte("\n") + // regular expressions that matches a policy add/change in a Terraform diff. + terraformPolicyAddLine = regexp.MustCompile(`\s*policy:\s+"(.*)"`) + terraformPolicyChangeLine = regexp.MustCompile(`\s*policy:\s+"(.*)" => "(.*)"`) +) + +func init() { + differPath, _ = which(differTools) +} + +// terraformPolicyChangeToDiff takes a Terraform policy change output line +// (i.e. from a Terraform plan) parses the JSON and outputs a unified diff. +func terraformPolicyChangeToDiff(differ, policyBefore, policyAfter string) ([]byte, error) { + jsonBefore, err := jsonPretty(unescape(policyBefore)) + if err != nil { + return nil, err + } + before, err := writeToTempFile(jsonBefore) + if err != nil { + return nil, err + } + defer os.Remove(before) + + jsonAfter, err := jsonPretty(unescape(policyAfter)) + if err != nil { + return nil, err + } + after, err := writeToTempFile(jsonAfter) + if err != nil { + return nil, err + } + defer os.Remove(after) + + return diff(differ, before, after) +} + +// diff invokes diff to output a diff of two files. +func diff(differ, file1, file2 string) ([]byte, error) { + cmd := exec.Command(differ, "-u", file1, file2) + out, err := cmd.Output() + + // We only want to throw an error here if the exit status was 2 or + // higher. From the diff man page: "Exit status is 0 if inputs are the + // same, 1 if different, 2 if trouble." + if err != nil { + exitErr, ok := err.(*exec.ExitError) + if !ok { + return nil, err + } + + status, ok := exitErr.Sys().(syscall.WaitStatus) + if !ok || status.ExitStatus() > 1 { + return nil, err + } + } + + return out, nil +} + +// jsonPretty takes unformatted JSON and indents it so it is human readable. If +// the JSON cannot be indented, the original JSON is returned. +func jsonPretty(in []byte) ([]byte, error) { + if len(in) == 0 { + return in, nil + } + var out bytes.Buffer + err := json.Indent(&out, in, "", " ") + if err != nil { + return nil, err + } + return out.Bytes(), nil +} + +// CanDisplayReadableTerraformPolicyChanges is true when the prerequisites for +// ReadableTerraformPolicyChanges are fulfilled +func CanDisplayReadableTerraformPolicyChanges() bool { + return differPath != "" +} + +func readableTerraformPolicyChangesWithDiffer(differ, terraformChanges string) (string, error) { + result := "" + var errs error + for _, line := range strings.Split(terraformChanges, "\n") { + // Check if the line matches a Terraform policy diff + changeGroups := terraformPolicyChangeLine.FindStringSubmatch(line) + addGroups := terraformPolicyAddLine.FindStringSubmatch(line) + if changeGroups == nil && addGroups == nil { + // If it doesn't match, just print the line verbatim and move on + result += line + result += "\n" + continue + } + + // Get a readable diff from the policy change + var difftext []byte + var err error + if changeGroups != nil { + difftext, err = terraformPolicyChangeToDiff(differ, changeGroups[1], changeGroups[2]) + } else { + difftext, err = terraformPolicyChangeToDiff(differ, "", addGroups[1]) + } + if err != nil { + errs = multierror.Append(errs, err) + result += line + result += "\n" + continue + } + + // Output a readable diff + result += "\n" + result += string(tail(difftext, 2, true)) + result += "\n" + } + + return result, errs +} + +// ReadableTerraformPolicyChanges takes the output of `terraform plan` and +// rewrites policy diff to be in unified diff format +func ReadableTerraformPolicyChanges(terraformChanges string) (string, error) { + return readableTerraformPolicyChangesWithDiffer(differPath, terraformChanges) +} + +// tail is an implementation of the unix tail command. If fromN is true, it is +// equivalent to `tail -n +K`. See `main tail` for more info. +func tail(input []byte, n int, fromN bool) []byte { + // split lines + sub := bytes.Split(input, newline) + if fromN { + return bytes.Join(sub[n:], newline) + } + return bytes.Join(sub[len(sub)-n:], newline) +} + +// unescape takes an escaped JSON string output by Terraform on the console +// and converts it to valid JSON. +func unescape(in string) []byte { + out := []byte(in) + out = bytes.Replace(out, []byte(`\n`), []byte("\n"), -1) + out = bytes.Replace(out, []byte(`\"`), []byte(`"`), -1) + out = bytes.Replace(out, []byte(`\\`), []byte(`\`), -1) + return out +} + +// writeToTempFile creates a temporary file and writes the specified data to +// it. +func writeToTempFile(data []byte) (filePath string, err error) { + tmpfile, err := ioutil.TempFile("", "") + if err != nil { + return "", err + } + + if len(data) > 0 { + tmpfile.Write(data) + tmpfile.Write(newline) + } + + return tmpfile.Name(), nil +} + +// which searches the $PATH for each of the candidates and returns the full +// path to the first program that exists. +func which(candidates []string) (string, error) { + for _, candidate := range candidates { + path, err := exec.LookPath(candidate) + if err == nil { + return path, nil + } + } + return "", fmt.Errorf("cannot find any of: %v in $PATH", candidates) +} diff --git a/astro/terraform/policy_diff_test.go b/astro/terraform/policy_diff_test.go new file mode 100644 index 0000000..38b59ba --- /dev/null +++ b/astro/terraform/policy_diff_test.go @@ -0,0 +1,146 @@ +/* + * Copyright (c) 2018 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package terraform + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +var ( + // Full path to differ for tests will be stored here on init + testDifferPath string +) + +func init() { + testDifferPath, _ = which([]string{"diff"}) +} + +func TestRewriteOutputChange(t *testing.T) { + if testDifferPath == "" { + t.Skip("skipping test since there is no diff program") + } + + inputText := ` +module.policies.data.aws_iam_policy_document.billing: Refreshing state... + +Your plan was also saved to the path below. Call the "apply" subcommand +with this plan file and Terraform will exactly execute this execution +plan. + +Path: mgmt.plan + +~ module.policies.aws_iam_policy.billing +policy: "{\n \"Version\": \"2012-10-17\",\n \"Statement\": [\n {\n \"Effect\": \"Allow\",\n \"Action\": [\n \"budgets:*\",\n \"aws-portal:View*\"\n ],\n \"Resource\": [\n \"*\"\n ]\n }\n ]\n}" => "{\n \"Version\": \"2012-10-17\",\n \"Statement\": [\n {\n \"Sid\": \"\",\n \"Effect\": \"Allow\",\n \"Action\": [\n \"budgets:*\",\n \"aws-portal:View*\"\n ],\n \"Resource\": \"*\"\n }\n ]\n}" + +Plan: 0 to add, 1 to change, 0 to destroy. +` + expectedOutput := ` +module.policies.data.aws_iam_policy_document.billing: Refreshing state... + +Your plan was also saved to the path below. Call the "apply" subcommand +with this plan file and Terraform will exactly execute this execution +plan. + +Path: mgmt.plan + +~ module.policies.aws_iam_policy.billing + +@@ -2,14 +2,13 @@ + "Version": "2012-10-17", + "Statement": [ + { ++ "Sid": "", + "Effect": "Allow", + "Action": [ + "budgets:*", + "aws-portal:View*" + ], +- "Resource": [ +- "*" +- ] ++ "Resource": "*" + } + ] + } + + +Plan: 0 to add, 1 to change, 0 to destroy. +` + + diffedPolicy, err := readableTerraformPolicyChangesWithDiffer(testDifferPath, inputText) + + assert.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expectedOutput), strings.TrimSpace(diffedPolicy)) +} + +func TestRewriteOutputAdd(t *testing.T) { + if testDifferPath == "" { + t.Skip("skipping test since there is no diff program") + } + + inputText := ` +module.policies.data.aws_iam_policy_document.billing: Refreshing state... + +Your plan was also saved to the path below. Call the "apply" subcommand +with this plan file and Terraform will exactly execute this execution +plan. + +Path: mgmt.plan + +~ module.policies.aws_iam_policy.billing +policy: "{\n \"Version\": \"2012-10-17\",\n \"Statement\": [\n {\n \"Sid\": \"\",\n \"Effect\": \"Allow\",\n \"Action\": [\n \"budgets:*\",\n \"aws-portal:View*\"\n ],\n \"Resource\": \"*\"\n }\n ]\n}" + +Plan: 0 to add, 1 to change, 0 to destroy. +` + expectedOutput := ` +module.policies.data.aws_iam_policy_document.billing: Refreshing state... + +Your plan was also saved to the path below. Call the "apply" subcommand +with this plan file and Terraform will exactly execute this execution +plan. + +Path: mgmt.plan + +~ module.policies.aws_iam_policy.billing + +@@ -0,0 +1,14 @@ ++{ ++ "Version": "2012-10-17", ++ "Statement": [ ++ { ++ "Sid": "", ++ "Effect": "Allow", ++ "Action": [ ++ "budgets:*", ++ "aws-portal:View*" ++ ], ++ "Resource": "*" ++ } ++ ] ++} + + +Plan: 0 to add, 1 to change, 0 to destroy. +` + + diffedPolicy, err := readableTerraformPolicyChangesWithDiffer(testDifferPath, inputText) + + assert.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expectedOutput), strings.TrimSpace(diffedPolicy)) +}