Repository: code-ready/crc
Pull request #1958 (2021-02-05 11:38:30)

Use regex to replace the carriage return to '\n' char
With this patch, carriage return for windows (`\r\n`) and unix ('\n')
replaced with `\n` char. Till now it was only replacing it for unix.




openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1958#" title="Author self-approved">praveenkumar</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [praveenkumar]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1958#pullrequestreview-584262369" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/1958#" title="Author self-approved">praveenkumar</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [cfergeau,praveenkumar]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci-robot: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
index af3e9dc07..e2e3a812a 100644
--- a/cmd/crc/cmd/start.go
+++ b/cmd/crc/cmd/start.go
@@ -129,6 +129,10 @@ type startResult struct {
 
 func (s *startResult) prettyPrintTo(writer io.Writer) error {
 	if s.Error != nil {
+		var e *crcErrors.PreflightError
+		if errors.As(s.Error, &e) {
+			logging.Warn("Preflight checks failed during `crc start`, please try to run `crc setup` first in case you haven't done so yet")
+		}
 		return s.Error
 	}
 	if s.ClusterConfig == nil {
diff --git a/pkg/crc/cluster/cluster.go b/pkg/crc/cluster/cluster.go
index 8937cdfd9..eee417580 100644
--- a/pkg/crc/cluster/cluster.go
+++ b/pkg/crc/cluster/cluster.go
@@ -4,6 +4,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -214,8 +215,9 @@ func addProxyCACertToCluster(sshRunner *ssh.Runner, ocConfig oc.Config, proxy *n
   }
 }
 `
-	// Replace the carriage return with `\n` char
-	p := fmt.Sprintf(proxyCABundleTemplate, strings.ReplaceAll(proxy.ProxyCACert, "\n", `\n`), trustedCAName)
+	// Replace the carriage return ("\n" or "\r\n") with literal `\n` string
+	re := regexp.MustCompile(`\r?\n`)
+	p := fmt.Sprintf(proxyCABundleTemplate, re.ReplaceAllString(proxy.ProxyCACert, `\n`), trustedCAName)
 	err := sshRunner.CopyData([]byte(p), proxyConfigMapFileName, 0644)
 	if err != nil {
 		return err
diff --git a/pkg/crc/config/viper_config.go b/pkg/crc/config/viper_config.go
index ba52376a4..12b0e9fda 100644
--- a/pkg/crc/config/viper_config.go
+++ b/pkg/crc/config/viper_config.go
@@ -1,7 +1,6 @@
 package config
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -45,21 +44,43 @@ func (c *ViperStorage) Get(key string) interface{} {
 }
 
 func (c *ViperStorage) Set(key string, value interface{}) error {
-	c.viper.Set(key, value)
-	return atomicWrite(c.viper, c.configFile)
+	in, err := ioutil.ReadFile(c.configFile)
+	if err != nil {
+		return err
+	}
+	var cfg map[string]interface{}
+	if err := json.Unmarshal(in, &cfg); err != nil {
+		return err
+	}
+	cfg[key] = value
+	bin, err := json.MarshalIndent(cfg, "", "  ")
+	if err != nil {
+		return err
+	}
+	if err := atomicWrite(bin, c.configFile); err != nil {
+		return err
+	}
+	return c.viper.ReadInConfig()
 }
 
 func (c *ViperStorage) Unset(key string) error {
-	settings := c.viper.AllSettings()
-	delete(settings, key)
-	bin, err := json.Marshal(settings)
+	in, err := ioutil.ReadFile(c.configFile)
+	if err != nil {
+		return err
+	}
+	var cfg map[string]interface{}
+	if err := json.Unmarshal(in, &cfg); err != nil {
+		return err
+	}
+	delete(cfg, key)
+	bin, err := json.MarshalIndent(cfg, "", "  ")
 	if err != nil {
 		return err
 	}
-	if err = c.viper.ReadConfig(bytes.NewReader(bin)); err != nil {
+	if err := atomicWrite(bin, c.configFile); err != nil {
 		return err
 	}
-	return atomicWrite(c.viper, c.configFile)
+	return c.viper.ReadInConfig()
 }
 
 // BindFlagset binds a flagset to their respective config properties
@@ -76,7 +97,7 @@ func ensureConfigFileExists(file string) error {
 	return err
 }
 
-func atomicWrite(viper *viper.Viper, configFile string) error {
+func atomicWrite(bin []byte, configFile string) error {
 	ext := filepath.Ext(configFile)
 	pattern := fmt.Sprintf("%s*%s", strings.TrimSuffix(filepath.Base(configFile), ext), ext)
 	tmpFile, err := ioutil.TempFile(filepath.Dir(configFile), pattern)
@@ -89,7 +110,7 @@ func atomicWrite(viper *viper.Viper, configFile string) error {
 	if err := tmpFile.Close(); err != nil {
 		return err
 	}
-	if err := viper.WriteConfigAs(tmpFile.Name()); err != nil {
+	if err := ioutil.WriteFile(tmpFile.Name(), bin, 0600); err != nil {
 		return err
 	}
 	return os.Rename(tmpFile.Name(), configFile)
diff --git a/pkg/crc/errors/multierror.go b/pkg/crc/errors/multierror.go
index 6d811b481..ea917c9f5 100644
--- a/pkg/crc/errors/multierror.go
+++ b/pkg/crc/errors/multierror.go
@@ -16,6 +16,18 @@ func (v vmNotExist) Error() string {
 
 const VMNotExist vmNotExist = "Machine does not exist. Use 'crc start' to create it"
 
+type PreflightError struct {
+	Err error
+}
+
+func (p *PreflightError) Error() string {
+	return p.Err.Error()
+}
+
+func (p *PreflightError) Unwrap() error {
+	return p.Err
+}
+
 type MultiError struct {
 	Errors []error
 }
diff --git a/pkg/crc/preflight/preflight.go b/pkg/crc/preflight/preflight.go
index ada6d0e19..00a4a943d 100644
--- a/pkg/crc/preflight/preflight.go
+++ b/pkg/crc/preflight/preflight.go
@@ -153,7 +153,10 @@ func doRegisterSettings(cfg config.Schema, checks []Check) {
 func StartPreflightChecks(config config.Storage) error {
 	experimentalFeatures := config.Get(cmdConfig.ExperimentalFeatures).AsBool()
 	mode := network.ParseMode(config.Get(cmdConfig.NetworkMode).AsString())
-	return doPreflightChecks(config, getPreflightChecks(experimentalFeatures, mode))
+	if err := doPreflightChecks(config, getPreflightChecks(experimentalFeatures, mode)); err != nil {
+		return &errors.PreflightError{Err: err}
+	}
+	return nil
 }
 
 // SetupHost performs the prerequisite checks and setups the host to run the cluster


Test patch:
diff --git a/pkg/crc/config/viper_config_test.go b/pkg/crc/config/viper_config_test.go
index 11f10c0cb..88819e8d7 100644
--- a/pkg/crc/config/viper_config_test.go
+++ b/pkg/crc/config/viper_config_test.go
@@ -157,6 +157,7 @@ func TestViperConfigBindFlagSet(t *testing.T) {
 	flagSet := pflag.NewFlagSet("start", pflag.ExitOnError)
 	flagSet.IntP(CPUs, "c", 4, "")
 	flagSet.StringP(NameServer, "n", "", "")
+	flagSet.StringP("extra", "e", "", "")
 
 	_ = storage.BindFlagSet(flagSet)
 
@@ -179,10 +180,9 @@ func TestViperConfigBindFlagSet(t *testing.T) {
 	_, err = config.Set(CPUs, "6")
 	assert.NoError(t, err)
 
-	assert.Equal(t, SettingValue{
-		Value:     6,
-		IsDefault: false,
-	}, config.Get(CPUs))
+	bin, err := ioutil.ReadFile(configFile)
+	assert.NoError(t, err)
+	assert.JSONEq(t, `{"cpus":6}`, string(bin))
 }
 
 func TestViperConfigCastSet(t *testing.T) {