diff --git a/riff-cli/cmd/create_test.go b/riff-cli/cmd/create_test.go index be50854a8..6ed6ee30a 100644 --- a/riff-cli/cmd/create_test.go +++ b/riff-cli/cmd/create_test.go @@ -17,6 +17,7 @@ package cmd import ( + "fmt" "os" "path/filepath" @@ -82,6 +83,7 @@ var _ = Describe("The create command", func() { err = rootCommand.Execute() Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(fmt.Errorf("Invokers must be installed, run `riff invokers apply --help` for help"))) Expect(".").NotTo(HaveUnstagedChanges()) }) @@ -112,19 +114,9 @@ var _ = Describe("The create command", func() { rootCommand.SetArgs(append([]string{"create"}, commonRiffArgs...)) - normalDocker.On("Exec", "build", "-t", "rifftest/matching-invoker:0.0.1", "."). - Return(nil). - Once() - functionYamlPath, _ := filepath.Abs("matching-invoker-function.yaml") - topicsYamlPath, _ := filepath.Abs("matching-invoker-topics.yaml") - normalKubeCtl.On("Exec", []string{"apply", "-f", functionYamlPath, "-f", topicsYamlPath}). - Return("function \"matching-invoker\" created\ntopic \"matching-invoker\" created", nil). - Once() - err = rootCommand.Execute() - Expect(err).NotTo(HaveOccurred()) - - Expect(".").NotTo(HaveUnstagedChanges()) + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(fmt.Errorf("The invoker must be specified. Pick one of: node"))) }) It("creates a function with an explicit invoker", func() { @@ -160,7 +152,7 @@ var _ = Describe("The create command", func() { rootCommand, initOptions, _, _, err := setupCreateTest(invokers, normalDocker, dryRunDocker, normalKubeCtl, dryRunKubeCtl) Expect(err).NotTo(HaveOccurred()) - rootCommand.SetArgs(append([]string{"create", "--dry-run", "../test_data/command/echo", "-a", "echo.sh", "-v", "0.0.1-snapshot"}, commonRiffArgs...)) + rootCommand.SetArgs(append([]string{"create", "command", "--dry-run", "../test_data/command/echo", "-a", "echo.sh", "-v", "0.0.1-snapshot"}, commonRiffArgs...)) dryRunDocker.On("Exec", "build", "-t", "rifftest/echo:0.0.1-snapshot", "../test_data/command/echo"). Return(nil). @@ -186,7 +178,7 @@ var _ = Describe("The create command", func() { rootCommand, _, _, _, err := setupCreateTest(invokers, normalDocker, dryRunDocker, normalKubeCtl, dryRunKubeCtl) Expect(err).NotTo(HaveOccurred()) - rootCommand.SetArgs(append([]string{"create", "--dry-run", "-a", "echo.sh"}, commonRiffArgs...)) + rootCommand.SetArgs(append([]string{"create", "command", "--dry-run", "-a", "echo.sh"}, commonRiffArgs...)) dryRunDocker.On("Exec", "build", "-t", "rifftest/echo:0.0.1", "."). Return(nil). @@ -208,7 +200,7 @@ var _ = Describe("The create command", func() { path, _ := filepath.Abs("../test_data/command/echo") - rootCommand.SetArgs(append([]string{"create", "--dry-run", "-f", path, "-v", "0.0.1-snapshot", "-a", "echo.sh"}, commonRiffArgs...)) + rootCommand.SetArgs(append([]string{"create", "command", "--dry-run", "-f", path, "-v", "0.0.1-snapshot", "-a", "echo.sh"}, commonRiffArgs...)) dryRunDocker.On("Exec", "build", "-t", "rifftest/echo:0.0.1-snapshot", path). Return(nil). @@ -232,7 +224,7 @@ var _ = Describe("The create command", func() { rootCommand, initOptions, _, _, err := setupCreateTest(invokers, normalDocker, dryRunDocker, normalKubeCtl, dryRunKubeCtl) Expect(err).NotTo(HaveOccurred()) - rootCommand.SetArgs([]string{"create", "--dry-run", "../test_data/command/echo", "-u", "me", "-a", "echo.sh"}) + rootCommand.SetArgs([]string{"create", "command", "--dry-run", "../test_data/command/echo", "-u", "me", "-a", "echo.sh"}) dryRunDocker.On("Exec", "build", "-t", "me/echo:0.0.1", "../test_data/command/echo"). Return(nil). diff --git a/riff-cli/cmd/init.go b/riff-cli/cmd/init.go index 13cce33bf..15a5f72e9 100644 --- a/riff-cli/cmd/init.go +++ b/riff-cli/cmd/init.go @@ -18,6 +18,7 @@ package cmd import ( "fmt" + "strings" "path/filepath" @@ -36,14 +37,14 @@ func Init(invokers []projectriff_v1.Invoker) (*cobra.Command, *options.InitOptio Use: "init", Short: "Initialize a function", Long: utils.InitCmdLong(), - Args: utils.AliasFlagToSoleArg("filepath"), RunE: func(cmd *cobra.Command, args []string) error { - err := initializer.Initialize(invokers, &initOptions) - if err != nil { - cmd.SilenceUsage = true + cmd.SilenceUsage = true + if len(invokers) == 0 { + return fmt.Errorf("Invokers must be installed, run `riff invokers apply --help` for help") } - return err + names := invokerNames(invokers) + return fmt.Errorf("The invoker must be specified. Pick one of: %s", strings.Join(names, ", ")) }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { initOptions.UserAccount = utils.GetUseraccountWithOverride("useraccount", *cmd.Flags()) @@ -82,8 +83,11 @@ func InitInvokers(invokers []projectriff_v1.Invoker, initOptions *options.InitOp Short: fmt.Sprintf("Initialize a %s function", invokerName), Long: utils.InitInvokerCmdLong(invoker), RunE: func(cmd *cobra.Command, args []string) error { - initOptions.InvokerName = invokerName - return initializer.Initialize(invokers, initOptions) + invoker, err := invokerForName(invokerName, invokers) + if err != nil { + return err + } + return initializer.Initialize(invoker, initOptions) }, } @@ -116,3 +120,19 @@ func validateInitOptions(options *options.InitOptions) error { return nil } +func invokerForName(name string, invokers []projectriff_v1.Invoker) (projectriff_v1.Invoker, error) { + for _, invoker := range invokers { + if invoker.ObjectMeta.Name == name { + return invoker, nil + } + } + return projectriff_v1.Invoker{}, fmt.Errorf("No invoker found for %s", name) +} + +func invokerNames(invokers []projectriff_v1.Invoker) []string { + names := []string{} + for _, invoker := range invokers { + names = append(names, invoker.ObjectMeta.Name) + } + return names +} diff --git a/riff-cli/cmd/init_test.go b/riff-cli/cmd/init_test.go index b28baddbc..3d5bbcf1a 100644 --- a/riff-cli/cmd/init_test.go +++ b/riff-cli/cmd/init_test.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -49,11 +50,12 @@ var _ = Describe("The init command", func() { err = rootCommand.Execute() Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(fmt.Errorf("Invokers must be installed, run `riff invokers apply --help` for help"))) Expect(".").NotTo(HaveUnstagedChanges()) }) - It("should fail if no matching invoker is defined", func() { + It("should fail if no invoker is specified", func() { os.Chdir("../test_data/riff-init/no-matching-invoker") invokers, err := stubInvokers("invokers/*.yaml") @@ -65,54 +67,7 @@ var _ = Describe("The init command", func() { err = rootCommand.Execute() Expect(err).To(HaveOccurred()) - - Expect(".").NotTo(HaveUnstagedChanges()) - }) - - It("should find an invoker based on an artifact", func() { - os.Chdir("../test_data/riff-init/matching-invoker") - - invokers, err := stubInvokers("invokers/*.yaml") - Expect(err).NotTo(HaveOccurred()) - rootCommand, _, _, _, err := setupInitTest(invokers) - Expect(err).NotTo(HaveOccurred()) - - rootCommand.SetArgs(append([]string{"init", "--artifact", "echo.js"}, commonRiffArgs...)) - - err = rootCommand.Execute() - Expect(err).NotTo(HaveOccurred()) - - Expect(".").NotTo(HaveUnstagedChanges()) - }) - - It("should find an artifact based on an invoker", func() { - os.Chdir("../test_data/riff-init/multiple-matching-invokers-with-one-selected-no-artifact") - - invokers, err := stubInvokers("invokers/*.yaml") - Expect(err).NotTo(HaveOccurred()) - rootCommand, _, _, _, err := setupInitTest(invokers) - Expect(err).NotTo(HaveOccurred()) - - rootCommand.SetArgs(append([]string{"init", "python3"}, commonRiffArgs...)) - - err = rootCommand.Execute() - Expect(err).NotTo(HaveOccurred()) - - Expect(".").NotTo(HaveUnstagedChanges()) - }) - - It("should fail when multiple invokers match", func() { - os.Chdir("../test_data/riff-init/multiple-matching-invokers") - - invokers, err := stubInvokers("invokers/*.yaml") - Expect(err).NotTo(HaveOccurred()) - rootCommand, _, _, _, err := setupInitTest(invokers) - Expect(err).NotTo(HaveOccurred()) - - rootCommand.SetArgs(append([]string{"init", "--artifact", "echo.py"}, commonRiffArgs...)) - - err = rootCommand.Execute() - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(fmt.Errorf("The invoker must be specified. Pick one of: node"))) Expect(".").NotTo(HaveUnstagedChanges()) }) diff --git a/riff-cli/pkg/initializer/initializer.go b/riff-cli/pkg/initializer/initializer.go index dd435d21c..5d1b6e833 100644 --- a/riff-cli/pkg/initializer/initializer.go +++ b/riff-cli/pkg/initializer/initializer.go @@ -31,13 +31,8 @@ import ( "github.com/projectriff/riff/riff-cli/pkg/templateutils" ) -func Initialize(invokers []projectriff_v1.Invoker, opts *options.InitOptions) error { - invoker, err := resolveInvoker(invokers, opts) - if err != nil { - return err - } - - err = resolveOptions(opts, invoker) +func Initialize(invoker projectriff_v1.Invoker, opts *options.InitOptions) error { + err := resolveOptions(opts, invoker) if err != nil { return err } @@ -91,108 +86,68 @@ func loadInvokersFromKubeCtl(kubeCtl kubectl.KubeCtl) ([]projectriff_v1.Invoker, return invokerList.Items, nil } -func resolveInvoker(invokers []projectriff_v1.Invoker, opts *options.InitOptions) (projectriff_v1.Invoker, error) { - var resolvedInvoker projectriff_v1.Invoker +type handlerOptions struct { + FunctionName string +} - // look for an exact invoker - if opts.InvokerName != "" { - for _, invoker := range invokers { - if opts.InvokerName == invoker.ObjectMeta.Name { - resolvedInvoker = invoker - } - } - if resolvedInvoker.ObjectMeta.Name == "" { - return projectriff_v1.Invoker{}, fmt.Errorf("Invoker %s not found", opts.InvokerName) - } +func (h handlerOptions) TitleCase(s string) string { + return strings.Title(s) +} - // restrict future searches to the resolved invoker - invokers = []projectriff_v1.Invoker{resolvedInvoker} +func resolveOptions(opts *options.InitOptions, invoker projectriff_v1.Invoker) error { + if opts.Input == "" { + opts.Input = opts.FunctionName } - if opts.Artifact == "" { - // look for a matching artifact - - // This will get slower as more invokers are introduced, more complex - // matching patterns are used and run in directory with more files. - // Considering the search is non-deterministic between calls if the - // invokers are updated, it may not be worth the effort. Forcing the - // user to specify the artifact will produce stable results + if opts.InvokerVersion == "" { + opts.InvokerVersion = invoker.Spec.Version + } + if opts.Artifact == "" { workdir, err := filepath.Abs(opts.FilePath) if err != nil { - return projectriff_v1.Invoker{}, err + return err } - artifacts, err := resolveArtifacts(workdir, invokers) + artifacts, err := resolveArtifacts(workdir, invoker) if err != nil { - return projectriff_v1.Invoker{}, err + return err } if len(artifacts) == 0 { - var registeredInvokers []string - for _, element := range invokers { - registeredInvokers = append(registeredInvokers, element.Name) - } - return projectriff_v1.Invoker{}, fmt.Errorf("No matching artifact found (using registered invokers: %v)", registeredInvokers) + return fmt.Errorf("No matching artifact found") } if len(artifacts) > 1 { - // TODO MAYBE attempt to find the "best" artifact - return projectriff_v1.Invoker{}, fmt.Errorf("Artifact must be specified") + // TODO attempt to find the "best" artifact + return fmt.Errorf("Artifact must be specified") } relativePath, err := filepath.Rel(workdir, artifacts[0]) if err != nil { - return projectriff_v1.Invoker{}, err + return err } opts.Artifact = relativePath } - if resolvedInvoker.ObjectMeta.Name != "" { - return resolvedInvoker, nil - } - - // look for a matching invoker - var matchingInvokers []projectriff_v1.Invoker - for _, invoker := range invokers { - matched := false - for _, matcher := range invoker.Spec.Matchers { - if matched { - continue - } - match, err := filepath.Match(matcher, opts.Artifact) - if err != nil { - return projectriff_v1.Invoker{}, err - } - if match { - matchingInvokers = append(matchingInvokers, invoker) - matched = true - } - } - } - if len(matchingInvokers) > 1 { - // TODO MAYBE attempt to find a clear "best" match - var names []string - for _, matchingInvoker := range matchingInvokers { - names = append(names, matchingInvoker.ObjectMeta.Name) + if opts.Handler != "" { + handler, err := templateutils.Apply(opts.Handler, "opts.Handler", handlerOptions{FunctionName: opts.FunctionName}) + if err != nil { + return err } - return projectriff_v1.Invoker{}, fmt.Errorf("Multiple matching invokers found, pick one of: %s: ", strings.Join(names, ", ")) - } - if len(matchingInvokers) == 0 { - return projectriff_v1.Invoker{}, fmt.Errorf("No invoker found matching %s", opts.Artifact) + opts.Handler = handler } - return matchingInvokers[0], nil + + return nil } -func resolveArtifacts(workdir string, invokers []projectriff_v1.Invoker) ([]string, error) { +func resolveArtifacts(workdir string, invoker projectriff_v1.Invoker) ([]string, error) { artifacts := make(map[string]bool) - for _, invoker := range invokers { - for _, matcher := range invoker.Spec.Matchers { - matches, err := filepath.Glob(filepath.Join(workdir, matcher)) - if err != nil { - return []string{}, nil - } - for _, match := range matches { - artifacts[match] = true - } + for _, matcher := range invoker.Spec.Matchers { + matches, err := filepath.Glob(filepath.Join(workdir, matcher)) + if err != nil { + return []string{}, nil + } + for _, match := range matches { + artifacts[match] = true } } keys := make([]string, 0, len(artifacts)) @@ -201,35 +156,3 @@ func resolveArtifacts(workdir string, invokers []projectriff_v1.Invoker) ([]stri } return keys, nil } - -type handlerOptions struct { - FunctionName string -} - -func (h handlerOptions) TitleCase(s string) string { - return strings.Title(s) -} - -func resolveOptions(opts *options.InitOptions, invoker projectriff_v1.Invoker) error { - if opts.Input == "" { - opts.Input = opts.FunctionName - } - - if opts.InvokerVersion == "" { - opts.InvokerVersion = invoker.Spec.Version - } - - // if opts.Artifact == "" { - // opts.Artifact = filepath.Base(functionArtifact) - // } - - if opts.Handler != "" { - handler, err := templateutils.Apply(opts.Handler, "opts.Handler", handlerOptions{FunctionName: opts.FunctionName}) - if err != nil { - return err - } - opts.Handler = handler - } - - return nil -}