Merge pull request #832 from shomron/issue-726-tls-skip-verify-init

Respect --tls-skip-verify for ks init, ks env * commands

pkg/actions/env_update.go

@@ -17,6 +17,7 @@ package actions
 
 import (
 	"fmt"
+	"net/http"
 
 	"github.com/ksonnet/ksonnet/pkg/app"
 	"github.com/ksonnet/ksonnet/pkg/lib"
@@ -37,7 +38,8 @@ type EnvUpdate struct {
 	app     app.App
 	envName string
 
-	genLibFn func(app.App, string, string) error
+	httpClient *http.Client
+	genLibFn   func(app.App, string, string, *http.Client) error
 }
 
 // RunEnvUpdate runs `env update`
@@ -48,7 +50,8 @@ func newEnvUpdate(m map[string]interface{}) (*EnvUpdate, error) {
 		app:     ol.LoadApp(),
 		envName: ol.LoadString(OptionEnvName),
 
-		genLibFn: genLib,
+		httpClient: ol.LoadHTTPClient(),
+		genLibFn:   genLib,
 	}
 
 	if ol.err != nil {
@@ -71,12 +74,12 @@ func (eu *EnvUpdate) run() error {
 		return err
 	}
 
-	return eu.genLibFn(eu.app, k8sSpecFlag, libPath)
+	return eu.genLibFn(eu.app, k8sSpecFlag, libPath, eu.httpClient)
 
 }
 
-func genLib(a app.App, k8sSpecFlag, libPath string) error {
-	libManager, err := lib.NewManager(k8sSpecFlag, a.Fs(), libPath)
+func genLib(a app.App, k8sSpecFlag, libPath string, httpClient *http.Client) error {
+	libManager, err := lib.NewManager(k8sSpecFlag, a.Fs(), libPath, httpClient)
 	if err != nil {
 		return err
 	}

pkg/actions/env_update_test.go

@@ -16,6 +16,7 @@
 package actions
 
 import (
+	"net/http"
 	"testing"
 
 	"github.com/ksonnet/ksonnet/pkg/app"
@@ -41,7 +42,7 @@ func TestEnvUpdate(t *testing.T) {
 		a, err := newEnvUpdate(in)
 		require.NoError(t, err)
 
-		a.genLibFn = func(_ app.App, k8sSpecFlag, libPath string) error {
+		a.genLibFn = func(_ app.App, k8sSpecFlag, libPath string, httpClient *http.Client) error {
 			assert.Equal(t, "version:v1.8.9", k8sSpecFlag)
 			assert.Equal(t, "/app/lib/v1.8.9", libPath)
 			return nil

pkg/actions/init.go

@@ -16,9 +16,12 @@
 package actions
 
 import (
+	"net/http"
+
 	"github.com/ksonnet/ksonnet/pkg/app"
 	"github.com/ksonnet/ksonnet/pkg/appinit"
 	"github.com/ksonnet/ksonnet/pkg/registry"
+	"github.com/ksonnet/ksonnet/pkg/util/github"
 	"github.com/spf13/afero"
 )
 
@@ -37,11 +40,11 @@ func RunInit(m map[string]interface{}) error {
 	return i.Run()
 }
 
-type appLoadFn func(fs afero.Fs, root string, skipFindRoot bool) (app.App, error)
+type appLoadFn func(fs afero.Fs, httpClient *http.Client, root string, skipFindRoot bool) (app.App, error)
 
-type appInitFn func(fs afero.Fs, name, rootPath, envName, k8sSpecFlag, serverURI, namespace string, registries []registry.Registry) error
+type appInitFn func(fs afero.Fs, httpClient *http.Client, name, rootPath, envName, k8sSpecFlag, serverURI, namespace string, registries []registry.Registry) error
 
-type initIncubatorFn func(app.App) (registry.Registry, error)
+type initIncubatorFn func(app.App, *http.Client) (registry.Registry, error)
 
 // Init creates a component namespace
 type Init struct {
@@ -57,6 +60,8 @@ type Init struct {
 	appInitFn       appInitFn
 	appLoadFn       appLoadFn
 	initIncubatorFn initIncubatorFn
+
+	httpClient *http.Client
 }
 
 // NewInit creates an instance of Init.
@@ -76,6 +81,8 @@ func NewInit(m map[string]interface{}) (*Init, error) {
 		appInitFn:       appinit.Init,
 		appLoadFn:       app.Load,
 		initIncubatorFn: initIncubator,
+
+		httpClient: ol.LoadHTTPClient(),
 	}
 
 	if ol.err != nil {
@@ -90,12 +97,12 @@ func (i *Init) Run() error {
 	var registries []registry.Registry
 
 	if !i.skipDefaultRegistries {
-		a, err := i.appLoadFn(i.fs, i.rootPath, true)
+		a, err := i.appLoadFn(i.fs, i.httpClient, i.rootPath, true)
 		if err != nil {
 			return err
 		}
 
-		gh, err := i.initIncubatorFn(a)
+		gh, err := i.initIncubatorFn(a, i.httpClient)
 		if err != nil {
 			return err
 		}
@@ -105,6 +112,7 @@ func (i *Init) Run() error {
 
 	return i.appInitFn(
 		i.fs,
+		i.httpClient,
 		i.name,
 		i.rootPath,
 		i.envName,
@@ -115,12 +123,14 @@ func (i *Init) Run() error {
 	)
 }
 
-func initIncubator(a app.App) (registry.Registry, error) {
+func initIncubator(a app.App, httpClient *http.Client) (registry.Registry, error) {
+	gh := github.NewGitHub(httpClient)
+
 	return registry.NewGitHub(
 		a,
 		&app.RegistryConfig{
 			Name:     "incubator",
 			Protocol: string(registry.ProtocolGitHub),
 			URI:      defaultIncubatorURI,
-		})
+		}, registry.GitHubClient(gh))
 }

pkg/actions/init_test.go

@@ -16,6 +16,7 @@
 package actions
 
 import (
+	"net/http"
 	"testing"
 
 	"github.com/ksonnet/ksonnet/pkg/app"
@@ -65,12 +66,13 @@ func TestInit(t *testing.T) {
 					OptionServer:                aServerURI,
 					OptionNamespace:             aNamespace,
 					OptionSkipDefaultRegistries: tc.skipRegistries,
+					OptionTLSSkipVerify:         false,
 				}
 
 				a, err := NewInit(in)
 				require.NoError(t, err)
 
-				a.appInitFn = func(fs afero.Fs, name, rootPath, envName, k8sSpecFlag, serverURI, namespace string, registries []registry.Registry) error {
+				a.appInitFn = func(fs afero.Fs, httpClient *http.Client, name, rootPath, envName, k8sSpecFlag, serverURI, namespace string, registries []registry.Registry) error {
 					assert.Equal(t, aFs, fs)
 					assert.Equal(t, aName, name)
 					assert.Equal(t, aRootPath, rootPath)
@@ -93,11 +95,11 @@ func TestInit(t *testing.T) {
 					return nil
 				}
 
-				a.appLoadFn = func(fs afero.Fs, root string, skipFindRoot bool) (app.App, error) {
+				a.appLoadFn = func(fs afero.Fs, httpClient *http.Client, root string, skipFindRoot bool) (app.App, error) {
 					return appMock, nil
 				}
 
-				a.initIncubatorFn = func(a app.App) (registry.Registry, error) {
+				a.initIncubatorFn = func(a app.App, httpClient *http.Client) (registry.Registry, error) {
 					r := &rmocks.Registry{}
 					r.On("Protocol").Return(registry.ProtocolGitHub)
 					r.On("URI").Return("github.com/ksonnet/parts/tree/master/incubator")

pkg/app/app.go

@@ -16,11 +16,11 @@
 package app
 
 import (
+	"net/http"
 	"os"
 	"path/filepath"
 	"sort"
 
-	"github.com/ksonnet/ksonnet/pkg/lib"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
@@ -49,9 +49,6 @@ var (
 	DefaultFilePermissions = os.FileMode(0644)
 	// DefaultFolderPermissions are the default permissions for a folder.
 	DefaultFolderPermissions = os.FileMode(0755)
-
-	// LibUpdater updates ksonnet lib versions.
-	LibUpdater = updateLibData
 )
 
 // App is a ksonnet application.
@@ -70,6 +67,8 @@ type App interface {
 	EnvironmentParams(name string) (string, error)
 	// Fs is the app's afero Fs.
 	Fs() afero.Fs
+	// HTTPClient is the app's http client
+	HTTPClient() *http.Client
 	// CheckUpgrade checks whether an app should be upgraded.
 	CheckUpgrade() (bool, error)
 	// LibPath returns the path of the lib for an environment.
@@ -102,7 +101,7 @@ type App interface {
 }
 
 // Load loads the application configuration.
-func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
+func Load(fs afero.Fs, httpClient *http.Client, cwd string, skipFindRoot bool) (App, error) {
 	log := log.WithField("action", "app.Load")
 	appRoot := cwd
 	if !skipFindRoot {
@@ -116,7 +115,7 @@ func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
 	spec, err := read(fs, appRoot)
 	if os.IsNotExist(err) {
 		// During `ks init`, app.yaml will not yet exist - generate a new one.
-		return NewApp010(fs, appRoot), nil
+		return NewApp010(fs, appRoot, httpClient), nil
 	}
 	if err != nil {
 		return nil, errors.Wrap(err, "reading app configuration")
@@ -126,13 +125,13 @@ func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
 	default:
 		return nil, errors.Errorf("unknown apiVersion %q in %s", spec.APIVersion, appYamlName)
 	case "0.0.1":
-		return NewApp001(fs, appRoot), nil
+		return NewApp001(fs, appRoot, httpClient), nil
 	case "0.1.0", "0.2.0":
 		// TODO TODO
 		// 0.1.0 will auto-upgraded to 0.2.0. 0.1.0 is read-compatible with
 		// 0.2.0, but will be persisted back as 0.2.0. This behavior will be
 		// subsequently changed with new upgrade framework.
-		a := NewApp010(fs, appRoot)
+		a := NewApp010(fs, appRoot, httpClient)
 		log.Debugf("Interpreting app version as latest (0.2.0)", a.baseApp)
 		a.config.APIVersion = "0.2.0"
 		a.baseApp.config.APIVersion = "0.2.0"
@@ -140,19 +139,6 @@ func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
 	}
 }
 
-func updateLibData(fs afero.Fs, k8sSpecFlag, libPath string) (string, error) {
-	lm, err := lib.NewManager(k8sSpecFlag, fs, libPath)
-	if err != nil {
-		return "", err
-	}
-
-	if err := lm.GenerateLibData(); err != nil {
-		return "", err
-	}
-
-	return lm.K8sVersion, nil
-}
-
 func app010LibPath(root string) string {
 	return filepath.Join(root, LibDirName)
 }

pkg/app/app001.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -43,8 +44,8 @@ type App001 struct {
 var _ App = (*App001)(nil)
 
 // NewApp001 creates an App001 instance.
-func NewApp001(fs afero.Fs, root string) *App001 {
-	ba := newBaseApp(fs, root)
+func NewApp001(fs afero.Fs, root string, httpClient *http.Client) *App001 {
+	ba := newBaseApp(fs, root, httpClient)
 
 	return &App001{
 		out:     os.Stdout,
@@ -80,7 +81,7 @@ func (a *App001) AddEnvironment(e *EnvironmentConfig, k8sSpecFlag string, isOver
 		return err
 	}
 
-	_, err = LibUpdater(a.fs, k8sSpecFlag, a.appLibPath(e.Name))
+	_, err = a.libUpdater.UpdateKSLib(k8sSpecFlag, a.appLibPath(e.Name))
 	return err
 }
 
@@ -294,7 +295,7 @@ func (a *App001) convertEnvironment(envName string, dryRun bool) error {
 	}
 
 	k8sSpecFlag := fmt.Sprintf("version:%s", env.KubernetesVersion)
-	_, err = LibUpdater(a.fs, k8sSpecFlag, app010LibPath(a.root))
+	_, err = a.libUpdater.UpdateKSLib(k8sSpecFlag, app010LibPath(a.root))
 	if err != nil {
 		return err
 	}

pkg/app/app001_test.go

@@ -254,17 +254,6 @@ func TestApp001_UpdateTargets(t *testing.T) {
 }
 
 func withApp001Fs(t *testing.T, appName string, fn func(app *App001)) {
-	ogLibUpdater := LibUpdater
-	LibUpdater = func(fs afero.Fs, k8sSpecFlag string, libPath string) (string, error) {
-		path := filepath.Join(libPath, "swagger.json")
-		stageFile(t, fs, "swagger.json", path)
-		return "v1.8.7", nil
-	}
-
-	defer func() {
-		LibUpdater = ogLibUpdater
-	}()
-
 	dir, err := ioutil.TempDir("", "")
 	require.NoError(t, err)
 
@@ -293,6 +282,12 @@ func withApp001Fs(t *testing.T, appName string, fn func(app *App001)) {
 
 	stageFile(t, fs, appName, "/app.yaml")
 
-	app := NewApp001(fs, "/")
+	app := NewApp001(fs, "/", nil)
+	app.libUpdater = fakeLibUpdater(func(k8sSpecFlag string, libPath string) (string, error) {
+		path := filepath.Join(libPath, "swagger.json")
+		stageFile(t, fs, "swagger.json", path)
+		return "v1.8.7", nil
+	})
+
 	fn(app)
 }

pkg/app/app010.go

@@ -18,6 +18,7 @@ package app
 import (
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -39,9 +40,19 @@ type App010 struct {
 
 var _ App = (*App010)(nil)
 
+// App010Opt is a constructor option for App010
+type App010Opt func(*App010)
+
+// App010OptLibUpdater returns an option for setting a KSLibUpdater on an App010
+func App010OptLibUpdater(libUpdater KSLibUpdater) App010Opt {
+	return func(a *App010) {
+		a.libUpdater = libUpdater
+	}
+}
+
 // NewApp010 creates an App010 instance.
-func NewApp010(fs afero.Fs, root string) *App010 {
-	ba := newBaseApp(fs, root)
+func NewApp010(fs afero.Fs, root string, httpClient *http.Client, opts ...App010Opt) *App010 {
+	ba := newBaseApp(fs, root, httpClient)
 
 	a := &App010{
 		baseApp: ba,
@@ -50,6 +61,10 @@ func NewApp010(fs afero.Fs, root string) *App010 {
 		libPaths: make(map[string]string),
 	}
 
+	for _, optFn := range opts {
+		optFn(a)
+	}
+
 	return a
 }
 
@@ -78,7 +93,7 @@ func (a *App010) AddEnvironment(newEnv *EnvironmentConfig, k8sSpecFlag string, i
 	}
 
 	if k8sSpecFlag != "" {
-		ver, err := LibUpdater(a.fs, k8sSpecFlag, app010LibPath(a.root))
+		ver, err := a.libUpdater.UpdateKSLib(k8sSpecFlag, app010LibPath(a.root))
 		if err != nil {
 			return err
 		}
@@ -149,7 +164,7 @@ func (a *App010) LibPath(envName string) (string, error) {
 	}
 
 	ver := fmt.Sprintf("version:%s", env.KubernetesVersion)
-	lm, err := lib.NewManager(ver, a.fs, app010LibPath(a.root))
+	lm, err := lib.NewManager(ver, a.fs, app010LibPath(a.root), a.httpClient)
 	if err != nil {
 		return "", err
 	}

pkg/app/app010_test.go

@@ -17,6 +17,7 @@ package app
 
 import (
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
@@ -26,6 +27,26 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type fakeTransport struct {
+	resp *http.Response
+	err  error
+}
+
+func (f *fakeTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	return f.resp, f.err
+}
+
+// fakeHTTPClient returns an http.Client that will respond with the predefined response and error.
+func fakeHTTPClient(resp *http.Response, err error) *http.Client {
+	c := &http.Client{
+		Transport: &fakeTransport{
+			resp: resp,
+			err:  err,
+		},
+	}
+	return c
+}
+
 func TestApp010_AddEnvironment(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(app *App010) {
 		envs, err := app.Environments()
@@ -196,6 +217,21 @@ func TestApp010_CheckUpgrade_legacy_environments(t *testing.T) {
 
 func TestApp0101_LibPath(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(app *App010) {
+		app.libUpdater = fakeLibUpdater(func(string, string) (string, error) {
+			return "v1.8.7", nil
+		})
+
+		specReader, err := os.Open("../cluster/testdata/swagger.json")
+		if err != nil {
+			require.NoError(t, err, "opening fixture: swagger.json")
+			return
+		}
+		defer specReader.Close()
+		app.httpClient = fakeHTTPClient(
+			&http.Response{
+				StatusCode: 200,
+				Body:       specReader,
+			}, nil)
 		path, err := app.LibPath("default")
 		require.NoError(t, err)
 
@@ -300,16 +336,13 @@ func TestApp0101_UpdateTargets(t *testing.T) {
 	})
 }
 
-func withApp010Fs(t *testing.T, appName string, fn func(app *App010)) {
-	ogLibUpdater := LibUpdater
-	LibUpdater = func(fs afero.Fs, k8sSpecFlag string, libPath string) (string, error) {
-		return "v1.8.7", nil
-	}
+type fakeLibUpdater func(k8sSpecFlag string, libPath string) (string, error)
 
-	defer func() {
-		LibUpdater = ogLibUpdater
-	}()
+func (f fakeLibUpdater) UpdateKSLib(k8sSpecFlag string, libPath string) (string, error) {
+	return f(k8sSpecFlag, libPath)
+}
 
+func withApp010Fs(t *testing.T, appName string, fn func(app *App010)) {
 	dir, err := ioutil.TempDir("", "")
 	require.NoError(t, err)
 
@@ -335,7 +368,10 @@ func withApp010Fs(t *testing.T, appName string, fn func(app *App010)) {
 
 	stageFile(t, fs, appName, "/app.yaml")
 
-	app := NewApp010(fs, "/")
+	app := NewApp010(fs, "/", nil)
+	app.libUpdater = fakeLibUpdater(func(string, string) (string, error) {
+		return "v1.8.7", nil
+	})
 
 	fn(app)
 }

pkg/app/base_app.go

@@ -16,6 +16,7 @@
 package app
 
 import (
+	"net/http"
 	"path/filepath"
 	"sync"
 
@@ -27,8 +28,12 @@ import (
 )
 
 type baseApp struct {
-	root string
-	fs   afero.Fs
+	root       string
+	fs         afero.Fs
+	httpClient *http.Client
+
+	// LibUpdater updates ksonnet lib versions.
+	libUpdater KSLibUpdater
 
 	config    *Spec
 	overrides *Override
@@ -38,9 +43,14 @@ type baseApp struct {
 	load func() error
 }
 
-func newBaseApp(fs afero.Fs, root string) *baseApp {
+func newBaseApp(fs afero.Fs, root string, httpClient *http.Client) *baseApp {
 	ba := &baseApp{
-		fs:     fs,
+		fs:         fs,
+		httpClient: httpClient,
+		libUpdater: ksLibUpdater{
+			fs:         fs,
+			httpClient: httpClient,
+		},
 		root:   root,
 		config: &Spec{},
 		overrides: &Override{
@@ -306,6 +316,10 @@ func (ba *baseApp) Fs() afero.Fs {
 	return ba.fs
 }
 
+func (ba *baseApp) HTTPClient() *http.Client {
+	return ba.httpClient
+}
+
 func (ba *baseApp) Root() string {
 	return ba.root
 }

pkg/app/base_app_test.go

@@ -48,7 +48,7 @@ func Test_baseapp_CurrentEnvironment(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			fs := afero.NewMemMapFs()
 			stageFile(t, fs, "app010_app.yaml", "/app.yaml")
-			ba := newBaseApp(fs, "/")
+			ba := newBaseApp(fs, "/", nil)
 
 			if tc.init != nil {
 				tc.init(t, fs)
@@ -80,7 +80,7 @@ func Test_baseapp_SetCurrentEnvironment(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			fs := afero.NewMemMapFs()
 			stageFile(t, fs, "app010_app.yaml", "/app.yaml")
-			ba := newBaseApp(fs, "/")
+			ba := newBaseApp(fs, "/", nil)
 
 			err := ba.SetCurrentEnvironment(tc.envName)
 			if tc.isErr {
@@ -102,7 +102,7 @@ func Test_baseApp_AddRegistry(t *testing.T) {
 
 	stageFile(t, fs, "app010_app.yaml", "/app.yaml")
 
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 
 	reg := &RegistryConfig{
 		Name: "new",
@@ -120,7 +120,7 @@ func Test_baseApp_AddRegistry_override(t *testing.T) {
 
 	stageFile(t, fs, "app010_app.yaml", "/app.yaml")
 
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 
 	reg := &RegistryConfig{
 		Name: "new",
@@ -138,7 +138,7 @@ func Test_baseApp_AddRegistry_override_existing(t *testing.T) {
 
 	stageFile(t, fs, "app010_app.yaml", "/app.yaml")
 
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 
 	reg := &RegistryConfig{
 		Name: "incubator",
@@ -172,7 +172,7 @@ func Test_baseApp_UpdateRegistry(t *testing.T) {
 			stageFile(t, fs, tc.appFilePath, "/app.yaml")
 		}
 
-		ba := newBaseApp(fs, "/")
+		ba := newBaseApp(fs, "/", nil)
 
 		// Test updating non-existing registry
 		err := ba.UpdateRegistry(&tc.regSpec)
@@ -242,7 +242,7 @@ func Test_baseApp_UpdateLibrary(t *testing.T) {
 			stageFile(t, fs, tc.appFilePath, "/app.yaml")
 		}
 
-		ba := newBaseApp(fs, "/")
+		ba := newBaseApp(fs, "/", nil)
 
 		// Test updating non-existing registry
 		err := ba.UpdateLib(tc.libCfg.Name, tc.env, &tc.libCfg)
@@ -265,7 +265,7 @@ func Test_baseApp_load_override(t *testing.T) {
 	stageFile(t, fs, "app010_app.yaml", "/app.yaml")
 	stageFile(t, fs, "add-registry-override.yaml", "/app.override.yaml")
 
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 
 	err := ba.load()
 	require.NoError(t, err)
@@ -280,7 +280,7 @@ func Test_baseApp_load_override_invalid(t *testing.T) {
 	stageFile(t, fs, "app010_app.yaml", "/app.yaml")
 	stageFile(t, fs, "add-registry-override-invalid.yaml", "/app.override.yaml")
 
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 
 	err := ba.load()
 	require.Error(t, err)
@@ -288,7 +288,7 @@ func Test_baseApp_load_override_invalid(t *testing.T) {
 
 func Test_baseApp_environment_override_is_merged(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	ba := newBaseApp(fs, "/")
+	ba := newBaseApp(fs, "/", nil)
 	ba.load = func() error {
 		return nil
 	}
@@ -341,7 +341,7 @@ func Test_baseApp_environment_override_is_merged(t *testing.T) {
 
 func Test_baseApp_environment_just_override(t *testing.T) {
 	fs := afero.NewMemMapFs()