diff --git a/client_int_test.go b/client_int_test.go index 73139ece7..fdefc04a4 100644 --- a/client_int_test.go +++ b/client_int_test.go @@ -204,7 +204,7 @@ func TestRemoteRepositories(t *testing.T) { // match those created by the kn func plugin CLI. func newClient(verbose bool) *fn.Client { builder := buildpacks.NewBuilder(verbose) - pusher := docker.NewPusher(verbose) + pusher := docker.NewPusher(docker.WithVerbose(verbose)) deployer := knative.NewDeployer(DefaultNamespace, verbose) remover := knative.NewRemover(DefaultNamespace, verbose) lister := knative.NewLister(DefaultNamespace, verbose) diff --git a/client_test.go b/client_test.go index 8d92f315c..d81b5ae38 100644 --- a/client_test.go +++ b/client_test.go @@ -63,7 +63,7 @@ func TestClient_InstantiationCreatesRepositoriesPath(t *testing.T) { } // Instruct the system to use the above test root directory as the home dir. - os.Setenv("XDG_CONFIG_HOME", rootAbs) + defer WithEnvVar(t, "XDG_CONFIG_HOME", rootAbs)() // The expected full path to the repositories should be: expected := filepath.Join(rootAbs, "func", "repositories") diff --git a/cmd/build.go b/cmd/build.go index a15ff1a4e..2310fea54 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -12,13 +12,6 @@ import ( fn "knative.dev/kn-plugin-func" ) -func buildConfigToClientOptions(cfg buildConfig) ClientOptions { - return ClientOptions{ - Registry: cfg.Registry, - Verbose: cfg.Verbose, - } -} - func NewBuildCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Use: "build", @@ -87,7 +80,7 @@ func ValidNamespaceAndRegistry(path string) survey.Validator { } } -func runBuild(cmd *cobra.Command, _ []string, clientFn ClientFactory) (err error) { +func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) { config, err := newBuildConfig().Prompt() if err != nil { if errors.Is(err, terminal.InterruptErr) { @@ -151,7 +144,16 @@ func runBuild(cmd *cobra.Command, _ []string, clientFn ClientFactory) (err error config.Registry = "" } - client := clientFn(buildConfigToClientOptions(config)) + // TODO(lkingland): The below deferred options gathering is what will + // re-enable the addition of alternative implementations of the Builder, + // unblocking PR https://github.com/knative-sandbox/kn-plugin-func/pull/842 + // the implementation of which will be inserted here. + + // Create a client using the registry defined in config plus any additional + // options provided (such as mocks for testing) + client, done := newClient(ClientConfig{Verbose: config.Verbose}, + fn.WithRegistry(config.Registry)) + defer done() err = client.Build(cmd.Context(), config.Path) if err == nil && config.Push { diff --git a/cmd/build_test.go b/cmd/build_test.go index 7bd37b92d..45a1b679e 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -8,6 +8,7 @@ import ( fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" + . "knative.dev/kn-plugin-func/testing" ) // TestBuild_InvalidRegistry ensures that running build specifying the name of the @@ -19,7 +20,7 @@ func TestBuild_InvalidRegistry(t *testing.T) { ) // Run this test in a temporary directory - defer fromTempDir(t)() + defer Fromtemp(t)() // Write a func.yaml config which does not specify an image funcYaml := `name: testymctestface namespace: "" @@ -38,11 +39,10 @@ created: 2021-01-01T00:00:00+00:00 t.Fatal(err) } - // Create a command with a client constructor fn that instantiates a client - // with a the mocked builder. - cmd := NewBuildCmd(func(options ClientOptions) *fn.Client { + // Create build command that will use a mock builder. + cmd := NewBuildCmd(NewClientFactory(func() *fn.Client { return fn.New(fn.WithBuilder(builder)) - }) + })) // Execute the command cmd.SetArgs(args) @@ -99,7 +99,7 @@ created: 2009-11-10 23:00:00`, }, } mockBuilder := mock.NewBuilder() - cmd := NewBuildCmd(func(options ClientOptions) *fn.Client { + cmd := NewBuildCmd(NewClientFactory(func() *fn.Client { pusher := mockPusher if tt.wantErr { pusher = failPusher @@ -108,7 +108,7 @@ created: 2009-11-10 23:00:00`, fn.WithBuilder(mockBuilder), fn.WithPusher(pusher), ) - }) + })) tempDir, err := os.MkdirTemp("", "func-tests") if err != nil { diff --git a/cmd/client.go b/cmd/client.go index 4427b62a0..f7d3aaaf9 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -1,6 +1,10 @@ package cmd import ( + "fmt" + "net/http" + "os" + fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/buildpacks" "knative.dev/kn-plugin-func/docker" @@ -12,101 +16,120 @@ import ( "knative.dev/kn-plugin-func/progress" ) -type ClientOptions struct { - Namespace string - Registry string - Repository string - RepositoriesPath string - Verbose bool +// ClientConfig settings for use with NewClient +// These are the minimum settings necessary to create the default client +// instance which has most subsystems initialized. +type ClientConfig struct { + // Namespace in the remote cluster to use for any client commands which + // touch the remote. Optional. Empty namespace indicates the namespace + // currently configured in the client's connection should be used. + Namespace string + + // Verbose logging. By default logging output is kept to the bare minimum. + // Use this flag to configure verbose logging throughout. + Verbose bool } -type ClientFactory func(opts ClientOptions) *fn.Client +// ClientFactory defines a constructor which assists in the creation of a Client +// for use by commands. +// See the NewClient constructor which is the fully populated ClientFactory used +// by commands by default. +// See NewClientFactory which constructs a minimal CientFactory for use +// during testing. +type ClientFactory func(ClientConfig, ...fn.Option) (*fn.Client, func()) -// NewDefaultClientFactory returns a function that creates instances of function.Client and a cleanup routine. -// -// This function may allocate resources that are used by produced instances of function.Client. -// -// To free these resources (after instances of function.Client are no longer in use) -// caller of this function has to invoke the cleanup routine. -// -// Usage: -// newClient, cleanUp := NewDefaultClientFactory() -// defer cleanUp() -// fnClient := newClient() -// // use your fnClient here... -func NewDefaultClientFactory() (newClient ClientFactory, cleanUp func() error) { - - var transportOpts []fnhttp.Option - var additionalCredLoaders []creds.CredentialsCallback - - switch { - case openshift.IsOpenShift(): - transportOpts = append(transportOpts, openshift.WithOpenShiftServiceCA()) - additionalCredLoaders = openshift.GetDockerCredentialLoaders() - default: +// NewClientFactory enables simple instantiation of an fn.Client, such as +// for mocking during tests or for minimal api usage. +// Given is a minimal Client constructor, Returned is full ClientFactory +// with the aspects of normal (full) Client construction (namespace, verbosity +// level, additional options and the returned cleanup function) ignored. +func NewClientFactory(n func() *fn.Client) ClientFactory { + return func(_ ClientConfig, _ ...fn.Option) (*fn.Client, func()) { + return n(), func() {} } +} - transport := fnhttp.NewRoundTripper(transportOpts...) - cleanUp = func() error { - return transport.Close() - } - - newClient = func(clientOptions ClientOptions) *fn.Client { - verbose := clientOptions.Verbose - builder := buildpacks.NewBuilder(verbose) - - progressListener := progress.New(verbose) - - credentialsProvider := creds.NewCredentialsProvider( - creds.WithPromptForCredentials(newPromptForCredentials()), - creds.WithPromptForCredentialStore(newPromptForCredentialStore()), - creds.WithTransport(transport), - creds.WithAdditionalCredentialLoaders(additionalCredLoaders...)) - - pusher := docker.NewPusher(verbose, - docker.WithCredentialsProvider(credentialsProvider), - docker.WithProgressListener(progressListener), - docker.WithTransport(transport)) - - deployer := knative.NewDeployer(clientOptions.Namespace, verbose) - - pipelinesProvider := tekton.NewPipelinesProvider(verbose, - tekton.WithNamespace(clientOptions.Namespace), - tekton.WithProgressListener(progressListener), - tekton.WithCredentialsProvider(credentialsProvider)) - - remover := knative.NewRemover(clientOptions.Namespace, verbose) - - describer := knative.NewDescriber(clientOptions.Namespace, verbose) - - lister := knative.NewLister(clientOptions.Namespace, verbose) - - runner := docker.NewRunner(verbose) - - opts := []fn.Option{ - fn.WithRepository(clientOptions.Repository), // URI of repository override - fn.WithRegistry(clientOptions.Registry), - fn.WithVerbose(verbose), - fn.WithTransport(transport), - fn.WithProgressListener(progressListener), - fn.WithBuilder(builder), - fn.WithPipelinesProvider(pipelinesProvider), - fn.WithRemover(remover), - fn.WithDescriber(describer), - fn.WithLister(lister), - fn.WithRunner(runner), - fn.WithDeployer(deployer), - fn.WithPusher(pusher), +// NewClient constructs an fn.Client with the majority of +// the concrete implementations set. Provide additional Options to this constructor +// to override or augment as needed, or override the ClientFactory passed to +// commands entirely to mock for testing. Note the reutrned cleanup function. +// 'Namespace' is optional. If not provided (see DefaultNamespace commentary), +// the currently configured is used. +// 'Verbose' indicates the system should write out a higher amount of logging. +// Example: +// client, done := NewClient("",false) +// defer done() +func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { + var ( + p = progress.New(cfg.Verbose) // updates the CLI + t = newTransport() // may provide a custom impl which proxies + c = newCredentialsProvider(t) // for accessing registries + o = []fn.Option{ // standard (shared) options for all commands + fn.WithVerbose(cfg.Verbose), + fn.WithProgressListener(p), + fn.WithTransport(t), + fn.WithBuilder(buildpacks.NewBuilder(cfg.Verbose)), + fn.WithRemover(knative.NewRemover(cfg.Namespace, cfg.Verbose)), + fn.WithDescriber(knative.NewDescriber(cfg.Namespace, cfg.Verbose)), + fn.WithLister(knative.NewLister(cfg.Namespace, cfg.Verbose)), + fn.WithRunner(docker.NewRunner(cfg.Verbose)), + fn.WithDeployer(knative.NewDeployer(cfg.Namespace, cfg.Verbose)), + fn.WithPipelinesProvider(tekton.NewPipelinesProvider( + tekton.WithNamespace(cfg.Namespace), + tekton.WithProgressListener(p), + tekton.WithCredentialsProvider(c), + tekton.WithVerbose(cfg.Verbose))), + fn.WithPusher(docker.NewPusher( + docker.WithCredentialsProvider(c), + docker.WithProgressListener(p), + docker.WithTransport(t), + docker.WithVerbose(cfg.Verbose))), } + ) - if clientOptions.RepositoriesPath != "" { - opts = append(opts, fn.WithRepositoriesPath(clientOptions.RepositoriesPath)) // path to repositories in disk + // Client is constructed with standard options plus any additional options + // which either augment or override the defaults. + client := fn.New(append(o, options...)...) + + // A deferrable cleanup function which is used to perform any cleanup, such + // as closing the transport + cleanup := func() { + if err := t.Close(); err != nil { + fmt.Fprintf(os.Stderr, "error closing http transport. %v", err) } - - return fn.New(opts...) } - return newClient, cleanUp + return client, cleanup +} + +// newTransport returns a transport with cluster-flavor-specific variations +// which take advantage of additional features offered by cluster variants. +func newTransport() fnhttp.RoundTripCloser { + if openshift.IsOpenShift() { + return fnhttp.NewRoundTripper(openshift.WithOpenShiftServiceCA()) + } + + // Other cluster variants ... + + return fnhttp.NewRoundTripper() // Default (vanilla k8s) +} + +// newCredentialsProvider returns a credentials provider which possibly +// has cluster-flavor specific additional credential loaders to take advantage +// of features or configuration nuances of cluster variants. +func newCredentialsProvider(t http.RoundTripper) docker.CredentialsProvider { + options := []creds.Opt{ + creds.WithPromptForCredentials(newPromptForCredentials()), + creds.WithPromptForCredentialStore(newPromptForCredentialStore()), + creds.WithTransport(t), + } + // The OpenShift variant has additional ways to load credentials + if openshift.IsOpenShift() { + options = append(options, + creds.WithAdditionalCredentialLoaders(openshift.GetDockerCredentialLoaders()...)) + } + // Other cluster variants can be supported here + return creds.NewCredentialsProvider(options...) } func GetDefaultRegistry() string { diff --git a/cmd/config_labels_test.go b/cmd/config_labels_test.go index 197a5622f..c5de7dfec 100644 --- a/cmd/config_labels_test.go +++ b/cmd/config_labels_test.go @@ -106,6 +106,7 @@ func TestNewConfigLabelsCmd(t *testing.T) { labels := &loaderSaver.f.Labels cmd := NewConfigLabelsCmd(&loaderSaver) + cmd.SetArgs([]string{}) // Do not use test command args run := createRunFunc(cmd, t) @@ -143,6 +144,7 @@ func TestListLabels(t *testing.T) { *labels = append(*labels, p("a", "b"), p("c", "d")) cmd := NewConfigLabelsCmd(&loaderSaver) + cmd.SetArgs([]string{}) // Do not use test command args ctx := context.Background() c, _, err := vt10x.NewVT10XConsole() @@ -151,8 +153,6 @@ func TestListLabels(t *testing.T) { } defer c.Close() - cmd.SetArgs(make([]string, 0)) - errChan := make(chan error, 1) func() { var err error diff --git a/cmd/create.go b/cmd/create.go index 73096835d..99da1cf89 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -25,14 +25,6 @@ type ErrInvalidRuntime error // ErrInvalidTemplate indicates that the passed template was invalid. type ErrInvalidTemplate error -func createConfigToClientOptions(cfg createConfig) ClientOptions { - return ClientOptions{ - RepositoriesPath: cfg.RepositoriesPath, - Repository: cfg.Repository, - Verbose: cfg.Verbose, - } -} - // NewCreateCmd creates a create command using the given client creator. func NewCreateCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ @@ -89,16 +81,14 @@ EXAMPLES cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)") // Help Action - cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { - runCreateHelp(cmd, args, newClient) - }) + cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { runCreateHelp(cmd, args, newClient) }) // Run Action cmd.RunE = func(cmd *cobra.Command, args []string) error { return runCreate(cmd, args, newClient) } - // Tab Completion + // Tab completion if err := cmd.RegisterFlagCompletionFunc("language", newRuntimeCompletionFunc(newClient)); err != nil { fmt.Fprintf(os.Stderr, "unable to provide language runtime suggestions: %v", err) } @@ -122,7 +112,10 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err // Client // From environment variables, flags, arguments, and user prompts if --confirm // (in increasing levels of precidence) - client := newClient(createConfigToClientOptions(cfg)) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepository(cfg.Repository), // Use exactly this repo OR + fn.WithRepositoriesPath(cfg.RepositoriesPath)) // Path on disk to installed repos + defer done() // Validate - a deeper validation than that which is performed when // instantiating the client with the raw config above. @@ -163,7 +156,10 @@ func runCreateHelp(cmd *cobra.Command, args []string, newClient ClientFactory) { cfg, err := newCreateConfig(cmd, args, newClient) failSoft(err) - client := newClient(createConfigToClientOptions(cfg)) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepositoriesPath(cfg.RepositoriesPath), + fn.WithRepository(cfg.Repository)) + defer done() options, err := runtimeTemplateOptions(client) // human-friendly failSoft(err) @@ -258,7 +254,8 @@ func newCreateConfig(cmd *cobra.Command, args []string, newClient ClientFactory) // Create a tempoarary client for use by the following prompts to complete // runtime/template suggestions etc - client := newClient(createConfigToClientOptions(cfg)) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) + defer done() // IN confirm mode. If also in an interactive terminal, run prompts. if interactiveTerminal() { @@ -508,7 +505,8 @@ func newRuntimeCompletionFunc(newClient ClientFactory) flagCompletionFunc { if err != nil { fmt.Fprintf(os.Stderr, "error creating client config for flag completion: %v", err) } - client := newClient(createConfigToClientOptions(cfg)) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) + defer done() return CompleteRuntimeList(cmd, args, toComplete, client) } } @@ -519,7 +517,8 @@ func newTemplateCompletionFunc(newClient ClientFactory) flagCompletionFunc { if err != nil { fmt.Fprintf(os.Stderr, "error creating client config for flag completion: %v", err) } - client := newClient(createConfigToClientOptions(cfg)) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) + defer done() return CompleteTemplateList(cmd, args, toComplete, client) } } diff --git a/cmd/create_test.go b/cmd/create_test.go index efd8b17bb..adc0bced8 100644 --- a/cmd/create_test.go +++ b/cmd/create_test.go @@ -2,26 +2,20 @@ package cmd import ( "errors" - "fmt" - "io/ioutil" - "os" "path/filepath" "testing" - fn "knative.dev/kn-plugin-func" + . "knative.dev/kn-plugin-func/testing" "knative.dev/kn-plugin-func/utils" ) // TestCreate_Execute ensures that an invocation of create with minimal settings // and valid input completes without error; degenerate case. func TestCreate_Execute(t *testing.T) { - defer fromTempDir(t)() + defer Fromtemp(t)() - // command with a client factory which yields a fully default client. - cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() }) - cmd.SetArgs([]string{ - fmt.Sprintf("--language=%s", "go"), - }) + cmd := NewCreateCmd(NewClient) + cmd.SetArgs([]string{"--language", "go"}) if err := cmd.Execute(); err != nil { t.Fatal(err) @@ -31,11 +25,10 @@ func TestCreate_Execute(t *testing.T) { // TestCreate_NoRuntime ensures that an invocation of create must be // done with a runtime. func TestCreate_NoRuntime(t *testing.T) { - defer fromTempDir(t)() + defer Fromtemp(t)() - // command with a client factory which yields a fully default client. - cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() }) - cmd.SetArgs([]string{}) + cmd := NewCreateCmd(NewClient) + cmd.SetArgs([]string{}) // Do not use test command args err := cmd.Execute() var e ErrNoRuntime @@ -47,13 +40,10 @@ func TestCreate_NoRuntime(t *testing.T) { // TestCreate_WithNoRuntime ensures that an invocation of create must be // done with one of the valid runtimes only. func TestCreate_WithInvalidRuntime(t *testing.T) { - defer fromTempDir(t)() + defer Fromtemp(t)() - // command with a client factory which yields a fully default client. - cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() }) - cmd.SetArgs([]string{ - fmt.Sprintf("--language=%s", "test"), - }) + cmd := NewCreateCmd(NewClient) + cmd.SetArgs([]string{"--language", "invalid"}) err := cmd.Execute() var e ErrInvalidRuntime @@ -65,14 +55,10 @@ func TestCreate_WithInvalidRuntime(t *testing.T) { // TestCreate_InvalidTemplate ensures that an invocation of create must be // done with one of the valid templates only. func TestCreate_InvalidTemplate(t *testing.T) { - defer fromTempDir(t)() + defer Fromtemp(t)() - // command with a client factory which yields a fully default client. - cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() }) - cmd.SetArgs([]string{ - fmt.Sprintf("--language=%s", "go"), - fmt.Sprintf("--template=%s", "events"), - }) + cmd := NewCreateCmd(NewClient) + cmd.SetArgs([]string{"--language", "go", "--template", "invalid"}) err := cmd.Execute() var e ErrInvalidTemplate @@ -84,14 +70,11 @@ func TestCreate_InvalidTemplate(t *testing.T) { // TestCreate_ValidatesName ensures that the create command only accepts // DNS-1123 labels for Function name. func TestCreate_ValidatesName(t *testing.T) { - defer fromTempDir(t)() - - // Create a new Create command with a fn.Client construtor - // which returns a default (noop) client suitable for tests. - cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() }) + defer Fromtemp(t)() // Execute the command with a function name containing invalid characters and // confirm the expected error is returned + cmd := NewCreateCmd(NewClient) cmd.SetArgs([]string{"invalid!"}) err := cmd.Execute() var e utils.ErrInvalidFunctionName @@ -100,78 +83,27 @@ func TestCreate_ValidatesName(t *testing.T) { } } -// TestCreate_RepositoriesPath ensures that the create command utilizes the -// expected repositories path, respecting the setting for XDG_CONFIG_PATH +// TestCreateConfig_RepositoriesPath ensures that the create command utilizes +// the expected repositories path, respecting the setting for XDG_CONFIG_PATH // when deriving the default -func TestCreate_RepositoriesPath(t *testing.T) { - defer fromTempDir(t)() +func TestCreateConfig_RepositoriesPath(t *testing.T) { + defer Fromtemp(t)() // Update XDG_CONFIG_HOME to point to some arbitrary location. - xdgConfigHome, err := ioutil.TempDir("", "alice") - if err != nil { - t.Fatal(err) - } - os.Setenv("XDG_CONFIG_HOME", xdgConfigHome) + xdgConfigHome := t.TempDir() + defer WithEnvVar(t, "XDG_CONFIG_HOME", xdgConfigHome)() - // The expected full path to repositories: + // The expected full path is XDG_CONFIG_HOME/func/repositories expected := filepath.Join(xdgConfigHome, "func", "repositories") - // Create command takes a function which will be invoked with the final - // state of the createConfig, usually used to do fn.Client instantiation - // after flags, environment variables, etc. are calculated. In this case it - // will validate the test condition: that config reflects the value of - // XDG_CONFIG_HOME, and secondarily the path suffix `func/repositories`. - cmd := NewCreateCmd(func(cfg ClientOptions) *fn.Client { - if cfg.RepositoriesPath != expected { - t.Fatalf("expected repositories default path to be '%v', got '%v'", expected, cfg.RepositoriesPath) - } - return fn.New() - }) - cmd.SetArgs([]string{ - fmt.Sprintf("--language=%s", "go"), - }) - // Invoke the command, which is an airball, but does invoke the client - // constructor, which which evaluates the aceptance condition of ensuring the - // default repositories path was updated based on XDG_CONFIG_HOME. - if err = cmd.Execute(); err != nil { - t.Fatalf("unexpected error running 'create' with a default (noop) client instance: %v", err) - } -} - -// Helpers ---- - -// change directory into a new temp directory. -// returned is a closure which cleans up; intended to be run as a defer: -// defer within(t, /some/path)() -func fromTempDir(t *testing.T) func() { - t.Helper() - tmp := mktmp(t) // create temp directory - owd := pwd(t) // original working directory - cd(t, tmp) // change to the temp directory - return func() { // return a deferable cleanup closure - os.RemoveAll(tmp) // remove temp directory - cd(t, owd) // change director back to original - } -} - -func mktmp(t *testing.T) string { - d, err := ioutil.TempDir("", "dir") + cmd := NewCreateCmd(NewClient) + cmd.SetArgs([]string{}) // Do not use test command args + cfg, err := newCreateConfig(cmd, []string{}, NewClient) if err != nil { t.Fatal(err) } - return d -} -func pwd(t *testing.T) string { - d, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - return d -} - -func cd(t *testing.T, dir string) { - if err := os.Chdir(dir); err != nil { - t.Fatal(err) + if cfg.RepositoriesPath != expected { + t.Fatalf("expected repositories default path to be '%v', got '%v'", expected, cfg.RepositoriesPath) } } diff --git a/cmd/delete.go b/cmd/delete.go index 3ce49f014..9aba57277 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -32,12 +32,12 @@ No local files are deleted. `, SuggestFor: []string{"remove", "rm", "del"}, ValidArgsFunction: CompleteFunctionList, - PreRunE: bindEnv("path", "confirm", "namespace", "all"), + PreRunE: bindEnv("path", "confirm", "all"), + SilenceUsage: true, // no usage dump on error } cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") cmd.Flags().StringP("all", "a", "true", "Delete all resources created for a function, eg. Pipelines, Secrets, etc. (Env: $FUNC_ALL) (allowed values: \"true\", \"false\")") - setNamespaceFlag(cmd) setPathFlag(cmd) cmd.SetHelpFunc(defaultTemplatedHelp) @@ -49,7 +49,7 @@ No local files are deleted. return cmd } -func runDelete(cmd *cobra.Command, args []string, clientFn ClientFactory) (err error) { +func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { config, err := newDeleteConfig(args).Prompt() if err != nil { if err == terminal.InterruptErr { @@ -87,10 +87,8 @@ func runDelete(cmd *cobra.Command, args []string, clientFn ClientFactory) (err e } // Create a client instance from the now-final config - client := clientFn(ClientOptions{ - Namespace: config.Namespace, - Verbose: config.Verbose, - }) + client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose}) + defer done() // Invoke remove using the concrete client impl return client.Remove(cmd.Context(), function, config.DeleteAll) diff --git a/cmd/delete_test.go b/cmd/delete_test.go index a21ed4c82..b8564fa74 100644 --- a/cmd/delete_test.go +++ b/cmd/delete_test.go @@ -6,15 +6,15 @@ import ( fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" + . "knative.dev/kn-plugin-func/testing" ) // TestDelete_ByName ensures that running delete specifying the name of the // Function explicitly as an argument invokes the remover appropriately. func TestDelete_ByName(t *testing.T) { var ( - testname = "testname" // explicit name for the Function - args = []string{testname} // passed as the lone argument - remover = mock.NewRemover() // with a mock remover + testname = "testname" // explicit name for the Function + remover = mock.NewRemover() // with a mock remover ) // Remover fails the test if it receives the incorrect name @@ -28,14 +28,12 @@ func TestDelete_ByName(t *testing.T) { // Create a command with a client constructor fn that instantiates a client // with a the mocked remover. - cmd := NewDeleteCmd(func(ClientOptions) *fn.Client { + cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client { return fn.New(fn.WithRemover(remover)) - }) + })) - // Execute the command - cmd.SetArgs(args) - err := cmd.Execute() - if err != nil { + cmd.SetArgs([]string{testname}) + if err := cmd.Execute(); err != nil { t.Fatal(err) } @@ -49,7 +47,7 @@ func TestDelete_ByName(t *testing.T) { // context invokes remove and with the correct name (reads name from func.yaml) func TestDelete_ByProject(t *testing.T) { // from within a new temporary directory - defer fromTempDir(t)() + defer Fromtemp(t)() // Write a func.yaml config which specifies a name funcYaml := `name: bar @@ -80,12 +78,12 @@ created: 2021-01-01T00:00:00+00:00 // Command with a Client constructor that returns client with the // mocked remover. - cmd := NewDeleteCmd(func(ClientOptions) *fn.Client { + cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client { return fn.New(fn.WithRemover(remover)) - }) + })) + cmd.SetArgs([]string{}) // Do not use test command args // Execute the command simulating no arguments. - cmd.SetArgs([]string{}) err := cmd.Execute() if err != nil { t.Fatal(err) @@ -108,9 +106,9 @@ func TestDelete_NameAndPathExclusivity(t *testing.T) { remover := mock.NewRemover() // Command with a Client constructor using the mock remover. - cmd := NewDeleteCmd(func(ClientOptions) *fn.Client { + cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client { return fn.New(fn.WithRemover(remover)) - }) + })) // Execute the command simulating the invalid argument combination of both // a path and an explicit name. diff --git a/cmd/deploy.go b/cmd/deploy.go index 8db10dc88..807d88450 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -17,14 +17,6 @@ import ( "knative.dev/kn-plugin-func/docker/creds" ) -func deployConfigToClientOptions(cfg deployConfig) ClientOptions { - return ClientOptions{ - Namespace: cfg.Namespace, - Verbose: cfg.Verbose, - Registry: cfg.Registry, - } -} - func NewDeployCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Use: "deploy", @@ -51,7 +43,7 @@ that is pushed to an image registry, and finally the function's Knative service {{.Name}} deploy --image quay.io/myuser/myfunc -n myns `, SuggestFor: []string{"delpoy", "deplyo"}, - PreRunE: bindEnv("image", "namespace", "path", "registry", "confirm", "build", "push", "git-url", "git-branch", "git-dir"), + PreRunE: bindEnv("image", "path", "registry", "confirm", "build", "push", "git-url", "git-branch", "git-dir"), } cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") @@ -66,7 +58,6 @@ that is pushed to an image registry, and finally the function's Knative service cmd.Flags().StringP("build", "b", fn.DefaultBuildType, fmt.Sprintf("Build specifies the way the function should be built. Supported types are %s (Env: $FUNC_BUILD)", fn.SupportedBuildTypes(true))) cmd.Flags().BoolP("push", "u", true, "Attempt to push the function image to registry before deploying (Env: $FUNC_PUSH)") setPathFlag(cmd) - setNamespaceFlag(cmd) if err := cmd.RegisterFlagCompletionFunc("build", CompleteDeployBuildType); err != nil { fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) @@ -163,7 +154,9 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err config.Registry = "" } - client := newClient(deployConfigToClientOptions(config)) + client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose}, + fn.WithRegistry(config.Registry)) + defer done() switch currentBuildType { case fn.BuildTypeLocal, "": diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index bdbb87700..2b8c5fbcc 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -9,6 +9,7 @@ import ( "k8s.io/utils/pointer" fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" + . "knative.dev/kn-plugin-func/testing" ) func Test_runDeploy(t *testing.T) { @@ -109,19 +110,27 @@ created: 2009-11-10 23:00:00`, }, } deployer := mock.NewDeployer() - defer fromTempDir(t)() - cmd := NewDeployCmd(func(opts ClientOptions) *fn.Client { + defer Fromtemp(t)() + cmd := NewDeployCmd(NewClientFactory(func() *fn.Client { return fn.New( fn.WithPipelinesProvider(pipeline), - fn.WithDeployer(deployer), - ) - }) + fn.WithDeployer(deployer)) + })) + cmd.SetArgs([]string{}) // Do not use test command args + // TODO: the below viper.SetDefault calls appear to be altering + // the default values of flags as a way set various values of flags. + // This could perhaps be better achieved by constructing an array + // of flag arguments, set via cmd.SetArgs(...). This would more directly + // test the use-case of flag values (as opposed to the indirect proxy + // of their defaults), and would avoid the need to call viper.Reset() to + // avoid affecting other tests. viper.SetDefault("git-url", tt.gitURL) viper.SetDefault("git-branch", tt.gitBranch) viper.SetDefault("git-dir", tt.gitDir) viper.SetDefault("build", tt.buildType) viper.SetDefault("registry", "docker.io/tigerteam") + defer viper.Reset() // set test case's func.yaml if err := os.WriteFile("func.yaml", []byte(tt.funcFile), os.ModePerm); err != nil { diff --git a/cmd/func/main.go b/cmd/func/main.go index 9ac9932a5..e40812b3a 100644 --- a/cmd/func/main.go +++ b/cmd/func/main.go @@ -10,8 +10,7 @@ import ( "knative.dev/kn-plugin-func/cmd" ) -// Statically-populated build metadata set -// by `make build`. +// Statically-populated build metadata set by `make build`. var date, vers, hash string func main() { @@ -20,34 +19,26 @@ func main() { sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) - go func() { <-sigs cancel() - // second sigint/sigterm is treated as sigkill - <-sigs + <-sigs // second sigint/sigterm is treated as sigkill os.Exit(137) }() - root, err := cmd.NewRootCmd(cmd.RootCommandConfig{ - Name: "func", - Date: date, - Version: vers, - Hash: hash, - }) + cfg := cmd.RootCommandConfig{ + Name: "func", + Version: cmd.Version{ + Date: date, + Vers: vers, + Hash: hash, + }} - if err != nil { + if err := cmd.NewRootCmd(cfg).ExecuteContext(ctx); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) - } - - if err := root.ExecuteContext(ctx); err != nil { if ctx.Err() != nil { os.Exit(130) - return } - // Errors are printed to STDERR output and the process exits with code of 1. - fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } } diff --git a/cmd/info.go b/cmd/info.go index 6b4441da2..fdae7b0ed 100644 --- a/cmd/info.go +++ b/cmd/info.go @@ -32,10 +32,9 @@ the current directory or from the directory specified with --path. `, SuggestFor: []string{"ifno", "describe", "fino", "get"}, ValidArgsFunction: CompleteFunctionList, - PreRunE: bindEnv("namespace", "output", "path"), + PreRunE: bindEnv("output", "path"), } - setNamespaceFlag(cmd) cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT)") setPathFlag(cmd) @@ -66,10 +65,8 @@ func runInfo(cmd *cobra.Command, args []string, newClient ClientFactory) (err er } // Create a client - client := newClient(ClientOptions{ - Namespace: config.Namespace, - Verbose: config.Verbose, - }) + client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose}) + defer done() // Get the description d, err := client.Info(cmd.Context(), config.Name, config.Path) diff --git a/cmd/invoke.go b/cmd/invoke.go index a1128985d..86c9b5959 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -99,7 +99,7 @@ EXAMPLES `, SuggestFor: []string{"emit", "emti", "send", "emit", "exec", "nivoke", "onvoke", "unvoke", "knvoke", "imvoke", "ihvoke", "ibvoke"}, - PreRunE: bindEnv("path", "format", "target", "id", "source", "type", "data", "content-type", "file", "confirm", "namespace"), + PreRunE: bindEnv("path", "format", "target", "id", "source", "type", "data", "content-type", "file", "confirm"), } // Flags @@ -113,11 +113,9 @@ EXAMPLES cmd.Flags().StringP("data", "", fn.DefaultInvokeData, "Data to send in the request. (Env: $FUNC_DATA)") cmd.Flags().StringP("file", "", "", "Path to a file to use as data. Overrides --data flag and should be sent with a correct --content-type. (Env: $FUNC_FILE)") cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively. (Env: $FUNC_CONFIRM)") - setNamespaceFlag(cmd) cmd.SetHelpFunc(defaultTemplatedHelp) - // Run Action cmd.RunE = func(cmd *cobra.Command, args []string) error { return runInvoke(cmd, args, newClient) } @@ -134,10 +132,8 @@ func runInvoke(cmd *cobra.Command, args []string, newClient ClientFactory) (err } // Client instance from env vars, flags, args and user prompts (if --confirm) - client := newClient(ClientOptions{ - Namespace: cfg.Namespace, - Verbose: cfg.Verbose, - }) + client, done := newClient(ClientConfig{Namespace: cfg.Namespace, Verbose: cfg.Verbose}) + defer done() // Message to send the running Function built from parameters gathered // from the user (or defaults) @@ -196,6 +192,7 @@ func newInvokeConfig(newClient ClientFactory) (cfg invokeConfig, err error) { File: viper.GetString("file"), Confirm: viper.GetBool("confirm"), Verbose: viper.GetBool("verbose"), + Namespace: viper.GetString("namespace"), } // If file was passed, read it in as data @@ -213,10 +210,8 @@ func newInvokeConfig(newClient ClientFactory) (cfg invokeConfig, err error) { } // Client instance for use during prompting. - client := newClient(ClientOptions{ - Namespace: cfg.Namespace, - Verbose: cfg.Verbose, - }) + client, done := newClient(ClientConfig{Namespace: cfg.Namespace, Verbose: cfg.Verbose}) + defer done() // If in interactive terminal mode, prompt to modify defaults. if interactiveTerminal() { diff --git a/cmd/list.go b/cmd/list.go index 90f11d195..0259d564f 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -35,12 +35,11 @@ Lists all deployed functions in a given namespace. {{.Name}} list --all-namespaces --output json `, SuggestFor: []string{"ls", "lsit"}, - PreRunE: bindEnv("namespace", "output"), + PreRunE: bindEnv("all-namespaces", "output"), } cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.") cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)") - setNamespaceFlag(cmd) if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil { fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) @@ -62,10 +61,8 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error return err } - client := newClient(ClientOptions{ - Namespace: config.Namespace, - Verbose: config.Verbose, - }) + client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose}) + defer done() items, err := client.List(cmd.Context()) if err != nil { diff --git a/cmd/repository.go b/cmd/repository.go index d6f04f05e..b7d2ea95c 100644 --- a/cmd/repository.go +++ b/cmd/repository.go @@ -12,29 +12,10 @@ import ( fn "knative.dev/kn-plugin-func" ) -// repositoryClientFn is a function which yields both a client and the final -// config used to instantiate. -type repositoryClientFn func([]string) (repositoryConfig, RepositoryClient, error) - -// newRepositoryClient is the default repositoryClientFn. -// It creates a config (which parses flags and environment variables) and uses -// the config to intantiate a client. This function is swapped out in tests -// with one which returns a mock client. -func newRepositoryClient(args []string) (repositoryConfig, RepositoryClient, error) { - cfg, err := newRepositoryConfig(args) - if err != nil { - return cfg, nil, err - } - client := repositoryClient{fn.New( - fn.WithRepositoriesPath(cfg.RepositoriesPath), - fn.WithVerbose(cfg.Verbose))} - return cfg, client, nil -} - // command constructors // -------------------- -func NewRepositoryCmd(clientFn repositoryClientFn) *cobra.Command { +func NewRepositoryCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Short: "Manage installed template repositories", Use: "repository", @@ -164,30 +145,31 @@ EXAMPLES cmd.SetHelpFunc(defaultTemplatedHelp) cmd.RunE = func(cmd *cobra.Command, args []string) error { - return runRepository(cmd, args, clientFn) + return runRepository(cmd, args, newClient) } - cmd.AddCommand(NewRepositoryListCmd(newRepositoryClient)) - cmd.AddCommand(NewRepositoryAddCmd(newRepositoryClient)) - cmd.AddCommand(NewRepositoryRenameCmd(newRepositoryClient)) - cmd.AddCommand(NewRepositoryRemoveCmd(newRepositoryClient)) + cmd.AddCommand(NewRepositoryListCmd(newClient)) + cmd.AddCommand(NewRepositoryAddCmd(newClient)) + cmd.AddCommand(NewRepositoryRenameCmd(newClient)) + cmd.AddCommand(NewRepositoryRemoveCmd(newClient)) return cmd } -func NewRepositoryListCmd(clientFn repositoryClientFn) *cobra.Command { +func NewRepositoryListCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Short: "List repositories", Use: "list", } cmd.RunE = func(_ *cobra.Command, args []string) error { - return runRepositoryList(args, clientFn) + return runRepositoryList(cmd, args, newClient) } + return cmd } -func NewRepositoryAddCmd(clientFn repositoryClientFn) *cobra.Command { +func NewRepositoryAddCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Short: "Add a repository", Use: "add ", @@ -197,13 +179,14 @@ func NewRepositoryAddCmd(clientFn repositoryClientFn) *cobra.Command { cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)") - cmd.RunE = func(_ *cobra.Command, args []string) error { - return runRepositoryAdd(args, clientFn) + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runRepositoryAdd(cmd, args, newClient) } + return cmd } -func NewRepositoryRenameCmd(clientFn repositoryClientFn) *cobra.Command { +func NewRepositoryRenameCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Short: "Rename a repository", Use: "rename ", @@ -212,13 +195,14 @@ func NewRepositoryRenameCmd(clientFn repositoryClientFn) *cobra.Command { cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)") - cmd.RunE = func(_ *cobra.Command, args []string) error { - return runRepositoryRename(args, clientFn) + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runRepositoryRename(cmd, args, newClient) } + return cmd } -func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command { +func NewRepositoryRemoveCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Short: "Remove a repository", Use: "remove ", @@ -229,9 +213,10 @@ func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command { cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)") - cmd.RunE = func(_ *cobra.Command, args []string) error { - return runRepositoryRemove(args, clientFn) + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runRepositoryRemove(cmd, args, newClient) } + return cmd } @@ -240,10 +225,8 @@ func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command { // Run // (list by default or interactive with -c|--confirm) -func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientFn) (err error) { - // Config - // Based on env, flags and (optionally) user pompting. - cfg, client, err := clientFn(args) +func runRepository(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { + cfg, err := newRepositoryConfig(args) if err != nil { return } @@ -253,6 +236,7 @@ func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientF return cmd.Help() } + // If in interactive mode, the user chan choose which subcommand to invoke // Prompt for action to perform question := &survey.Question{ Name: "Action", @@ -266,34 +250,31 @@ func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientF return } - // Passthrough client constructor - // We already instantiated a client and a config using clientFn above - // (possibly prompting etc.), so the clientFn used for the subcommand - // delegation can be effectively a closure around those values so the - // user is not re-prompted: - c := func([]string) (repositoryConfig, RepositoryClient, error) { return cfg, client, nil } - // Run the command indicated switch answer.Action { case "list": - return runRepositoryList(args, c) + return runRepositoryList(cmd, args, newClient) case "add": - return runRepositoryAdd(args, c) + return runRepositoryAdd(cmd, args, newClient) case "rename": - return runRepositoryRename(args, c) + return runRepositoryRename(cmd, args, newClient) case "remove": - return runRepositoryRemove(args, c) + return runRepositoryRemove(cmd, args, newClient) } return fmt.Errorf("invalid action '%v'", answer.Action) // Unreachable } // List -func runRepositoryList(args []string, clientFn repositoryClientFn) (err error) { - cfg, client, err := clientFn(args) +func runRepositoryList(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { + cfg, err := newRepositoryConfig(args) if err != nil { return } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepositoriesPath(cfg.RepositoriesPath)) + defer done() + // List all repositories given a client instantiated about config. rr, err := client.Repositories().All() if err != nil { @@ -313,19 +294,21 @@ func runRepositoryList(args []string, clientFn repositoryClientFn) (err error) { } // Add -func runRepositoryAdd(args []string, clientFn repositoryClientFn) (err error) { +func runRepositoryAdd(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { // Supports both composable, discrete CLI commands or prompt-based "config" // by setting the argument values (name and ulr) to value of positional args, // but only requires them if not prompting. If prompting, those values // become the prompt defaults. - // Client - // (repositories location, verbosity, confirm) - cfg, client, err := clientFn(args) + cfg, err := newRepositoryConfig(args) if err != nil { return } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepositoriesPath(cfg.RepositoriesPath)) + defer done() + // Preconditions // If not confirming/prompting, assert the args were both provided. if len(args) != 2 && !cfg.Confirm { @@ -398,11 +381,14 @@ func runRepositoryAdd(args []string, clientFn repositoryClientFn) (err error) { } // Rename -func runRepositoryRename(args []string, clientFn repositoryClientFn) (err error) { - cfg, client, err := clientFn(args) +func runRepositoryRename(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { + cfg, err := newRepositoryConfig(args) if err != nil { return } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepositoriesPath(cfg.RepositoriesPath)) + defer done() // Preconditions if len(args) != 2 && !cfg.Confirm { @@ -466,11 +452,14 @@ func runRepositoryRename(args []string, clientFn repositoryClientFn) (err error) } // Remove -func runRepositoryRemove(args []string, clientFn repositoryClientFn) (err error) { - cfg, client, err := clientFn(args) +func runRepositoryRemove(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { + cfg, err := newRepositoryConfig(args) if err != nil { return } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, + fn.WithRepositoriesPath(cfg.RepositoriesPath)) + defer done() // Preconditions if len(args) != 1 && !cfg.Confirm { @@ -558,7 +547,7 @@ func runRepositoryRemove(args []string, clientFn repositoryClientFn) (err error) // Installed repositories // All repositories which have been installed (does not include builtin) -func installedRepositories(client RepositoryClient) ([]string, error) { +func installedRepositories(client *fn.Client) ([]string, error) { // Client API contract stipulates the list always lists the defeault builtin // repo, and always lists it at index 0 repositories, err := client.Repositories().List() @@ -652,34 +641,3 @@ func (c repositoryConfig) prompt() (repositoryConfig, error) { err := survey.Ask(qs, &c) return c, err } - -// Type Gymnastics -// --------------- - -// RepositoryClient enumerates the API required of a functions.Client to -// implement the various repository commands. -type RepositoryClient interface { - Repositories() Repositories -} - -// Repositories enumerates the API required of the object returned from -// client.Repositories. -type Repositories interface { - All() ([]fn.Repository, error) - List() ([]string, error) - Add(name, url string) (string, error) - Rename(old, new string) error - Remove(name string) error -} - -// repositoryClient implements RepositoryClient by embedding -// a functions.Client and overriding the Repositories accessor -// to return an interaface type. This is because an instance -// of functions.Client can not be directly treated as a RepositoryClient due -// to the return value of Repositories being a concrete type. -// This is hopefully not The Way. -type repositoryClient struct{ *fn.Client } - -func (c repositoryClient) Repositories() Repositories { - return Repositories(c.Client.Repositories()) -} diff --git a/cmd/repository_test.go b/cmd/repository_test.go index 5c42a2a23..69e2c445f 100644 --- a/cmd/repository_test.go +++ b/cmd/repository_test.go @@ -6,32 +6,23 @@ import ( "strings" "testing" - "knative.dev/kn-plugin-func/mock" + . "knative.dev/kn-plugin-func/testing" ) // TestRepository_List ensures that the 'list' subcommand shows the client's -// set of repositories by name and prints the list as expected. +// set of repositories by name for builtin repositories, by explicitly +// setting the repositories path to a new path which includes no others. func TestRepository_List(t *testing.T) { - var ( - client = mock.NewClient() - list = NewRepositoryListCmd(testRepositoryClientFn(client)) - ) - - // Set the repositories path, which will be passed to the client instance - // in the form of a config. - os.Setenv("FUNC_REPOSITORIES_PATH", "testpath") + defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())() + cmd := NewRepositoryListCmd(NewClient) + cmd.SetArgs([]string{}) // Do not use test command args // Execute the command, capturing the output sent to stdout stdout := piped(t) - if err := list.Execute(); err != nil { + if err := cmd.Execute(); err != nil { t.Fatal(err) } - // Assert the repository flag setting was preserved during execution - if client.RepositoriesPath != "testpath" { - t.Fatal("repositories env not passed to client") - } - // Assert the output matches expectd (whitespace trimmed) expect := "default" output := stdout() @@ -44,18 +35,20 @@ func TestRepository_List(t *testing.T) { // arguments, respects the repositories path flag, and the expected name is echoed // upon subsequent 'list'. func TestRepository_Add(t *testing.T) { + defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())() var ( - client = mock.NewClient() - add = NewRepositoryAddCmd(testRepositoryClientFn(client)) - list = NewRepositoryListCmd(testRepositoryClientFn(client)) + add = NewRepositoryAddCmd(NewClient) + list = NewRepositoryListCmd(NewClient) stdout = piped(t) ) - os.Setenv("FUNC_REPOSITORIES_PATH", "testpath") + // Do not use test command args + add.SetArgs([]string{}) + list.SetArgs([]string{}) // add [flags] add.SetArgs([]string{ "newrepo", - "https://git.example.com/user/repo", + TestRepoURI("repository", t), }) // Parse flags and args, performing action @@ -63,11 +56,6 @@ func TestRepository_Add(t *testing.T) { t.Fatal(err) } - // Assert the repositories flag was parsed and provided to client - if client.RepositoriesPath != "testpath" { - t.Fatal("repositories path not passed to client") - } - // List post-add, capturing output from stdout if err := list.Execute(); err != nil { t.Fatal(err) @@ -85,17 +73,20 @@ func TestRepository_Add(t *testing.T) { // positional arguments, respects the repositories path flag, and the name is // reflected as having been reanamed upon subsequent 'list'. func TestRepository_Rename(t *testing.T) { + defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())() var ( - client = mock.NewClient() - add = NewRepositoryAddCmd(testRepositoryClientFn(client)) - rename = NewRepositoryRenameCmd(testRepositoryClientFn(client)) - list = NewRepositoryListCmd(testRepositoryClientFn(client)) + add = NewRepositoryAddCmd(NewClient) + rename = NewRepositoryRenameCmd(NewClient) + list = NewRepositoryListCmd(NewClient) stdout = piped(t) ) - os.Setenv("FUNC_REPOSITORIES_PATH", "testpath") + // Do not use test command args + add.SetArgs([]string{}) + rename.SetArgs([]string{}) + list.SetArgs([]string{}) // add a repo which will be renamed - add.SetArgs([]string{"newrepo", "https://git.example.com/user/repo"}) + add.SetArgs([]string{"newrepo", TestRepoURI("repository", t)}) if err := add.Execute(); err != nil { t.Fatal(err) } @@ -111,11 +102,6 @@ func TestRepository_Rename(t *testing.T) { t.Fatal(err) } - // Assert the repositories flag was parsed and provided to client - if client.RepositoriesPath != "testpath" { - t.Fatal("repositories path not passed to client") - } - // List post-rename, capturing output from stdout if err := list.Execute(); err != nil { t.Fatal(err) @@ -133,17 +119,20 @@ func TestRepository_Rename(t *testing.T) { // its argument, respects the repositorieis flag, and the entry is removed upon // subsequent 'list'. func TestRepository_Remove(t *testing.T) { + defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())() var ( - client = mock.NewClient() - add = NewRepositoryAddCmd(testRepositoryClientFn(client)) - remove = NewRepositoryRemoveCmd(testRepositoryClientFn(client)) - list = NewRepositoryListCmd(testRepositoryClientFn(client)) + add = NewRepositoryAddCmd(NewClient) + remove = NewRepositoryRemoveCmd(NewClient) + list = NewRepositoryListCmd(NewClient) stdout = piped(t) ) - os.Setenv("FUNC_REPOSITORIES_PATH", "testpath") + // Do not use test command args + add.SetArgs([]string{}) + remove.SetArgs([]string{}) + list.SetArgs([]string{}) // add a repo which will be removed - add.SetArgs([]string{"newrepo", "https://git.example.com/user/repo"}) + add.SetArgs([]string{"newrepo", TestRepoURI("repository", t)}) if err := add.Execute(); err != nil { t.Fatal(err) } @@ -158,11 +147,6 @@ func TestRepository_Remove(t *testing.T) { t.Fatal(err) } - // Assert the repositories flag was parsed and provided to client - if client.RepositoriesPath != "testpath" { - t.Fatal("repositories flag not passed to client") - } - // List post-remove, capturing output from stdout if err := list.Execute(); err != nil { t.Fatal(err) @@ -179,30 +163,6 @@ func TestRepository_Remove(t *testing.T) { // Helpers // ------- -// testClientFn returns a repositoryClientFn which always returns the provided -// mock client. The client may have various function implementations overridden -// so as to test particular cases, and a config object is created from the -// flags and environment variables in the same way as the actual commands, with -// the effective value recorded on the mock as members for test assertions. -func testRepositoryClientFn(client *mock.Client) repositoryClientFn { - c := testRepositoryClient{client} // type gymnastics - return func(args []string) (repositoryConfig, RepositoryClient, error) { - cfg, err := newRepositoryConfig(args) - client.Confirm = cfg.Confirm - client.RepositoriesPath = cfg.RepositoriesPath - if err != nil { - return cfg, c, err - } - return cfg, c, nil - } -} - -type testRepositoryClient struct{ *mock.Client } - -func (c testRepositoryClient) Repositories() Repositories { - return Repositories(c.Client.Repositories()) -} - // pipe the output of stdout to a buffer whose value is returned // from the returned function. Call pipe() to start piping output // to the buffer, call the returned function to access the data in diff --git a/cmd/root.go b/cmd/root.go index 14fa5254d..4a374d057 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -11,22 +11,19 @@ import ( "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/client/pkg/util" - fn "knative.dev/kn-plugin-func" ) type RootCommandConfig struct { - Name string // usually `func` or `kn func` - Date string - Version string - Hash string + Name string // usually `func` or `kn func` + Version NewClient ClientFactory } // NewRootCmd creates the root of the command tree defines the command name, description, globally // available flags, etc. It has no action of its own, such that running the // resultant binary with no arguments prints the help/usage text. -func NewRootCmd(config RootCommandConfig) (*cobra.Command, error) { +func NewRootCmd(config RootCommandConfig) *cobra.Command { cmd := &cobra.Command{ // Use must be set to exactly config.Name, as this field is overloaded to // be used in subcommand help text as the command with possible prefix: @@ -66,36 +63,29 @@ EXAMPLES // Flags // persistent flags are available to all subcommands implicitly - cmd.PersistentFlags().BoolP("verbose", "v", false, "print verbose logs ($FUNC_VERBOSE)") - err := viper.BindPFlag("verbose", cmd.PersistentFlags().Lookup("verbose")) - if err != nil { - return cmd, err + // Note they are bound immediately here as opposed to other subcommands + // because this root command is not actually executed during tests, and + // therefore PreRunE and other event-based listeners are not invoked. + cmd.PersistentFlags().BoolP("verbose", "v", false, "Print verbose logs ($FUNC_VERBOSE)") + if err := viper.BindPFlag("verbose", cmd.PersistentFlags().Lookup("verbose")); err != nil { + fmt.Fprintf(os.Stderr, "error binding flag: %v\n", err) + } + cmd.PersistentFlags().StringP("namespace", "n", "", "The namespace on the cluster used for remote commands. By default, the namespace func.yaml is used or the currently active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") + if err := viper.BindPFlag("namespace", cmd.PersistentFlags().Lookup("namespace")); err != nil { + fmt.Fprintf(os.Stderr, "error binding flag: %v\n", err) } // Version - // Gather the statically-set version values (populated durin build) into - // a version structure used by both --version flag and the `version` subcmd - // Overrides the --version template to match the output format from the - // version subcommand: nothing but the version. - version := Version{ - Date: config.Date, - Vers: config.Version, - Hash: config.Hash, - } - cmd.Version = version.String() + cmd.Version = config.Version.String() cmd.SetVersionTemplate(`{{printf "%s\n" .Version}}`) + // Client + // Use the provided ClientFactory or default to NewClient newClient := config.NewClient - if newClient == nil { - var cleanUp func() error - newClient, cleanUp = NewDefaultClientFactory() - cmd.PersistentPostRunE = func(cmd *cobra.Command, args []string) error { - return cleanUp() - } + newClient = NewClient } - cmd.AddCommand(NewVersionCmd(version)) cmd.AddCommand(NewCreateCmd(newClient)) cmd.AddCommand(NewConfigCmd()) cmd.AddCommand(NewBuildCmd(newClient)) @@ -104,18 +94,19 @@ EXAMPLES cmd.AddCommand(NewInfoCmd(newClient)) cmd.AddCommand(NewListCmd(newClient)) cmd.AddCommand(NewInvokeCmd(newClient)) - cmd.AddCommand(NewRepositoryCmd(newRepositoryClient)) - cmd.AddCommand(NewRunCmd(newRunClient)) + cmd.AddCommand(NewRepositoryCmd(newClient)) + cmd.AddCommand(NewRunCmd(newClient)) cmd.AddCommand(NewCompletionCmd()) + cmd.AddCommand(NewVersionCmd(config.Version)) // Help // Overridden to process the help text as a template and have // access to the provided Client instance. cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { - runRootHelp(cmd, args, version) + runRootHelp(cmd, args, config.Version) }) - return cmd, nil + return cmd // NOTE Default Action // No default action is provided triggering the default of displaying the help @@ -342,11 +333,6 @@ func setPathFlag(cmd *cobra.Command) { cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") } -// setNamespaceFlag ensures common text/wording when the --namespace flag is used -func setNamespaceFlag(cmd *cobra.Command) { - cmd.Flags().StringP("namespace", "n", "", "The namespace on the cluster. By default, the namespace in func.yaml is used or the currently active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") -} - type Version struct { // Date of compilation Date string diff --git a/cmd/root_test.go b/cmd/root_test.go index d3fc00531..198b372af 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -11,8 +11,58 @@ import ( "knative.dev/client/pkg/util" fn "knative.dev/kn-plugin-func" + . "knative.dev/kn-plugin-func/testing" ) +func TestRoot_PersistentFlags(t *testing.T) { + tests := []struct { + name string + args []string + skipNamespace bool + }{ + { + name: "provided as root flags", + args: []string{"--verbose", "--namespace=namespace", "list"}, + }, + { + name: "provided as sub-command flags", + args: []string{"list", "--verbose", "--namespace=namespace"}, + }, + { + name: "provided as sub-sub-command flags", + args: []string{"repositories", "list", "--verbose"}, + skipNamespace: true, // NOTE: no sub-sub commands yet use namespace, so it is not checked. + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer Fromtemp(t)() // from a temp dir, deferred cleanup + cmd := NewCreateCmd(NewClient) // Create a new Function + cmd.SetArgs([]string{"--language", "go"}) // providing language (the only required param) + if err := cmd.Execute(); err != nil { // fail on any errors + t.Fatal(err) + } + + // Assert the persistent variables were propagated to the Client constructor + // when the command is actually invoked. + cmd = NewRootCmd(RootCommandConfig{NewClient: func(cfg ClientConfig, _ ...fn.Option) (*fn.Client, func()) { + if cfg.Namespace != "namespace" && !tt.skipNamespace { + t.Fatal("namespace not propagated") + } + if cfg.Verbose != true { + t.Fatal("verbose not propagated") + } + return fn.New(), func() {} + }}) + cmd.SetArgs(tt.args) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + }) + } +} + func TestRoot_mergeEnvMaps(t *testing.T) { a := "A" @@ -121,31 +171,35 @@ func TestRoot_mergeEnvMaps(t *testing.T) { } } -func TestRoot_CMDParameterized(t *testing.T) { +// TestRoot_CommandNameParameterized confirmst that the command name, as +// printed in help text, is parameterized based on the constructor parameters +// of the root command. This allows, for example, to have help text correct +// when both embedded as a plugin or standalone. +func TestRoot_CommandNameParameterized(t *testing.T) { expectedSynopsis := "%v [-v|--verbose] [args]" tests := []string{ - "func", - "kn func", + "func", // standalone + "kn func", // kn plugin } - for _, test := range tests { + for _, testName := range tests { var ( - cfg = RootCommandConfig{Name: test} - cmd, _ = NewRootCmd(cfg) - out = strings.Builder{} + cmd = NewRootCmd(RootCommandConfig{Name: testName}) + out = strings.Builder{} ) + cmd.SetArgs([]string{}) // Do not use test command args cmd.SetOut(&out) if err := cmd.Help(); err != nil { t.Fatal(err) } - if cmd.Use != cfg.Name { - t.Fatalf("expected command Use '%v', got '%v'", cfg.Name, cmd.Use) + if cmd.Use != testName { + t.Fatalf("expected command Use '%v', got '%v'", testName, cmd.Use) } - if !strings.Contains(out.String(), fmt.Sprintf(expectedSynopsis, cfg.Name)) { - t.Logf("Testing '%v'\n", test) + if !strings.Contains(out.String(), fmt.Sprintf(expectedSynopsis, testName)) { + t.Logf("Testing '%v'\n", testName) t.Log(out.String()) - t.Fatalf("Help text does not include substituted name '%v'", cfg.Name) + t.Fatalf("Help text does not include substituted name '%v'", testName) } } } @@ -171,11 +225,6 @@ func TestVerbose(t *testing.T) { args: []string{"--verbose", "version"}, want: "v0.42.0-cafe-1970-01-01\n", }, - { - name: "version not as sub-command", - args: []string{"--version"}, - want: "v0.42.0\n", - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -183,20 +232,17 @@ func TestVerbose(t *testing.T) { var out bytes.Buffer - cmd, err := NewRootCmd(RootCommandConfig{ - Name: "func", - Date: "1970-01-01", - Version: "v0.42.0", - Hash: "cafe", - }) - if err != nil { - t.Fatal(err) - } + cmd := NewRootCmd(RootCommandConfig{ + Name: "func", + Version: Version{ + Date: "1970-01-01", + Vers: "v0.42.0", + Hash: "cafe", + }}) cmd.SetArgs(tt.args) cmd.SetOut(&out) - err = cmd.Execute() - if err != nil { + if err := cmd.Execute(); err != nil { t.Fatal(err) } @@ -205,5 +251,4 @@ func TestVerbose(t *testing.T) { } }) } - } diff --git a/cmd/run.go b/cmd/run.go index 43795a24c..ed7589c06 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -10,30 +10,9 @@ import ( "knative.dev/client/pkg/util" fn "knative.dev/kn-plugin-func" - "knative.dev/kn-plugin-func/buildpacks" - "knative.dev/kn-plugin-func/docker" - "knative.dev/kn-plugin-func/progress" ) -func newRunClient(cfg runConfig) *fn.Client { - bc := newBuildConfig() - runner := docker.NewRunner(cfg.Verbose) - - // builder fields - builder := buildpacks.NewBuilder(cfg.Verbose) - listener := progress.New(cfg.Verbose) - return fn.New( - fn.WithBuilder(builder), - fn.WithProgressListener(listener), - fn.WithRegistry(bc.Registry), - fn.WithRunner(runner), - fn.WithVerbose(cfg.Verbose)) -} - -type runClientFn func(runConfig) *fn.Client - -func NewRunCmd(clientFn runClientFn) *cobra.Command { - +func NewRunCmd(newClient ClientFactory) *cobra.Command { cmd := &cobra.Command{ Use: "run", Short: "Run the function locally", @@ -50,26 +29,27 @@ specified by --path flag. The function must already have been built with the 'bu {{.Name}} run `, SuggestFor: []string{"rnu"}, - PreRunE: bindEnv("build", "path"), + PreRunE: bindEnv("build", "path", "registry"), } cmd.Flags().StringArrayP("env", "e", []string{}, "Environment variable to set in the form NAME=VALUE. "+ "You may provide this flag multiple times for setting multiple environment variables. "+ "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") - setPathFlag(cmd) cmd.Flags().BoolP("build", "b", false, "Build the function only if the function has not been built before") + cmd.Flags().StringP("registry", "r", GetDefaultRegistry(), "Registry + namespace part of the image if building, ex 'quay.io/myuser' (Env: $FUNC_REGISTRY)") + setPathFlag(cmd) cmd.SetHelpFunc(defaultTemplatedHelp) cmd.RunE = func(cmd *cobra.Command, args []string) error { - return runRun(cmd, args, clientFn) + return runRun(cmd, args, newClient) } return cmd } -func runRun(cmd *cobra.Command, args []string, clientFn runClientFn) (err error) { +func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { config, err := newRunConfig(cmd) if err != nil { return @@ -95,8 +75,11 @@ func runRun(cmd *cobra.Command, args []string, clientFn runClientFn) (err error) return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) } - // Client for use running (and potentially building) - client := clientFn(config) + // Client for use running (and potentially building), using the config + // gathered plus any additional option overrieds (such as for providing + // mocks when testing for builder and runner) + client, done := newClient(ClientConfig{Verbose: config.Verbose}, fn.WithRegistry(config.Registry)) + defer done() // Build if not built and --build if config.Build && !function.Built() { @@ -141,6 +124,9 @@ type runConfig struct { // Perform build if function hasn't been built yet Build bool + + // Registry for the build tag if building + Registry string } func newRunConfig(cmd *cobra.Command) (c runConfig, err error) { @@ -153,6 +139,7 @@ func newRunConfig(cmd *cobra.Command) (c runConfig, err error) { Build: viper.GetBool("build"), Path: viper.GetString("path"), Verbose: viper.GetBool("verbose"), // defined on root + Registry: viper.GetString("registry"), EnvToUpdate: envToUpdate, EnvToRemove: envToRemove, }, nil diff --git a/cmd/run_test.go b/cmd/run_test.go index 7edd16fe2..5028dcd02 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -9,10 +9,10 @@ import ( "github.com/ory/viper" fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" + . "knative.dev/kn-plugin-func/testing" ) func TestRun_Run(t *testing.T) { - tests := []struct { name string // name of the test desc string // description of the test @@ -73,7 +73,7 @@ created: 2009-11-10 23:00:00`, for _, tt := range tests { // run as a sub-test t.Run(tt.name, func(t *testing.T) { - defer fromTempDir(t)() + defer Fromtemp(t)() runner := mock.NewRunner() if tt.runError != nil { @@ -88,13 +88,14 @@ created: 2009-11-10 23:00:00`, // using a command whose client will be populated with mock // builder and mock runner, each of which may be set to error if the // test has an error defined. - cmd := NewRunCmd(func(rc runConfig) *fn.Client { + cmd := NewRunCmd(NewClientFactory(func() *fn.Client { return fn.New( fn.WithRunner(runner), fn.WithBuilder(builder), fn.WithRegistry("ghcr.com/reg"), ) - }) + })) + cmd.SetArgs([]string{}) // Do not use test command args // set test case's build viper.SetDefault("build", tt.buildFlag) diff --git a/docker/pusher.go b/docker/pusher.go index 96ee3b6a3..076f0c47a 100644 --- a/docker/pusher.go +++ b/docker/pusher.go @@ -78,12 +78,18 @@ func WithPusherDockerClientFactory(dockerClientFactory PusherDockerClientFactory } } +func WithVerbose(verbose bool) Opt { + return func(pusher *Pusher) { + pusher.verbose = verbose + } +} + func EmptyCredentialsProvider(ctx context.Context, registry string) (Credentials, error) { return Credentials{}, nil } // NewPusher creates an instance of a docker-based image pusher. -func NewPusher(verbose bool, opts ...Opt) *Pusher { +func NewPusher(opts ...Opt) *Pusher { result := &Pusher{ credentialsProvider: EmptyCredentialsProvider, progressListener: &fn.NoopProgressListener{}, @@ -92,7 +98,6 @@ func NewPusher(verbose bool, opts ...Opt) *Pusher { c, _, err := NewClient(client.DefaultDockerHost) return c, err }, - verbose: verbose, } for _, opt := range opts { opt(result) diff --git a/docker/pusher_test.go b/docker/pusher_test.go index e2625a05b..60ac74b89 100644 --- a/docker/pusher_test.go +++ b/docker/pusher_test.go @@ -105,9 +105,10 @@ func TestDaemonPush(t *testing.T) { dockerClientFactory := func() (docker.PusherDockerClient, error) { return dockerClient, nil } - pusher := docker.NewPusher(false, + pusher := docker.NewPusher( docker.WithCredentialsProvider(testCredProvider), - docker.WithPusherDockerClientFactory(dockerClientFactory)) + docker.WithPusherDockerClientFactory(dockerClientFactory), + ) f := fn.Function{ Image: functionImageLocal, @@ -182,10 +183,11 @@ func TestNonDaemonPush(t *testing.T) { return dockerClient, nil } - pusher := docker.NewPusher(false, + pusher := docker.NewPusher( docker.WithTransport(transport), docker.WithCredentialsProvider(testCredProvider), - docker.WithPusherDockerClientFactory(dockerClientFactory)) + docker.WithPusherDockerClientFactory(dockerClientFactory), + ) f := fn.Function{ Image: functionImageRemote, diff --git a/function_labels_unit_test.go b/function_labels_unit_test.go index c89e4f212..965f0c957 100644 --- a/function_labels_unit_test.go +++ b/function_labels_unit_test.go @@ -4,8 +4,9 @@ package function import ( - "os" "testing" + + . "knative.dev/kn-plugin-func/testing" ) func Test_validateLabels(t *testing.T) { @@ -28,10 +29,10 @@ func Test_validateLabels(t *testing.T) { valueLocalEnvIncorrect2 := "{{ MY_ENV }}" valueLocalEnvIncorrect3 := "{{env:MY_ENV}}foo" - os.Setenv("BAD_EXAMPLE", ":invalid") + defer WithEnvVar(t, "BAD_EXAMPLE", ":invalid")() valueLocalEnvIncorrect4 := "{{env:BAD_EXAMPLE}}" - os.Setenv("GOOD_EXAMPLE", "valid") + defer WithEnvVar(t, "GOOD_EXAMPLE", "valid")() valueLocalEnv4 := "{{env:GOOD_EXAMPLE}}" tests := []struct { diff --git a/pipelines/tekton/pipeplines_provider.go b/pipelines/tekton/pipeplines_provider.go index 3530fca72..92af1cb93 100644 --- a/pipelines/tekton/pipeplines_provider.go +++ b/pipelines/tekton/pipeplines_provider.go @@ -53,8 +53,14 @@ func WithCredentialsProvider(credentialsProvider docker.CredentialsProvider) Opt } } -func NewPipelinesProvider(verbose bool, opts ...Opt) *PipelinesProvider { - pp := &PipelinesProvider{verbose: verbose} +func WithVerbose(verbose bool) Opt { + return func(pp *PipelinesProvider) { + pp.verbose = verbose + } +} + +func NewPipelinesProvider(opts ...Opt) *PipelinesProvider { + pp := &PipelinesProvider{} for _, opt := range opts { opt(pp) diff --git a/plugin/plugin.go b/plugin/plugin.go index 3fb439196..9efb02055 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -35,18 +35,17 @@ func (f *funcPlugin) Execute(args []string) error { cancel() }() - rootConfig := cmd.RootCommandConfig{ - Name: "kn func", - } + version := cmd.Version{} info, _ := debug.ReadBuildInfo() for _, dep := range info.Deps { if strings.Contains(dep.Path, "knative.dev/kn-plugin-func") { - rootConfig.Version, rootConfig.Hash = dep.Version, dep.Sum + version.Vers = dep.Version + version.Hash = dep.Sum } } - rootCmd, _ := cmd.NewRootCmd(rootConfig) + rootCmd := cmd.NewRootCmd(cmd.RootCommandConfig{Name: "kn func", Version: version}) oldArgs := os.Args defer (func() { diff --git a/testing/testing.go b/testing/testing.go index b6ed7a1a0..ea210cc72 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -91,9 +91,18 @@ func Mktemp(t *testing.T) (string, func()) { } } +// Fromtemp is like Mktemp, but does not bother returing the temp path. +func Fromtemp(t *testing.T) func() { + _, done := Mktemp(t) + return done +} + // tempdir creates a new temporary directory and returns its path. // errors fail the current test. func tempdir(t *testing.T) string { + // NOTE: Not using t.TempDir() because it is sometimes helpful during + // debugging to skip running the returned deferred cleanup function + // and manually inspect the contents of the test's temp directory. d, err := ioutil.TempDir("", "dir") if err != nil { t.Fatal(err) @@ -139,6 +148,7 @@ func TestRepoURI(name string, t *testing.T) string { // WithEnvVar sets an environment variable // and returns deferrable function that restores previous value of the environment variable. +// TODO: replace with t.Setenv when we upgrade to go.1.17 func WithEnvVar(t *testing.T, name, value string) func() { t.Helper() oldDh, hadDh := os.LookupEnv(name)