feat: introduce --output option for "fix" cmd

- introduced --output/-o (mut.exclusive with --overwrite)
- add more examples
- add more integration tests

Fixes GoogleContainerTools#6456.

Signed-off-by: Ahmet Alp Balkan <[email protected]>

Author: ahmetb

cmd/skaffold/app/cmd/fix.go

@@ -32,26 +32,41 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/yaml"
 )
 
-var toVersion string
+var (
+	toVersion     string
+	fixOutputPath string
+)
 
 func NewCmdFix() *cobra.Command {
 	return NewCmd("fix").
 		WithDescription("Update old configuration to a newer schema version").
 		WithExample("Update \"skaffold.yaml\" in the current folder to the latest version", "fix").
 		WithExample("Update \"skaffold.yaml\" in the current folder to version \"skaffold/v1\"", "fix --version skaffold/v1").
+		WithExample("Update \"skaffold.yaml\" in the current folder in-place", "fix --overwrite").
+		WithExample("Update \"skaffold.yaml\" and write the output to a new file", "fix --output skaffold.new.yaml").
 		WithCommonFlags().
 		WithFlags([]*Flag{
 			{Value: &overwrite, Name: "overwrite", DefValue: false, Usage: "Overwrite original config with fixed config"},
 			{Value: &toVersion, Name: "version", DefValue: latestV1.Version, Usage: "Target schema version to upgrade to"},
+			{Value: &fixOutputPath, Name: "output", Shorthand: "o", DefValue: "", Usage: "File to write the changed config (instead of standard output)"},
 		}).
 		NoArgs(doFix)
 }
 
 func doFix(_ context.Context, out io.Writer) error {
-	return fix(out, opts.ConfigurationFile, toVersion, overwrite)
+	if overwrite && fixOutputPath != "" {
+		return fmt.Errorf("--overwrite and --output/-o cannot be used together")
+	}
+	var toFile string
+	if fixOutputPath != "" {
+		toFile = fixOutputPath
+	} else if overwrite {
+		toFile = opts.ConfigurationFile
+	}
+	return fix(out, opts.ConfigurationFile, toFile, toVersion)
 }
 
-func fix(out io.Writer, configFile string, toVersion string, overwrite bool) error {
+func fix(out io.Writer, configFile, outFile string, toVersion string) error {
 	parsedCfgs, err := schema.ParseConfig(configFile)
 	if err != nil {
 		return err
@@ -97,11 +112,11 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err
 	if err != nil {
 		return fmt.Errorf("marshaling new config: %w", err)
 	}
-	if overwrite {
-		if err := ioutil.WriteFile(configFile, newCfg, 0644); err != nil {
+	if outFile != "" {
+		if err := ioutil.WriteFile(outFile, newCfg, 0644); err != nil {
 			return fmt.Errorf("writing config file: %w", err)
 		}
-		output.Default.Fprintf(out, "New config at version %s generated and written to %s\n", toVersion, opts.ConfigurationFile)
+		output.Default.Fprintf(out, "New config at version %s generated and written to %s\n", toVersion, outFile)
 	} else {
 		out.Write(newCfg)
 	}

cmd/skaffold/app/cmd/fix_test.go

@@ -158,14 +158,14 @@ build:
 			cfgFile := t.TempFile("config", []byte(test.inputYaml))
 
 			var b bytes.Buffer
-			err := fix(&b, cfgFile, test.targetVersion, false)
+			err := fix(&b, cfgFile, "", test.targetVersion)
 
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.output, b.String())
 		})
 	}
 }
 
-func TestFixOverwrite(t *testing.T) {
+func TestFixToFileOverwrite(t *testing.T) {
 	inputYaml := `apiVersion: skaffold/v1alpha4
 kind: Config
 build:
@@ -203,7 +203,7 @@ deploy:
 		cfgFile := t.TempFile("config", []byte(inputYaml))
 
 		var b bytes.Buffer
-		err := fix(&b, cfgFile, latestV1.Version, true)
+		err := fix(&b, cfgFile, cfgFile, latestV1.Version)
 
 		output, _ := ioutil.ReadFile(cfgFile)
 

docs/content/en/docs/references/cli/_index.md

@@ -803,9 +803,16 @@ Examples:
   # Update "skaffold.yaml" in the current folder to version "skaffold/v1"
   skaffold fix --version skaffold/v1
 
+  # Update "skaffold.yaml" in the current folder in-place
+  skaffold fix --overwrite
+
+  # Update "skaffold.yaml" and write the output to a new file
+  skaffold fix --output skaffold.new.yaml
+
 Options:
   -f, --filename='skaffold.yaml': Path or URL to the Skaffold config file
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
+  -o, --output='': File to write the changed config (instead of standard output)
       --overwrite=false: Overwrite original config with fixed config
       --remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
       --sync-remote-cache='missing': Controls how Skaffold manages the remote config cache (see `remote-cache-dir`). One of `always` (default), `missing`, or `never`. `always` syncs remote repositories to latest on access. `missing` only clones remote repositories if they do not exist locally. `never` means the user takes responsibility for updating remote repositories.
@@ -822,6 +829,7 @@ Env vars:
 
 * `SKAFFOLD_FILENAME` (same as `--filename`)
 * `SKAFFOLD_MODULE` (same as `--module`)
+* `SKAFFOLD_OUTPUT` (same as `--output`)
 * `SKAFFOLD_OVERWRITE` (same as `--overwrite`)
 * `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
 * `SKAFFOLD_SYNC_REMOTE_CACHE` (same as `--sync-remote-cache`)

integration/fix_test.go

@@ -17,17 +17,45 @@ limitations under the License.
 package integration
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/integration/skaffold"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+	yaml "gopkg.in/yaml.v2"
 )
 
-func TestFix(t *testing.T) {
+func TestFixExclusiveOptions(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
 
+	out := skaffold.Fix().InDir("testdata/fix").RunOrFailOutput(t)
+	out, err := skaffold.Fix("--overwrite", "--output=ignored").WithConfig("-").InDir("testdata/fix").
+		WithStdin(out).RunWithCombinedOutput(t)
+	testutil.CheckError(t, true, err)
+	testutil.CheckContains(t, "cannot be used together", string(out))
+}
+
+func TestFixStdout(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
 	ns, _ := SetupNamespace(t)
 
 	out := skaffold.Fix().InDir("testdata/fix").RunOrFailOutput(t)
-
 	skaffold.Run().WithConfig("-").InDir("testdata/fix").InNs(ns.Name).WithStdin(out).RunOrFail(t)
 }
+
+func TestFixOutputFile(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
+
+	out := skaffold.Fix("--output", filepath.Join("updated.yaml")).InDir("testdata/fix").RunOrFailOutput(t)
+	testutil.CheckContains(t, "written to updated.yaml", string(out))
+	defer os.Remove(filepath.Join("testdata", "fix", "updated.yaml"))
+
+	f, err := ioutil.ReadFile(filepath.Join("testdata", "fix", "updated.yaml"))
+	testutil.CheckError(t, false, err)
+
+	parsed := make(map[string]interface{})
+	err = yaml.UnmarshalStrict(f, parsed)
+	testutil.CheckError(t, false, err)
+}