fix(preview): broken tls in jx preview

fixes #2196

- the `omitempty` json tag was missing from the ExposeControllerConfig struct, which resulted in empty values being present in the extraValues.yaml file - thus overriding valid values from the values.yaml file
- the cmdline flags passed through `jx preview` for the ExposeControllerConfig struct were not used to generate the extraValues.yaml file
- ths `tls-acme` flag's default value was `"false"`, which was not an empty value, and thus was overriding the value from the values.yaml file. If we use the empty string as default value, it will still default to false in the exposecontroller, while allowing the values.yaml file from the preview chart to enable TLS

also extracting the preview values config in a func, to unit test it

pkg/config/helm_values.go

@@ -9,10 +9,10 @@ import (
 
 type ExposeControllerConfig struct {
 	Domain   string `yaml:"domain,omitempty"`
-	Exposer  string `yaml:"exposer"`
-	HTTP     string `yaml:"http"`
-	TLSAcme  string `yaml:"tlsacme"`
-	PathMode string `yaml:"pathMode"`
+	Exposer  string `yaml:"exposer,omitempty"`
+	HTTP     string `yaml:"http,omitempty"`
+	TLSAcme  string `yaml:"tlsacme,omitempty"`
+	PathMode string `yaml:"pathMode,omitempty"`
 }
 type ExposeController struct {
 	Config      ExposeControllerConfig `yaml:"config,omitempty"`
@@ -87,7 +87,7 @@ func (c *HelmValuesConfig) AddExposeControllerValues(cmd *cobra.Command, ignoreD
 
 	cmd.Flags().StringVarP(&c.ExposeController.Config.HTTP, "http", "", "true", "Toggle creating http or https ingress rules")
 	cmd.Flags().StringVarP(&c.ExposeController.Config.Exposer, "exposer", "", "Ingress", "Used to describe which strategy exposecontroller should use to access applications")
-	cmd.Flags().StringVarP(&c.ExposeController.Config.TLSAcme, "tls-acme", "", "false", "Used to enable automatic TLS for ingress")
+	cmd.Flags().StringVarP(&c.ExposeController.Config.TLSAcme, "tls-acme", "", "", "Used to enable automatic TLS for ingress")
 	cmd.Flags().BoolVarP(&keepJob, "keep-exposecontroller-job", "", false, "Prevents Helm deleting the exposecontroller Job and Pod after running.  Useful for debugging exposecontroller logs but you will need to manually delete the job if you update an environment")
 
 	annotations := make(map[string]string)

pkg/jx/cmd/preview.go

@@ -434,30 +434,11 @@ func (o *PreviewOptions) Run() error {
 		return err
 	}
 
-	repository, err := getImageName()
-	if err != nil {
-		return err
-	}
-
-	tag, err := getImageTag()
+	values, err := o.GetPreviewValuesConfig(domain)
 	if err != nil {
 		return err
 	}
 
-	values := config.PreviewValuesConfig{
-		ExposeController: &config.ExposeController{
-			Config: config.ExposeControllerConfig{
-				Domain: domain,
-			},
-		},
-		Preview: &config.Preview{
-			Image: &config.Image{
-				Repository: repository,
-				Tag:        tag,
-			},
-		},
-	}
-
 	config, err := values.String()
 	if err != nil {
 		return err
@@ -798,6 +779,35 @@ func (o *PreviewOptions) defaultValues(ns string, warnMissingName bool) error {
 	return nil
 }
 
+// GetPreviewValuesConfig returns the PreviewValuesConfig to use as extraValues for helm
+func (o *PreviewOptions) GetPreviewValuesConfig(domain string) (*config.PreviewValuesConfig, error) {
+	repository, err := getImageName()
+	if err != nil {
+		return nil, err
+	}
+
+	tag, err := getImageTag()
+	if err != nil {
+		return nil, err
+	}
+
+	if o.HelmValuesConfig.ExposeController == nil {
+		o.HelmValuesConfig.ExposeController = &config.ExposeController{}
+	}
+	o.HelmValuesConfig.ExposeController.Config.Domain = domain
+
+	values := config.PreviewValuesConfig{
+		ExposeController: o.HelmValuesConfig.ExposeController,
+		Preview: &config.Preview{
+			Image: &config.Image{
+				Repository: repository,
+				Tag:        tag,
+			},
+		},
+	}
+	return &values, nil
+}
+
 func writePreviewURL(o *PreviewOptions, url string) {
 	previewFileName := filepath.Join(o.Dir, ".previewUrl")
 	err := ioutil.WriteFile(previewFileName, []byte(url), 0644)

pkg/jx/cmd/preview_test.go

@@ -0,0 +1,89 @@
+package cmd_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/jenkins-x/jx/pkg/config"
+	"github.com/jenkins-x/jx/pkg/jx/cmd"
+)
+
+func TestGetPreviewValuesConfig(t *testing.T) {
+	tests := []struct {
+		opts               cmd.PreviewOptions
+		env                map[string]string
+		domain             string
+		expectedYAMLConfig string
+	}{
+		{
+			opts: cmd.PreviewOptions{
+				HelmValuesConfig: config.HelmValuesConfig{
+					ExposeController: &config.ExposeController{},
+				},
+			},
+			env: map[string]string{
+				cmd.DOCKER_REGISTRY: "my.registry",
+				cmd.ORG:             "my-org",
+				cmd.APP_NAME:        "my-app",
+				cmd.PREVIEW_VERSION: "1.0.0",
+			},
+			expectedYAMLConfig: `expose: {}
+preview:
+  image:
+    repository: my.registry/my-org/my-app
+    tag: 1.0.0
+`,
+		},
+		{
+			opts: cmd.PreviewOptions{
+				HelmValuesConfig: config.HelmValuesConfig{
+					ExposeController: &config.ExposeController{
+						Config: config.ExposeControllerConfig{
+							HTTP:    "false",
+							TLSAcme: "true",
+						},
+					},
+				},
+			},
+			env: map[string]string{
+				cmd.DOCKER_REGISTRY: "my.registry",
+				cmd.ORG:             "my-org",
+				cmd.APP_NAME:        "my-app",
+				cmd.PREVIEW_VERSION: "1.0.0",
+			},
+			domain: "jenkinsx.io",
+			expectedYAMLConfig: `expose:
+  config:
+    domain: jenkinsx.io
+    http: "false"
+    tlsacme: "true"
+preview:
+  image:
+    repository: my.registry/my-org/my-app
+    tag: 1.0.0
+`,
+		},
+	}
+
+	for i, test := range tests {
+		for k, v := range test.env {
+			os.Setenv(k, v)
+		}
+
+		config, err := test.opts.GetPreviewValuesConfig(test.domain)
+		if err != nil {
+			t.Errorf("[%d] got unexpected err: %v", i, err)
+			continue
+		}
+
+		configYAML, err := config.String()
+		if err != nil {
+			t.Errorf("[%d] %v", i, err)
+			continue
+		}
+
+		if test.expectedYAMLConfig != configYAML {
+			t.Errorf("[%d] expected %#v but got %#v", i, test.expectedYAMLConfig, configYAML)
+		}
+	}
+}