Unverified Commit c7b94fe5 authored by Oren Shomron's avatar Oren Shomron Committed by GitHub
Browse files

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

Add --tls-skip-verify global option affecting outgoing HTTP requests
parents 746561a9 de033c2c
......@@ -21,16 +21,17 @@ import (
"github.com/ksonnet/ksonnet/pkg/actions"
)
func Test_protoptypeSearchCmd(t *testing.T) {
func Test_prototypeSearchCmd(t *testing.T) {
cases := []cmdTestCase{
{
name: "in general",
args: []string{"prototype", "search", "name"},
action: actionPrototypeSearch,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionQuery: "name",
actions.OptionOutput: "",
actions.OptionApp: nil,
actions.OptionQuery: "name",
actions.OptionOutput: "",
actions.OptionTLSSkipVerify: false,
},
},
{
......@@ -38,9 +39,10 @@ func Test_protoptypeSearchCmd(t *testing.T) {
args: []string{"prototype", "search", "name", "-o", "json"},
action: actionPrototypeSearch,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionQuery: "name",
actions.OptionOutput: "json",
actions.OptionApp: nil,
actions.OptionQuery: "name",
actions.OptionOutput: "json",
actions.OptionTLSSkipVerify: false,
},
},
{
......
......@@ -106,6 +106,7 @@ func newPrototypeUseCmd(a app.App) *cobra.Command {
m := map[string]interface{}{
actions.OptionApp: a,
actions.OptionArguments: rawArgs,
// We don't pass flagTLSSkipVerify because flag parsing is disabled
}
return runAction(actionPrototypeUse, m)
......
......@@ -28,8 +28,9 @@ func Test_prototypeUseCmd(t *testing.T) {
args: []string{"prototype", "use", "name", "--containerPort", "8080"},
action: actionPrototypeUse,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionArguments: []string{"name", "--containerPort", "8080"},
actions.OptionApp: nil,
actions.OptionArguments: []string{"name", "--containerPort", "8080"},
actions.OptionTLSSkipVerify: false,
},
},
{
......@@ -37,8 +38,9 @@ func Test_prototypeUseCmd(t *testing.T) {
args: []string{"generate", "name", "--containerPort", "8080"},
action: actionPrototypeUse,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionArguments: []string{"name", "--containerPort", "8080"},
actions.OptionApp: nil,
actions.OptionArguments: []string{"name", "--containerPort", "8080"},
actions.OptionTLSSkipVerify: false,
},
},
{
......
......@@ -79,10 +79,11 @@ func newRegistryAddCmd(a app.App) *cobra.Command {
}
m := map[string]interface{}{
actions.OptionApp: a,
actions.OptionName: args[0],
actions.OptionURI: args[1],
actions.OptionOverride: viper.GetBool(vRegistryAddOverride),
actions.OptionApp: a,
actions.OptionName: args[0],
actions.OptionURI: args[1],
actions.OptionOverride: viper.GetBool(vRegistryAddOverride),
actions.OptionTLSSkipVerify: viper.GetBool(flagTLSSkipVerify),
}
return runAction(actionRegistryAdd, m)
......
......@@ -28,11 +28,12 @@ func Test_registryAddCmd(t *testing.T) {
args: []string{"registry", "add", "name", "uri"},
action: actionRegistryAdd,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionName: "name",
actions.OptionURI: "uri",
actions.OptionOverride: false,
actions.OptionVersion: "",
actions.OptionApp: nil,
actions.OptionName: "name",
actions.OptionURI: "uri",
actions.OptionOverride: false,
actions.OptionVersion: "",
actions.OptionTLSSkipVerify: false,
},
},
{
......
......@@ -21,6 +21,7 @@ import (
"github.com/ksonnet/ksonnet/pkg/actions"
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)
var (
......@@ -51,8 +52,9 @@ func newRegistryDescribeCmd(a app.App) *cobra.Command {
}
m := map[string]interface{}{
actions.OptionApp: a,
actions.OptionName: args[0],
actions.OptionApp: a,
actions.OptionName: args[0],
actions.OptionTLSSkipVerify: viper.GetBool(flagTLSSkipVerify),
}
return runAction(actionRegistryDescribe, m)
......
......@@ -28,8 +28,9 @@ func Test_envRegistryDescribe(t *testing.T) {
args: []string{"registry", "describe", "name"},
action: actionRegistryDescribe,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionName: "name",
actions.OptionApp: nil,
actions.OptionName: "name",
actions.OptionTLSSkipVerify: false,
},
},
{
......
......@@ -56,8 +56,9 @@ func newRegistryListCmd(a app.App) *cobra.Command {
}
m := map[string]interface{}{
actions.OptionApp: a,
actions.OptionOutput: viper.GetString(vRegistryListOutput),
actions.OptionApp: a,
actions.OptionOutput: viper.GetString(vRegistryListOutput),
actions.OptionTLSSkipVerify: viper.GetBool(flagTLSSkipVerify),
}
return runAction(actionRegistryList, m)
......
......@@ -28,8 +28,9 @@ func Test_registryListCmd(t *testing.T) {
args: []string{"registry", "list"},
action: actionRegistryList,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionOutput: "",
actions.OptionApp: nil,
actions.OptionOutput: "",
actions.OptionTLSSkipVerify: false,
},
},
{
......@@ -37,8 +38,9 @@ func Test_registryListCmd(t *testing.T) {
args: []string{"registry", "list", "-o", "json"},
action: actionRegistryList,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionOutput: "json",
actions.OptionApp: nil,
actions.OptionOutput: "json",
actions.OptionTLSSkipVerify: false,
},
},
{
......
......@@ -28,6 +28,7 @@ import (
"github.com/shomron/pflag"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/spf13/viper"
// Register auth plugins
_ "k8s.io/client-go/plugin/pkg/client/auth"
......@@ -197,6 +198,8 @@ func NewRoot(appFs afero.Fs, wd string, args []string) (*cobra.Command, error) {
rootCmd.PersistentFlags().CountP(flagVerbose, "v", "Increase verbosity. May be given multiple times.")
rootCmd.PersistentFlags().Set("logtostderr", "true")
rootCmd.PersistentFlags().Bool(flagTLSSkipVerify, false, "Skip verification of TLS server certificates")
viper.BindPFlag(flagTLSSkipVerify, rootCmd.PersistentFlags().Lookup(flagTLSSkipVerify))
rootCmd.AddCommand(newApplyCmd(a))
rootCmd.AddCommand(newComponentCmd(a))
......
......@@ -19,6 +19,7 @@ import (
"github.com/ksonnet/ksonnet/pkg/actions"
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)
const (
......@@ -56,8 +57,9 @@ func newUpgradeCmd(a app.App) *cobra.Command {
}
m := map[string]interface{}{
actions.OptionApp: a,
actions.OptionDryRun: dryRun,
actions.OptionApp: a,
actions.OptionDryRun: dryRun,
actions.OptionTLSSkipVerify: viper.GetBool(flagTLSSkipVerify),
}
return runAction(actionUpgrade, m)
......
......@@ -28,8 +28,9 @@ func Test_upgradeCmd(t *testing.T) {
args: []string{"upgrade"},
action: actionUpgrade,
expected: map[string]interface{}{
actions.OptionApp: nil,
actions.OptionDryRun: false,
actions.OptionApp: nil,
actions.OptionDryRun: false,
actions.OptionTLSSkipVerify: false,
},
},
}
......
......@@ -16,14 +16,17 @@
package registry
import (
"net/http"
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/ksonnet/ksonnet/pkg/helm"
"github.com/ksonnet/ksonnet/pkg/util/github"
"github.com/pkg/errors"
)
// Add adds a registry with `name`, `protocol`, and `uri` to
// the current ksonnet application.
func Add(a app.App, protocol Protocol, name string, uri string, isOverride bool) (*Spec, error) {
func Add(a app.App, protocol Protocol, name string, uri string, isOverride bool, httpClient *http.Client) (*Spec, error) {
var r Registry
var err error
......@@ -35,12 +38,13 @@ func Add(a app.App, protocol Protocol, name string, uri string, isOverride bool)
switch protocol {
case ProtocolGitHub:
r, err = githubFactory(a, initSpec)
var ghc = github.NewGitHub(httpClient)
r, err = githubFactory(a, initSpec, GitHubClient(ghc))
case ProtocolFilesystem:
r, err = NewFs(a, initSpec)
case ProtocolHelm:
var hc *helm.HTTPClient
hc, err = helm.NewHTTPClient(initSpec.URI, nil)
hc, err = helm.NewHTTPClient(initSpec.URI, httpClient)
if err != nil {
return nil, errors.Wrap(err, "initializing helm HTTP client")
}
......
......@@ -77,11 +77,11 @@ func TestAdd_GitHub(t *testing.T) {
Return(registryContent, nil, nil)
ghOpt := GitHubClient(ghMock)
githubFactory = func(a app.App, registryRef *app.RegistryConfig) (*GitHub, error) {
githubFactory = func(a app.App, registryRef *app.RegistryConfig, opts ...GitHubOpt) (*GitHub, error) {
return NewGitHub(a, registryRef, ghOpt)
}
spec, err := Add(appMock, ProtocolGitHub, "new", "github.com/foo/bar", true)
spec, err := Add(appMock, ProtocolGitHub, "new", "github.com/foo/bar", true, nil)
require.NoError(t, err)
require.Equal(t, registrySpec, spec)
......@@ -116,7 +116,7 @@ func TestAdd_Helm(t *testing.T) {
return NewHelm(a, registryConfig, httpClient, nil)
}
spec, err := Add(appMock, ProtocolHelm, "new", "http://example.com", false)
spec, err := Add(appMock, ProtocolHelm, "new", "http://example.com", false, nil)
require.NoError(t, err)
expected := &Spec{
......@@ -136,14 +136,14 @@ func TestAdd_Helm(t *testing.T) {
func TestAdd_fs(t *testing.T) {
test.WithApp(t, "/app", func(a *amocks.App, fs afero.Fs) {
_, err := Add(a, ProtocolFilesystem, "/invalid", "", false)
_, err := Add(a, ProtocolFilesystem, "/invalid", "", false, nil)
require.Error(t, err)
})
}
func TestAdd_invalid(t *testing.T) {
test.WithApp(t, "/app", func(a *amocks.App, fs afero.Fs) {
_, err := Add(a, Protocol("invalid"), "", "", false)
_, err := Add(a, Protocol("invalid"), "", "", false, nil)
require.Error(t, err)
})
}
......
......@@ -17,6 +17,7 @@ package registry
import (
"fmt"
"net/http"
"path/filepath"
"strings"
......@@ -30,7 +31,7 @@ import (
// CacheDependency vendors registry dependencies.
// TODO: create unit tests for this once mocks for this package are
// worked out.
func CacheDependency(a app.App, checker InstalledChecker, d pkg.Descriptor, customName string, force bool) (*app.LibraryConfig, error) {
func CacheDependency(a app.App, checker InstalledChecker, d pkg.Descriptor, customName string, force bool, httpClient *http.Client) (*app.LibraryConfig, error) {
logger := log.WithFields(log.Fields{
"action": "registry.CacheDependency",
"part": d.Name,
......@@ -58,7 +59,7 @@ func CacheDependency(a app.App, checker InstalledChecker, d pkg.Descriptor, cust
return nil, fmt.Errorf("registry '%s' does not exist", d.Registry)
}
r, err := Locate(a, regRefSpec)
r, err := Locate(a, regRefSpec, httpClient)
if err != nil {
return nil, err
}
......
......@@ -71,7 +71,7 @@ func Test_CacheDependency(t *testing.T) {
var checker installedChecker
d := pkg.Descriptor{Registry: lib.Registry, Name: lib.Name}
_, err := CacheDependency(a, &checker, d, "", false)
_, err := CacheDependency(a, &checker, d, "", false, nil)
require.NoError(t, err)
test.AssertExists(t, fs, filepath.Join(a.Root(), "vendor", lib.Registry, lib.Name, "parts.yaml"))
......
......@@ -43,12 +43,12 @@ var (
// errInvalidURI is an invalid github uri error.
errInvalidURI = fmt.Errorf("Invalid GitHub URI: try navigating in GitHub to the URI of the folder containing the 'yaml', and using that URI instead. Generally, this URI should be of the form 'github.com/{organization}/{repository}/tree/{branch}/[path-to-directory]'")
githubFactory = func(a app.App, spec *app.RegistryConfig) (*GitHub, error) {
return NewGitHub(a, spec)
githubFactory = func(a app.App, spec *app.RegistryConfig, opts ...GitHubOpt) (*GitHub, error) {
return NewGitHub(a, spec, opts...)
}
)
type ghFactoryFn func(a app.App, spec *app.RegistryConfig) (*GitHub, error)
type ghFactoryFn func(a app.App, spec *app.RegistryConfig, opts ...GitHubOpt) (*GitHub, error)
// GitHubClient is an option for the setting a github client.
func GitHubClient(c github.GitHub) GitHubOpt {
......@@ -190,7 +190,13 @@ func (gh *GitHub) FetchRegistrySpec() (*Spec, error) {
// Get the latest matching commit to determine staleness of cache
sha, err := gh.resolveLatestSHA()
if err != nil || sha == "" {
log.Warnf("%v", errors.Wrapf(err, "unable to resolve commit for refspec: %v", gh.hd.refSpec))
errMsg := errors.Wrapf(err, "unable to resolve commit for refspec: %v", gh.hd.refSpec)
if registrySpec == nil || cachedVersion == "" {
// In this case, we failed both the cache and to fetch from remote
return nil, errMsg
}
log.Warnf("%v", errMsg)
log.Warnf("falling back to cached version (%v)", cachedVersion)
updateLibVersions(registrySpec, gh.hd.refSpec)
return registrySpec, nil
......
......@@ -695,3 +695,15 @@ func TestGitHub_CacheRoot(t *testing.T) {
assert.Equal(t, tc.expected, result, tc.name)
}
}
func Test_githubFactory(t *testing.T) {
regCfg := &app.RegistryConfig{
Name: "incubator",
URI: "github.com/foo/bar",
}
ghMock := &mocks.GitHub{}
optGh := GitHubClient(ghMock)
gh, err := githubFactory(nil, regCfg, optGh)
require.NoError(t, err, "github constructor")
assert.Equal(t, ghMock, gh.ghClient)
}
......@@ -16,22 +16,25 @@
package registry
import (
"net/http"
"path/filepath"
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/ksonnet/ksonnet/pkg/helm"
"github.com/ksonnet/ksonnet/pkg/util/github"
"github.com/pkg/errors"
)
// Locate locates a registry given a spec.
func Locate(a app.App, spec *app.RegistryConfig) (Registry, error) {
func Locate(a app.App, spec *app.RegistryConfig, httpClient *http.Client) (Registry, error) {
switch Protocol(spec.Protocol) {
case ProtocolGitHub:
return githubFactory(a, spec)
var ghc = github.NewGitHub(httpClient)
return githubFactory(a, spec, GitHubClient(ghc))
case ProtocolFilesystem:
return NewFs(a, spec)
case ProtocolHelm:
client, err := helm.NewHTTPClient(spec.URI, nil)
client, err := helm.NewHTTPClient(spec.URI, httpClient)
if err != nil {
return nil, err
}
......@@ -58,7 +61,7 @@ func registrySpecFilePath(a app.App, r Registry) string {
}
// List returns a list of alphabetically sorted Registries.
func List(ksApp app.App) ([]Registry, error) {
func List(ksApp app.App, httpClient *http.Client) ([]Registry, error) {
var registries []Registry
appRegistries, err := ksApp.Registries()
if err != nil {
......@@ -66,7 +69,7 @@ func List(ksApp app.App) ([]Registry, error) {
}
for name, regRef := range appRegistries {
regRef.Name = name
r, err := Locate(ksApp, regRef)
r, err := Locate(ksApp, regRef, httpClient)
if err != nil {
return nil, err
}
......
......@@ -35,7 +35,7 @@ func Test_List(t *testing.T) {
Return("12345", nil)
ghcOpt := GitHubClient(c)
githubFactory = func(a app.App, spec *app.RegistryConfig) (*GitHub, error) {
githubFactory = func(a app.App, spec *app.RegistryConfig, opts ...GitHubOpt) (*GitHub, error) {
return NewGitHub(a, spec, ghcOpt)
}
......@@ -49,7 +49,7 @@ func Test_List(t *testing.T) {
appMock := &mocks.App{}
appMock.On("Registries").Return(specs, nil)
registries, err := List(appMock)
registries, err := List(appMock, nil)
require.NoError(t, err)
require.Len(t, registries, 1)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment