From 834ce080614cadf04add3c27bb42756c48905112 Mon Sep 17 00:00:00 2001 From: Aleksandr Britvin Date: Sat, 10 Aug 2024 20:53:35 +0200 Subject: [PATCH 1/3] #63: Fix incorrect discovering and parsing when in root or home directory. Update linter. --- .golangci.yaml | 25 ++-- Makefile | 2 +- app.go | 176 +++++++++++++++-------------- example/actions/alias/action.yaml | 10 ++ example/actions/alias/main.sh | 4 + gen.go | 5 +- pkg/action/action.go | 29 +++-- pkg/action/action_test.go | 6 +- pkg/action/cobra.go | 18 +-- pkg/action/discover.go | 102 +++++++++++++---- pkg/action/discover.skip.go | 43 +++++++ pkg/action/discover.unix.go | 72 ++++++++++++ pkg/action/discover.windows.go | 58 ++++++++++ pkg/action/discover_test.go | 71 ++++++++---- pkg/action/discover_unix.go | 19 ---- pkg/action/discover_windows.go | 26 ----- pkg/action/env.container_test.go | 20 ++-- pkg/action/loader.go | 2 +- pkg/action/manager.go | 111 +++++++++++++----- pkg/action/yaml.def.go | 10 +- pkg/action/yaml_const_test.go | 3 + pkg/driver/tty.go | 2 +- pkg/log/logger.go | 17 ++- plugins/actionnaming/plugin.go | 73 ++++++++++++ plugins/builder/builder.go | 2 +- plugins/builder/plugin.go | 10 +- plugins/default.go | 1 + plugins/verbosity/plugin.go | 11 +- plugins/yamldiscovery/embed/tar.go | 3 +- plugins/yamldiscovery/plugin.go | 50 ++------ types.go | 2 + 31 files changed, 683 insertions(+), 300 deletions(-) create mode 100644 example/actions/alias/action.yaml create mode 100755 example/actions/alias/main.sh create mode 100644 pkg/action/discover.skip.go create mode 100644 pkg/action/discover.unix.go create mode 100644 pkg/action/discover.windows.go delete mode 100644 pkg/action/discover_unix.go delete mode 100644 pkg/action/discover_windows.go create mode 100644 plugins/actionnaming/plugin.go diff --git a/.golangci.yaml b/.golangci.yaml index 4dd22ca..98f1742 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -3,23 +3,16 @@ run: deadline: 10s issues-exit-code: 1 tests: true - skip-dirs: - - bin - - vendor - - var - - tmp - skip-files: - - \.pb\.go$ - - \.pb\.goclay\.go$ output: - format: colored-line-number + formats: + - format: colored-line-number print-issued-lines: true print-linter-name: true linters-settings: govet: - check-shadowing: true + shadow: true golint: min-confidence: 0 dupl: @@ -41,10 +34,20 @@ linters: - goconst - gosec - goimports - - megacheck # (staticcheck + gosimple + unused in one linter) - enable before push + - gosimple + - staticcheck + - unused issues: exclude-use-default: false + exclude-dirs: + - bin + - vendor + - var + - tmp + exclude-files: + - \.pb\.go$ + - \.pb\.goclay\.go$ exclude: # # _ instead of err checks # - G104 diff --git a/Makefile b/Makefile index 0bffa37..beed7ef 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ LOCAL_BIN:=$(CURDIR)/bin # Linter config. GOLANGCI_BIN:=$(LOCAL_BIN)/golangci-lint -GOLANGCI_TAG:=1.55.2 +GOLANGCI_TAG:=1.59.1 .PHONY: all all: deps test build diff --git a/app.go b/app.go index 8274231..09c40f4 100644 --- a/app.go +++ b/app.go @@ -1,6 +1,7 @@ package launchr import ( + "context" "embed" "errors" "fmt" @@ -10,6 +11,7 @@ import ( "reflect" "sort" "strings" + "time" "github.com/spf13/cobra" @@ -21,28 +23,22 @@ import ( var ( errTplAssetsNotFound = "assets not found for requested plugin %s" + errDiscoveryTimeout = "action discovery timeout exceeded" ) -// ActionsGroup is a cobra command group definition +// ActionsGroup is a cobra command group definition. var ActionsGroup = &cobra.Group{ ID: "actions", Title: "Actions:", } -type launchrCfg struct { - ActionsNaming []struct { - Search string `yaml:"search"` - Replace string `yaml:"replace"` - } `yaml:"actions_naming"` -} - type appImpl struct { - rootCmd *cobra.Command + cmd *cobra.Command streams cli.Streams workDir string cfgDir string services map[ServiceInfo]Service - actionMngr action.Manager + actionMngr action.ManagerUnsafe pluginMngr PluginManager config Config mFS []ManagedFS @@ -148,27 +144,68 @@ func (app *appImpl) GetPluginAssets(p Plugin) fs.FS { return subFS } +// earlyPeekFlags tries to parse flags early to allow change behavior before cobra has booted. +func (app *appImpl) earlyPeekFlags(c *cobra.Command) { + args := os.Args[1:] + // Parse args with cobra. + // We can't guess cmd because nothing has been defined yet. + // We don't care about error because there won't be any on clean cmd. + _, flags, _ := c.Find(args) + // Quick parse arguments to see if a version or help was requested. + for i := 0; i < len(flags); i++ { + // Skip discover actions if we check version. + if flags[i] == "--version" { + app.skipActions = true + } + + if app.reqCmd == "" && !strings.HasPrefix(flags[i], "-") { + app.reqCmd = args[i] + } + } +} + // init initializes application and plugins. func (app *appImpl) init() error { var err error + // Set root cobra command. + app.cmd = &cobra.Command{ + Use: name, + //Short: "", // @todo + //Long: ``, // @todo + SilenceErrors: true, // Handled manually. + Version: version, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + } + app.earlyPeekFlags(app.cmd) + + // Set io streams. + app.streams = cli.StandardStreams() + app.cmd.SetIn(app.streams.In()) + app.cmd.SetOut(app.streams.Out()) + app.cmd.SetErr(app.streams.Err()) + // Set working dir and config dir. app.cfgDir = "." + name app.workDir, err = filepath.Abs(".") if err != nil { return err } + // Initialize managed FS for action discovery. app.mFS = make([]ManagedFS, 0, 4) app.RegisterFS(action.NewDiscoveryFS(os.DirFS(app.workDir), app.GetWD())) + // Prepare dependencies. - app.streams = cli.StandardStreams() app.services = make(map[ServiceInfo]Service) app.pluginMngr = launchr.NewPluginManagerWithRegistered() + // @todo consider home dir for global config. app.config = launchr.ConfigFromFS(os.DirFS(app.cfgDir)) app.actionMngr = action.NewManager( action.WithDefaultRunEnvironment, action.WithContainerRunEnvironmentConfig(app.config, name+"_"), action.WithValueProcessors(), - ) + ).(action.ManagerUnsafe) // Ensure we can use unsafe functionality. // Register services for other modules. app.AddService(app.actionMngr) @@ -182,79 +219,63 @@ func (app *appImpl) init() error { } } - // Quick parse arguments to see if a version or help was requested. - args := os.Args[1:] - for i := 0; i < len(args); i++ { - // Skip discover actions if we check version. - if args[i] == "--version" { - app.skipActions = true - } - - if app.reqCmd == "" && !strings.HasPrefix(args[i], "-") { - app.reqCmd = args[i] - } - } - // Discover actions. if !app.skipActions { - var launchrConfig *launchrCfg - err = app.config.Get("launchrctl", &launchrConfig) - if err != nil { + if err = app.discoverActions(); err != nil { return err } + } + + return nil +} - for _, p := range getPluginByType[ActionDiscoveryPlugin](app) { - for _, fs := range app.GetRegisteredFS() { - actions, err := p.DiscoverActions(fs) - if err != nil { - return err - } - for _, actConf := range actions { - if err = actConf.EnsureLoaded(); err != nil { - return err - } - - if launchrConfig != nil && len(launchrConfig.ActionsNaming) > 0 { - actID := actConf.ID - for _, an := range launchrConfig.ActionsNaming { - actID = strings.ReplaceAll(actID, an.Search, an.Replace) - } - actConf.ID = actID - } - - app.actionMngr.Add(actConf) - } +func (app *appImpl) discoverActions() (err error) { + var discovered []*action.Action + idp := app.actionMngr.GetActionIDProvider() + // @todo configure from flags + // Define timeout for cases when we may traverse the whole FS, e.g. in / or home. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + for _, p := range getPluginByType[action.DiscoveryPlugin](app) { + for _, fs := range app.GetRegisteredFS() { + actions, errDis := p.DiscoverActions(ctx, fs, idp) + if errDis != nil { + return errDis } + discovered = append(discovered, actions...) } } + // Failed to discover actions in reasonable time. + if errCtx := ctx.Err(); errCtx != nil { + return errors.New(errDiscoveryTimeout) + } - return nil -} + // Add discovered actions. + for _, a := range discovered { + app.actionMngr.Add(a) + } -func (app *appImpl) exec() error { - // Set root cobra command. - var rootCmd = &cobra.Command{ - Use: name, - //Short: "", // @todo - //Long: ``, // @todo - SilenceErrors: true, // Handled manually. - Version: version, - RunE: func(cmd *cobra.Command, args []string) error { - return cmd.Help() - }, + // Alter all registered actions. + for _, p := range getPluginByType[action.AlterActionsPlugin](app) { + err = p.AlterActions() + if err != nil { + return err + } } + // @todo maybe cache discovery result for performance. + return err +} +func (app *appImpl) exec() error { if app.skipActions { - rootCmd.SetVersionTemplate(Version().Full()) + app.cmd.SetVersionTemplate(Version().Full()) } // Convert actions to cobra commands. - actions := app.actionMngr.AllRef() + actions := app.actionMngr.AllUnsafe() // Check the requested command to see what actions we must actually load. if app.reqCmd != "" { - aliases := app.actionMngr.AllAliasRef() - if alias, ok := aliases[app.reqCmd]; ok { - app.reqCmd = alias - } + // Check if an alias was provided to find the real action. + app.reqCmd = app.actionMngr.GetIDFromAlias(app.reqCmd) a, ok := actions[app.reqCmd] if ok { // Use only the requested action. @@ -267,37 +288,28 @@ func (app *appImpl) exec() error { // @todo consider cobra completion and caching between runs. if !app.skipActions { if len(actions) > 0 { - rootCmd.AddGroup(ActionsGroup) + app.cmd.AddGroup(ActionsGroup) } for _, a := range actions { - a = app.actionMngr.Decorate(a) - if err := a.EnsureLoaded(); err != nil { - fmt.Fprintf(os.Stdout, "[WARNING] Action %q was skipped because it has an incorrect definition:\n%v\n", a.ID, err) - continue - } + a, _ = app.actionMngr.Get(a.ID) cmd, err := action.CobraImpl(a, app.Streams()) if err != nil { fmt.Fprintf(os.Stdout, "[WARNING] Action %q was skipped:\n%v\n", a.ID, err) continue } cmd.GroupID = ActionsGroup.ID - rootCmd.AddCommand(cmd) + app.cmd.AddCommand(cmd) } } // Add cobra commands from plugins. for _, p := range getPluginByType[CobraPlugin](app) { - if err := p.CobraAddCommands(rootCmd); err != nil { + if err := p.CobraAddCommands(app.cmd); err != nil { return err } } - // Set io streams. - app.rootCmd = rootCmd - rootCmd.SetIn(app.streams.In()) - rootCmd.SetOut(app.streams.Out()) - rootCmd.SetErr(app.streams.Err()) - return app.rootCmd.Execute() + return app.cmd.Execute() } // Execute is a cobra entrypoint to the launchr app. diff --git a/example/actions/alias/action.yaml b/example/actions/alias/action.yaml new file mode 100644 index 0000000..a373bb1 --- /dev/null +++ b/example/actions/alias/action.yaml @@ -0,0 +1,10 @@ +action: + title: aliasaction + description: Test alias definition + image: buildargs:latest + alias: + - "alias1" + - "alias2" + command: + - sh + - /action/main.sh diff --git a/example/actions/alias/main.sh b/example/actions/alias/main.sh new file mode 100755 index 0000000..3800648 --- /dev/null +++ b/example/actions/alias/main.sh @@ -0,0 +1,4 @@ +#!/bin/sh +set -ex +whoami +id diff --git a/gen.go b/gen.go index 5326542..83d7dcc 100644 --- a/gen.go +++ b/gen.go @@ -21,6 +21,9 @@ func (app *appImpl) gen(buildPath string, wordDir string) error { } // Clean build path before generating. err = filepath.WalkDir(buildPath, func(path string, dir fs.DirEntry, err error) error { + if err != nil { + return err + } if path == buildPath { return nil } @@ -54,7 +57,7 @@ func (app *appImpl) gen(buildPath string, wordDir string) error { } } if len(initSet) > 0 { - var tplName = "init.gen" + tplName := "init.gen" tpl, err := template.New(tplName).Parse(initGenTemplate) if err != nil { return err diff --git a/pkg/action/action.go b/pkg/action/action.go index fd51f73..8b121e0 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -29,10 +29,11 @@ type Action struct { // wd is a working directory set from app level. // Usually current working directory, but may be overridden by a plugin. - wd string - fsdir string // fsdir is a base directory where the action was discovered (for better ID naming). - fpath string // fpath is a path to action definition file. - def *Definition // def is an action definition. Loaded by Loader, may be nil when not initialized. + wd string + fsdir string // fsdir is a base directory where the action was discovered (for better ID idp). + fpath string // fpath is a path to action definition file. + def *Definition // def is an action definition. Loaded by Loader, may be nil when not initialized. + defRaw *Definition // defRaw is a raw action definition. Loaded by Loader, may be nil when not initialized. env RunEnvironment // env is the run environment driver to execute the action. input Input // input is a container for env variables. @@ -55,9 +56,10 @@ type ( ) // NewAction creates a new action. -func NewAction(id, wd, fsdir, fpath string) *Action { +func NewAction(wd, fsdir, fpath string) *Action { + // We don't define ID here because we use Action object for + // context creation to calculate ID later. return &Action{ - ID: id, wd: wd, fsdir: fsdir, fpath: fpath, @@ -107,7 +109,7 @@ func (a *Action) WorkDir() string { } // Filepath returns action file path. -func (a *Action) Filepath() string { return a.fpath } +func (a *Action) Filepath() string { return filepath.Join(a.fsdir, a.fpath) } // Dir returns an action file directory. func (a *Action) Dir() string { return filepath.Dir(a.Filepath()) } @@ -118,6 +120,16 @@ func (a *Action) SetRunEnvironment(env RunEnvironment) { a.env = env } // DefinitionEncoded returns encoded action file content. func (a *Action) DefinitionEncoded() ([]byte, error) { return a.Loader.Content() } +// Raw returns unprocessed action definition. It is faster and may produce fewer errors. +// It may be helpful if needed to peek inside the action file to read header. +func (a *Action) Raw() (*Definition, error) { + var err error + if a.defRaw == nil { + a.defRaw, err = a.Loader.LoadRaw() + } + return a.defRaw, err +} + // EnsureLoaded loads an action file with replaced arguments and options. func (a *Action) EnsureLoaded() (err error) { if a.def != nil { @@ -166,7 +178,6 @@ func (a *Action) SetInput(input Input) (err error) { return a.EnsureLoaded() } -// validateJSONSchema validates arguments and options according to func (a *Action) processOptions(opts TypeOpts) error { for _, optDef := range a.ActionDef().Options { if _, ok := opts[optDef.Name]; !ok { @@ -238,7 +249,7 @@ func (a *Action) processValue(value interface{}, valueType jsonschema.Type, toAp // validateJSONSchema validates arguments and options according to // a specified json schema in action definition. // @todo move to jsonschema -func (a *Action) validateJSONSchema(inp Input) error { +func (a *Action) validateJSONSchema(inp Input) error { //nolint:unused jsch := a.JSONSchema() // @todo cache jsonschema and resources. b, err := json.Marshal(jsch) diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 6d71f6b..5b29e93 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -1,6 +1,7 @@ package action import ( + "context" "fmt" "os" "path/filepath" @@ -12,9 +13,10 @@ import ( func Test_Action(t *testing.T) { assert := assert.New(t) // Prepare an action. - fs := _getFsMapActions(1, validFullYaml, true) + fs := _getFsMapActions(1, validFullYaml, genPathTypeValid) ad := NewYamlDiscovery(NewDiscoveryFS(fs, "")) - actions, err := ad.Discover() + ctx := context.Background() + actions, err := ad.Discover(ctx) assert.NoError(err) assert.NotEmpty(actions) act := actions[0] diff --git a/pkg/action/cobra.go b/pkg/action/cobra.go index 8507d20..cb19a23 100644 --- a/pkg/action/cobra.go +++ b/pkg/action/cobra.go @@ -5,9 +5,8 @@ import ( "reflect" "strings" - "github.com/spf13/pflag" - "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/launchrctl/launchr/pkg/cli" "github.com/launchrctl/launchr/pkg/jsonschema" @@ -16,7 +15,11 @@ import ( // CobraImpl returns cobra command implementation for an action command. func CobraImpl(a *Action, streams cli.Streams) (*cobra.Command, error) { - actConf := a.ActionDef() + def, err := a.Raw() + if err != nil { + return nil, err + } + actConf := def.Action argsDef := actConf.Arguments use := a.ID for _, p := range argsDef { @@ -28,13 +31,14 @@ func CobraImpl(a *Action, streams cli.Streams) (*cobra.Command, error) { Use: use, // Using custom args validation in ValidateInput. // @todo: maybe we need a long template for arguments description + // @todo: have aliases documented in help Short: getDesc(actConf.Title, actConf.Description), Aliases: actConf.Aliases, RunE: func(cmd *cobra.Command, args []string) error { // Pass to the run environment its flags. if env, ok := a.env.(RunEnvironmentFlags); ok { runOpts = filterFlags(cmd, runOpts) - err := env.UseFlags(derefOpts(runOpts)) + err = env.UseFlags(derefOpts(runOpts)) if err != nil { return err } @@ -48,14 +52,14 @@ func CobraImpl(a *Action, streams cli.Streams) (*cobra.Command, error) { ArgsRaw: args, } if runEnv, ok := a.env.(RunEnvironmentFlags); ok { - if err := runEnv.ValidateInput(a, input.Args); err != nil { + if err = runEnv.ValidateInput(a, input.Args); err != nil { return err } } cmd.SilenceUsage = true // Don't show usage help on a runtime error. - if err := a.SetInput(input); err != nil { + if err = a.SetInput(input); err != nil { return err } @@ -65,7 +69,7 @@ func CobraImpl(a *Action, streams cli.Streams) (*cobra.Command, error) { } // Collect action flags. - err := setCobraOptions(cmd, actConf.Options, options) + err = setCobraOptions(cmd, actConf.Options, options) if err != nil { return nil, err } diff --git a/pkg/action/discover.go b/pkg/action/discover.go index 94724c4..a70256f 100644 --- a/pkg/action/discover.go +++ b/pkg/action/discover.go @@ -2,12 +2,15 @@ package action import ( + "context" "fmt" "io/fs" + "os" "path/filepath" "sort" "strings" "sync" + "time" "github.com/launchrctl/launchr/internal/launchr" "github.com/launchrctl/launchr/pkg/log" @@ -20,7 +23,13 @@ var actionsSubdir = strings.Join([]string{"", actionsDirname, ""}, string(filepa // DiscoveryPlugin is a launchr plugin to discover actions. type DiscoveryPlugin interface { launchr.Plugin - DiscoverActions(fs launchr.ManagedFS) ([]*Action, error) + DiscoverActions(ctx context.Context, fs launchr.ManagedFS, idp IDProvider) ([]*Action, error) +} + +// AlterActionsPlugin is a launchr plugin to alter registered actions. +type AlterActionsPlugin interface { + launchr.Plugin + AlterActions() error } // DiscoveryFS is a file system to discover actions. @@ -50,44 +59,81 @@ type DiscoveryStrategy interface { Loader(l FileLoadFn, p ...LoadProcessor) Loader } +// IDProvider provides an ID for an action. +type IDProvider interface { + GetID(a *Action) string +} + // Discovery defines a common functionality for discovering action files. type Discovery struct { fs DiscoveryFS fsDir string - s DiscoveryStrategy + ds DiscoveryStrategy + idp IDProvider } // NewDiscovery creates an instance of action discovery. func NewDiscovery(fs DiscoveryFS, ds DiscoveryStrategy) *Discovery { fsDir := launchr.GetFsAbsPath(fs.fs) - return &Discovery{fs, fsDir, ds} + return &Discovery{ + fs: fs, + fsDir: fsDir, + ds: ds, + idp: DefaultIDProvider{}, + } } func (ad *Discovery) isValid(path string, d fs.DirEntry) bool { i := strings.LastIndex(path, actionsSubdir) - if d.IsDir() || i == -1 || isHidden(path) { + // Invalid paths for action definition file. + if d.IsDir() || + // No "actions" directory in the path. + i == -1 || + // Must not be hidden itself. + isHiddenPath(path) || + // Count depth of directories inside actions, must be only 1, not deeper. + // Nested actions are not allowed. + // dir/actions/1/action.yaml - OK, dir/actions/1/2/action.yaml - NOK. + strings.Count(path[i+len(actionsSubdir):], string(filepath.Separator)) > 1 { return false } - return strings.Count(path[i+len(actionsSubdir):], string(filepath.Separator)) == 1 && // Nested actions are not allowed. - ad.s.IsValid(d.Name()) + return ad.ds.IsValid(d.Name()) } // findFiles searches for a filename in a given dir. // Returns an array of relative file paths. -func (ad *Discovery) findFiles() chan string { +func (ad *Discovery) findFiles(ctx context.Context) chan string { ch := make(chan string, 10) go func() { + longOpTimeout := time.After(5 * time.Second) err := fs.WalkDir(ad.fs, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err + select { + // Show feedback on a long-running walk. + case <-longOpTimeout: + log.Warn("It takes more time than expected to discover actions.\nProbably you are running outside a project directory.") + // Stop walking if the context has expired. + case <-ctx.Done(): + return fs.SkipAll + default: + // Continue to scan. } - - if d.IsDir() && isHidden(path) { + // Skip OS specific directories to prevent going too deep. + // Skip hidden directories. + if d.IsDir() && (isHiddenPath(path) || skipSystemDirs(ad.fsDir, path)) { return fs.SkipDir } + if err != nil { + // Skip dir on access denied. + if os.IsPermission(err) && d.IsDir() { + return fs.SkipDir + } + // Stop walking on unknown error. + return err + } + // Check if the file is a candidate to be an action file. if ad.isValid(path, d) { ch <- path } @@ -109,12 +155,14 @@ func (ad *Discovery) findFiles() chan string { // Discover traverses the file structure for a given discovery path. // Returns array of Action. // If an action is invalid, it's ignored. -func (ad *Discovery) Discover() ([]*Action, error) { +func (ad *Discovery) Discover(ctx context.Context) ([]*Action, error) { + defer log.DebugTimer("Action discovering")() wg := sync.WaitGroup{} mx := sync.Mutex{} actions := make([]*Action, 0, 32) - for f := range ad.findFiles() { + // Traverse the FS. + for f := range ad.findFiles(ctx) { wg.Add(1) go func(f string) { defer wg.Done() @@ -137,22 +185,34 @@ func (ad *Discovery) Discover() ([]*Action, error) { // parseFile parses file f and returns an action. func (ad *Discovery) parseFile(f string) *Action { - id := getActionID(f) - if id == "" { - panic(fmt.Errorf("action id cannot be empty, file %q", f)) - } - a := NewAction(id, absPath(ad.fs.wd), ad.fsDir, filepath.Join(ad.fsDir, f)) - a.Loader = ad.s.Loader( + a := NewAction(absPath(ad.fs.wd), ad.fsDir, f) + a.Loader = ad.ds.Loader( func() (fs.File, error) { return ad.fs.Open(f) }, envProcessor{}, inputProcessor{}, ) + // Assign ID to an action. + a.ID = ad.idp.GetID(a) + if a.ID == "" { + panic(fmt.Errorf("action id cannot be empty, file %q", f)) + } + return a } -// getActionID parses filename and returns CLI command name. +// SetActionIDProvider sets discovery specific action id provider. +func (ad *Discovery) SetActionIDProvider(idp IDProvider) { + ad.idp = idp +} + +// DefaultIDProvider is a default action id provider. +type DefaultIDProvider struct{} + +// GetID implements IDProvider interface. +// It parses action filename and returns CLI command name. // Empty string if the command name can't be generated. -func getActionID(f string) string { +func (idp DefaultIDProvider) GetID(a *Action) string { + f := a.fpath s := filepath.Dir(f) i := strings.LastIndex(s, actionsSubdir) if i == -1 { diff --git a/pkg/action/discover.skip.go b/pkg/action/discover.skip.go new file mode 100644 index 0000000..136953e --- /dev/null +++ b/pkg/action/discover.skip.go @@ -0,0 +1,43 @@ +// Package action provides implementations of discovering and running actions. +package action + +func skipSystemDirs(root string, path string) bool { + if root == "" { + // We are in virtual FS. + return false + } + + dirs := []string{ + // Python specific. + "__pycache__", + "venv", + // JS specific stuff. + "node_modules", + // Usually project dependencies. + "vendor", + } + + // Check application specific. + if existsInSlice(dirs, path) { + return true + } + // Skip in root. + if isRootPath(root) && existsInSlice(skipRootDirs, path) { + return true + } + // Skip user specific directories. + if isUserHomeDir(path) && existsInSlice(skipUserDirs, path) { + return true + } + + return false +} + +func existsInSlice[T comparable](slice []T, el T) bool { + for _, v := range slice { + if v == el { + return true + } + } + return false +} diff --git a/pkg/action/discover.unix.go b/pkg/action/discover.unix.go new file mode 100644 index 0000000..bc21a86 --- /dev/null +++ b/pkg/action/discover.unix.go @@ -0,0 +1,72 @@ +//go:build !windows +// +build !windows + +package action + +import ( + "path/filepath" + "strings" +) + +var skipRootDirs = []string{ + // Unix + // It's not recommended to run in root, + // but if so we will skip most of the paths. + "bin", + "sbin", + "lib", + "etc", + "var", + "tmp", + "dev", + "proc", + "sys", + "boot", + "srv", + + // MacOs + "System", + "Library", + "Applications", +} + +var skipUserDirs = []string{ + // Go root is usually in home and have a lot of packages. + "go", + + // MacOs + "Applications", + "Documents", + "Desktop", + "Downloads", + "Library", + "Music", + "Pictures", + "Movies", + "Public", +} + +func isHiddenPath(path string) bool { + if path == "." { + return false + } + dirs := strings.Split(path, string(filepath.Separator)) + for _, v := range dirs { + if v[0] == '.' { + return true + } + } + + return false +} + +func isRootPath(path string) bool { + return path == "/" +} + +func isUserHomeDir(path string) bool { + abs, _ := filepath.Abs(path) + linux, _ := filepath.Match("/home/*/*", abs) + macOs, _ := filepath.Match("/Users/*/*", abs) + return linux || macOs +} diff --git a/pkg/action/discover.windows.go b/pkg/action/discover.windows.go new file mode 100644 index 0000000..5bda95f --- /dev/null +++ b/pkg/action/discover.windows.go @@ -0,0 +1,58 @@ +//go:build windows +// +build windows + +// Package action provides implementations of discovering and running actions. +package action + +import ( + "path/filepath" + "syscall" +) + +var skipRootDirs = []string{ + "Windows", + "Program Files", + "Program Files (x86)", + "ProgramData", +} + +var skipUserDirs = []string{ + "AppData", + "Desktop", + "Documents", + "Downloads", + "Music", + "Pictures", + "Videos", + "Favorites", + "Public", +} + +func isHiddenPath(path string) bool { + absPath, err := filepath.Abs(path) + if err != nil { + return false + } + + pointer, err := syscall.UTF16PtrFromString(`\\?\` + absPath) + if err != nil { + return false + } + + attributes, err := syscall.GetFileAttributes(pointer) + if err != nil { + return false + } + + return attributes&syscall.FILE_ATTRIBUTE_HIDDEN != 0 +} + +func isRootPath(path string) bool { + return path == `C:\` +} + +func isUserHomeDir(path string) bool { + abs, _ := filepath.Abs(path) + win, _ := filepath.Match(`C:\Users\*\*`, abs) + return win +} diff --git a/pkg/action/discover_test.go b/pkg/action/discover_test.go index 8ce714d..6d25086 100644 --- a/pkg/action/discover_test.go +++ b/pkg/action/discover_test.go @@ -1,6 +1,7 @@ package action import ( + "context" "io/fs" "math/rand" "path" @@ -11,6 +12,14 @@ import ( "github.com/stretchr/testify/assert" ) +type genPathType int + +const ( + genPathTypeValid genPathType = iota // genPathTypeValid is a valid actions path + genPathTypeArbitrary // genPathTypeArbitrary is a random string without actions directory. + genPathTypeGHActions // genPathTypeGHActions is an incorrect hidden path but with actions directory. +) + func Test_Discover(t *testing.T) { t.Parallel() @@ -20,14 +29,15 @@ func Test_Discover(t *testing.T) { expCnt int } - allValid := _getFsMapActions(7, validEmptyVersionYaml, true) + allValid := _getFsMapActions(7, validEmptyVersionYaml, genPathTypeValid) invalidYaml := _mergeFsMaps( - _getFsMapActions(7, validEmptyVersionYaml, true), - _getFsMapActions(3, invalidEmptyCmdYaml, true), + _getFsMapActions(7, validEmptyVersionYaml, genPathTypeValid), + _getFsMapActions(3, invalidEmptyCmdYaml, genPathTypeValid), ) invalidPath := _mergeFsMaps( - _getFsMapActions(7, validEmptyVersionYaml, true), - _getFsMapActions(3, validEmptyVersionYaml, false), + _getFsMapActions(7, validEmptyVersionYaml, genPathTypeValid), + _getFsMapActions(3, validEmptyVersionYaml, genPathTypeArbitrary), + _getFsMapActions(3, validEmptyVersionYaml, genPathTypeGHActions), ) // @todo test path contains 2 actions in same dir. @@ -40,13 +50,14 @@ func Test_Discover(t *testing.T) { // Invalid yaml paths are ignored. {"invalid paths", invalidPath, 7}, } + ctx := context.Background() for _, tt := range tts { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() ad := NewYamlDiscovery(NewDiscoveryFS(tt.fs, "")) - actions, err := ad.Discover() + actions, err := ad.Discover(ctx) if err != nil { t.Errorf("unexpected error %v", err) } @@ -59,20 +70,21 @@ func Test_Discover(t *testing.T) { func Test_Discover_ActionWD(t *testing.T) { // Test if working directory is correctly set to actions on discovery. - tfs := _getFsMapActions(1, validEmptyVersionYaml, true) + tfs := _getFsMapActions(1, validEmptyVersionYaml, genPathTypeValid) var expFPath string for expFPath = range tfs { break } expectedWD := "expectedWD" ad := NewYamlDiscovery(NewDiscoveryFS(tfs, expectedWD)) - actions, err := ad.Discover() + ctx := context.Background() + actions, err := ad.Discover(ctx) assert.NoError(t, err) assert.Equal(t, expFPath, actions[0].fpath) assert.Equal(t, absPath(expectedWD), actions[0].wd) ad = NewYamlDiscovery(NewDiscoveryFS(tfs, "")) - actions, err = ad.Discover() + actions, err = ad.Discover(ctx) assert.NoError(t, err) assert.Equal(t, expFPath, actions[0].fpath) assert.Equal(t, absPath(""), actions[0].wd) @@ -103,16 +115,17 @@ func Test_Discover_isValid(t *testing.T) { } tts := []testCase{ - {"valid yaml", "1/2/actions/3/action.yaml", true}, // Valid action.yaml path. - {"valid yml", "1/2/actions/3/action.yml", true}, // Valid action.yml path. - {"random file", "1/2/actions/3/random.yaml", false}, // Random yaml name. - {"incorrect filename prefix", "1/2/actions/3/myaction.yaml", false}, // Incorrect prefix. - {"incorrect filename suffix", "1/2/actions/3/action.yaml.bkp", false}, // Incorrect suffix. - {"incorrect path", "1/2/3/action.yaml", false}, // File not inside an "actions" directory. - {"incorrect hidden path", ".1/2/actions/3/action.yml", false}, // Invalid hidden directory - {"nested action", "1/2/actions/3/4/5/action.yaml", false}, // There is a deeper nesting in actions directory. - {"root action", "actions/verb/action.yaml", false}, // Actions are located in root. - {"dir", "1/2/actions/3", false}, // A directory is given. + {"valid yaml", "1/2/actions/3/action.yaml", true}, // Valid action.yaml path. + {"valid yml", "1/2/actions/3/action.yml", true}, // Valid action.yml path. + {"random file", "1/2/actions/3/random.yaml", false}, // Random yaml name. + {"incorrect filename prefix", "1/2/actions/3/myaction.yaml", false}, // Incorrect prefix. + {"incorrect filename suffix", "1/2/actions/3/action.yaml.bkp", false}, // Incorrect suffix. + {"incorrect path", "1/2/3/action.yaml", false}, // File not inside an "actions" directory. + {"incorrect hidden root path", ".1/2/actions/3/action.yml", false}, // Invalid hidden directory. + {"incorrect hidden subdir path", "1/2/.github/actions/3/action.yml", false}, // Invalid hidden subdirectory. + {"nested action", "1/2/actions/3/4/5/action.yaml", false}, // There is a deeper nesting in actions directory. + {"root action", "actions/verb/action.yaml", false}, // Actions are located in root. + {"dir", "1/2/actions/3", false}, // A directory is given. } // Run tests. @@ -129,7 +142,7 @@ func Test_Discover_isValid(t *testing.T) { } } -func Test_Discover_getActionName(t *testing.T) { +func Test_Discover_IDProvider(t *testing.T) { t.Parallel() type testCase struct { path string @@ -151,30 +164,38 @@ func Test_Discover_getActionName(t *testing.T) { // Unexpected path, but valid. {"1/2/3/actions/4/5/6/random.yaml", "1.2.3:4.5.6"}, } + idp := DefaultIDProvider{} for _, tt := range tts { - res := getActionID(tt.path) + res := idp.GetID(&Action{fpath: tt.path}) if tt.exp != res { t.Errorf("expected %q, got %q", tt.exp, res) } } } -func _generateActionPath(d int, validPath bool) string { +func _generateActionPath(d int, pathType genPathType) string { elems := make([]string, 0, d+3) for i := 0; i < d; i++ { elems = append(elems, namesgenerator.GetRandomName(0)) } - if validPath { + switch pathType { + case genPathTypeValid: elems = append(elems, actionsDirname) + case genPathTypeGHActions: + elems = append(elems, ".github", actionsDirname) + case genPathTypeArbitrary: + fallthrough + default: + // Do nothing. } elems = append(elems, namesgenerator.GetRandomName(0), "action.yaml") return path.Join(elems...) } -func _getFsMapActions(num int, str string, validPath bool) fstest.MapFS { +func _getFsMapActions(num int, str string, pathType genPathType) fstest.MapFS { m := make(fstest.MapFS) for i := 0; i < num; i++ { - fa := _generateActionPath(rand.Intn(5)+1, validPath) //nolint:gosec + fa := _generateActionPath(rand.Intn(5)+1, pathType) //nolint:gosec m[fa] = &fstest.MapFile{Data: []byte(str)} } return m diff --git a/pkg/action/discover_unix.go b/pkg/action/discover_unix.go deleted file mode 100644 index 7151a1b..0000000 --- a/pkg/action/discover_unix.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build !windows -// +build !windows - -package action - -import ( - "path/filepath" -) - -func isHidden(path string) bool { - pathList := filepath.SplitList(path) - for _, v := range pathList { - if len(v) > 1 && v[0:1] == "." { - return true - } - } - - return false -} diff --git a/pkg/action/discover_windows.go b/pkg/action/discover_windows.go deleted file mode 100644 index 671fe87..0000000 --- a/pkg/action/discover_windows.go +++ /dev/null @@ -1,26 +0,0 @@ -// Package action provides implementations of discovering and running actions. -package action - -import ( - "path/filepath" - "syscall" -) - -func isHidden(path string) bool { - absPath, err := filepath.Abs(path) - if err != nil { - return false - } - - pointer, err := syscall.UTF16PtrFromString(`\\?\` + absPath) - if err != nil { - return false - } - - attributes, err := syscall.GetFileAttributes(pointer) - if err != nil { - return false - } - - return attributes&syscall.FILE_ATTRIBUTE_HIDDEN != 0 -} diff --git a/pkg/action/env.container_test.go b/pkg/action/env.container_test.go index 0e2dcc4..9fc8e6e 100644 --- a/pkg/action/env.container_test.go +++ b/pkg/action/env.container_test.go @@ -390,7 +390,7 @@ func Test_ContainerExec_containerWait(t *testing.T) { tts := []testCase{ { "condition removed", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0} }, types.WaitConditionRemoved, @@ -398,7 +398,7 @@ func Test_ContainerExec_containerWait(t *testing.T) { }, { "condition next exit", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0} }, types.WaitConditionNextExit, @@ -406,7 +406,7 @@ func Test_ContainerExec_containerWait(t *testing.T) { }, { "return exit code", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 2} }, types.WaitConditionRemoved, @@ -414,7 +414,7 @@ func Test_ContainerExec_containerWait(t *testing.T) { }, { "fail on container run", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0, Error: errors.New("fail run")} }, types.WaitConditionRemoved, @@ -422,7 +422,7 @@ func Test_ContainerExec_containerWait(t *testing.T) { }, { "fail on wait", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(_ chan types.ContainerWaitResponse, errCh chan error) { errCh <- errors.New("fail wait") }, types.WaitConditionRemoved, @@ -605,7 +605,7 @@ func Test_ContainerExec(t *testing.T) { tts := []testCase{ { "successful run", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0} }, successSteps, @@ -652,7 +652,7 @@ func Test_ContainerExec(t *testing.T) { }, { "error on container attach", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0} }, append( @@ -668,7 +668,7 @@ func Test_ContainerExec(t *testing.T) { }, { "error start container", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 0} }, append( @@ -684,7 +684,7 @@ func Test_ContainerExec(t *testing.T) { }, { "container return error", - func(resCh chan types.ContainerWaitResponse, errCh chan error) { + func(resCh chan types.ContainerWaitResponse, _ chan error) { resCh <- types.ContainerWaitResponse{StatusCode: 2} }, append( @@ -781,7 +781,6 @@ func callContainerDriverMockFn(d *mockdriver.MockContainerRunner, step mockCallI case -1: call.AnyTimes() default: - } if call != nil && prev != nil { call.After(prev) @@ -827,7 +826,6 @@ func Test_ConfigImageBuildInfo(t *testing.T) { } }) } - } const cfgYaml = ` diff --git a/pkg/action/loader.go b/pkg/action/loader.go index e80b942..0f0d667 100644 --- a/pkg/action/loader.go +++ b/pkg/action/loader.go @@ -79,7 +79,7 @@ func (p inputProcessor) Process(ctx LoadContext, b []byte) ([]byte, error) { return b, nil } a := ctx.Action - def, err := ctx.Action.Loader.LoadRaw() + def, err := ctx.Action.Raw() if err != nil { return nil, err } diff --git a/pkg/action/manager.go b/pkg/action/manager.go index 51928a1..f7b326c 100644 --- a/pkg/action/manager.go +++ b/pkg/action/manager.go @@ -8,6 +8,7 @@ import ( "time" "github.com/launchrctl/launchr/internal/launchr" + "github.com/launchrctl/launchr/pkg/log" ) // Manager handles actions and its execution. @@ -15,38 +16,53 @@ type Manager interface { launchr.Service // All returns all actions copied and decorated. All() map[string]*Action - // AllRef returns all original action values from the storage. - // Use it only if you need to read-only actions without allocations. It may be unsafe to read/write the map. - // If you need to run actions, use Get or All, it will provide configured for run Action. - AllRef() map[string]*Action - // AllAliasRef returns map of all aliased actions - AllAliasRef() map[string]string // Get returns a copy of an action from the manager with default decorators. Get(id string) (*Action, bool) - // GetRef returns an original action value from the storage. - GetRef(id string) (*Action, bool) + // Add saves an action in the manager. + Add(*Action) + // Delete deletes the action from the manager. + Delete(id string) + // Decorate decorates an action with given behaviors and returns its copy. + // If functions withFn are not provided, default functions are applied. + Decorate(a *Action, withFn ...DecorateWithFn) *Action + // GetIDFromAlias returns a real action ID by its alias. If not, returns alias. + GetIDFromAlias(alias string) string + + // GetActionIDProvider returns global application action id provider. + GetActionIDProvider() IDProvider + // SetActionIDProvider sets global application action id provider. + SetActionIDProvider(p IDProvider) + // AddValueProcessor adds processor to list of available processors AddValueProcessor(name string, vp ValueProcessor) // GetValueProcessors returns list of available processors GetValueProcessors() map[string]ValueProcessor - // Decorate decorates an action with given behaviors and returns its copy. - // If functions withFn are not provided, default functions are applied. - Decorate(a *Action, withFn ...DecorateWithFn) *Action - // Add saves an action in the manager. - Add(*Action) + // DefaultRunEnvironment provides the default action run environment. DefaultRunEnvironment() RunEnvironment - // Run executes an action in foreground. Run(ctx context.Context, a *Action) (RunInfo, error) // RunBackground executes an action in background. - RunBackground(ctx context.Context, a *Action, runId string) (RunInfo, chan error) + RunBackground(ctx context.Context, a *Action, runID string) (RunInfo, chan error) // RunInfoByAction returns all running actions by action id. RunInfoByAction(aid string) []RunInfo // RunInfoByID returns an action matching run id. RunInfoByID(id string) (RunInfo, bool) } +// ManagerUnsafe is an extension of the Manager interface for unsafe access to actions. +// Consider twice before using it! +type ManagerUnsafe interface { + Manager + // AllUnsafe returns all original action values from the storage. + // Use it only if you need to read-only actions without allocations. + // It is unsafe to operate the actions as they are the original and affect the whole app. + // If you need to run actions, use Get or All, it will provide configured for run Action. + AllUnsafe() map[string]*Action + // GetUnsafe returns an original action value from the storage. + GetUnsafe(id string) (*Action, bool) +} + // DecorateWithFn is a type alias for functions accepted in a Manager.Decorate interface method. type DecorateWithFn = func(m Manager, a *Action) @@ -58,6 +74,7 @@ type actionManagerMap struct { mxRun sync.Mutex dwFns []DecorateWithFn processors map[string]ValueProcessor + idProvider IDProvider } // NewManager constructs a new action manager. @@ -80,25 +97,51 @@ func (m *actionManagerMap) Add(a *Action) { defer m.mx.Unlock() m.actionStore[a.ID] = a - for _, alias := range a.ActionDef().Aliases { - m.actionAliases[alias] = a.ID + // Collect action aliases. + def, err := a.Raw() + if err != nil { + return + } + for _, alias := range def.Action.Aliases { + id, ok := m.actionAliases[alias] + if ok { + log.Warn("Alias %q is already defined by %q", alias, id) + } else { + m.actionAliases[alias] = a.ID + } } } -func (m *actionManagerMap) AllRef() map[string]*Action { +func (m *actionManagerMap) AllUnsafe() map[string]*Action { m.mx.Lock() defer m.mx.Unlock() return copyMap(m.actionStore) } -func (m *actionManagerMap) AllAliasRef() map[string]string { +func (m *actionManagerMap) GetIDFromAlias(alias string) string { + if id, ok := m.actionAliases[alias]; ok { + return id + } + return alias +} + +func (m *actionManagerMap) Delete(id string) { m.mx.Lock() defer m.mx.Unlock() - return copyMap(m.actionAliases) + _, ok := m.actionStore[id] + if !ok { + return + } + delete(m.actionStore, id) + for _, idAlias := range m.actionAliases { + if idAlias == id { + delete(m.actionAliases, id) + } + } } func (m *actionManagerMap) All() map[string]*Action { - ret := m.AllRef() + ret := m.AllUnsafe() for k, v := range ret { ret[k] = m.Decorate(v, m.dwFns...) } @@ -106,12 +149,12 @@ func (m *actionManagerMap) All() map[string]*Action { } func (m *actionManagerMap) Get(id string) (*Action, bool) { - a, ok := m.GetRef(id) + a, ok := m.GetUnsafe(id) // Process action with default decorators and return a copy to have an isolated scope. return m.Decorate(a, m.dwFns...), ok } -func (m *actionManagerMap) GetRef(id string) (*Action, bool) { +func (m *actionManagerMap) GetUnsafe(id string) (*Action, bool) { m.mx.Lock() defer m.mx.Unlock() a, ok := m.actionStore[id] @@ -144,6 +187,20 @@ func (m *actionManagerMap) Decorate(a *Action, withFns ...DecorateWithFn) *Actio return a } +func (m *actionManagerMap) GetActionIDProvider() IDProvider { + if m.idProvider == nil { + m.SetActionIDProvider(nil) + } + return m.idProvider +} + +func (m *actionManagerMap) SetActionIDProvider(p IDProvider) { + if p == nil { + p = DefaultIDProvider{} + } + m.idProvider = p +} + func (m *actionManagerMap) DefaultRunEnvironment() RunEnvironment { return NewDockerEnvironment() } @@ -187,9 +244,9 @@ func (m *actionManagerMap) Run(ctx context.Context, a *Action) (RunInfo, error) return m.registerRun(a, ""), a.Execute(ctx) } -func (m *actionManagerMap) RunBackground(ctx context.Context, a *Action, runId string) (RunInfo, chan error) { - // @todo change runId to runOptions with possibility to create filestream names in webUI. - ri := m.registerRun(a, runId) +func (m *actionManagerMap) RunBackground(ctx context.Context, a *Action, runID string) (RunInfo, chan error) { + // @todo change runID to runOptions with possibility to create filestream names in webUI. + ri := m.registerRun(a, runID) chErr := make(chan error) go func() { m.updateRunStatus(ri.ID, "running") @@ -234,7 +291,7 @@ func WithDefaultRunEnvironment(m Manager, a *Action) { func WithContainerRunEnvironmentConfig(cfg launchr.Config, prefix string) DecorateWithFn { r := LaunchrConfigImageBuildResolver{cfg} ccr := NewImageBuildCacheResolver(cfg) - return func(m Manager, a *Action) { + return func(_ Manager, a *Action) { if env, ok := a.env.(ContainerRunEnvironment); ok { env.AddImageBuildResolver(r) env.SetImageBuildCacheResolver(ccr) diff --git a/pkg/action/yaml.def.go b/pkg/action/yaml.def.go index 461d335..1cdd60f 100644 --- a/pkg/action/yaml.def.go +++ b/pkg/action/yaml.def.go @@ -26,6 +26,7 @@ const ( sErrInvalidActionArgName = "argument name %q is not valid" sErrInvalidActionOptName = "option name %q is not valid" sErrDupActionVarName = "argument or option name %q is already defined, a variable name must be unique in the action definition" + sErrActionDefMissing = "action definition is missing in the declaration" ) type errUnsupportedActionVersion struct { @@ -83,7 +84,7 @@ func CreateFromYamlTpl(b []byte) (*Definition, error) { return CreateFromYaml(r) } -// Definition is a representation of an action file +// Definition is a representation of an action file. type Definition struct { Version string `yaml:"version"` WD string `yaml:"working_directory"` @@ -126,12 +127,15 @@ func (d *Definition) UnmarshalYAML(node *yaml.Node) (err error) { return nil } -func validateV1(_ *Definition) error { +func validateV1(d *Definition) error { // The schema is validated on parsing. + if d.Action == nil { + return errors.New(sErrActionDefMissing) + } return nil } -// DefAction holds action configuration +// DefAction holds action configuration. type DefAction struct { Title string `yaml:"title"` Description string `yaml:"description"` diff --git a/pkg/action/yaml_const_test.go b/pkg/action/yaml_const_test.go index bf3658c..2e1a25d 100644 --- a/pkg/action/yaml_const_test.go +++ b/pkg/action/yaml_const_test.go @@ -14,6 +14,9 @@ working_directory: "{{ .current_working_dir }}" action: title: Title description: Description + alias: + - alias1 + - alias2 arguments: - name: arg1 title: Argument 1 diff --git a/pkg/driver/tty.go b/pkg/driver/tty.go index 8118c0e..286ceb9 100644 --- a/pkg/driver/tty.go +++ b/pkg/driver/tty.go @@ -63,7 +63,7 @@ func initTtySize(ctx context.Context, d ContainerRunner, cli cli.Streams, id str } } if err != nil { - fmt.Fprintln(cli.Err(), "failed to resize tty, using default size") + _, _ = fmt.Fprintln(cli.Err(), "failed to resize tty, using default size") } }() } diff --git a/pkg/log/logger.go b/pkg/log/logger.go index 639e544..b186ff8 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -2,6 +2,8 @@ // and provide logging functionality interface. package log +import "time" + // Level defines a log level. type Level int @@ -20,7 +22,7 @@ type Config struct { Verbosity Level } -// Logger interface defines leveled logging functionality +// Logger interface defines leveled logging functionality. type Logger interface { Debug(format string, v ...any) Info(format string, v ...any) @@ -72,3 +74,16 @@ func Fatal(format string, v ...any) { func SetLevel(lvl Level) { l.SetLevel(lvl) } + +// DebugTimer returns a function that prints the name argument and +// the elapsed time between the call to timer and the call to +// the returned function. The returned function is intended to +// be used in a defer statement: +// +// defer DebugTimer("sum")(). +func DebugTimer(name string) func() { + start := time.Now() + return func() { + Debug("%s took %v\n", name, time.Since(start).Round(time.Millisecond)) + } +} diff --git a/plugins/actionnaming/plugin.go b/plugins/actionnaming/plugin.go new file mode 100644 index 0000000..fbcaad3 --- /dev/null +++ b/plugins/actionnaming/plugin.go @@ -0,0 +1,73 @@ +// Package actionnaming is a plugin of launchr to adjust action ids. +package actionnaming + +import ( + "strings" + + "github.com/launchrctl/launchr/internal/launchr" + "github.com/launchrctl/launchr/pkg/action" +) + +type launchrCfg struct { + ActionsNaming []actionsNaming `yaml:"actions_naming"` +} + +type actionsNaming struct { + Search string `yaml:"search"` + Replace string `yaml:"replace"` +} + +func init() { + launchr.RegisterPlugin(Plugin{}) +} + +// Plugin is launchr plugin to improve actions naming. +type Plugin struct{} + +// PluginInfo implements launchr.Plugin interface. +func (p Plugin) PluginInfo() launchr.PluginInfo { + return launchr.PluginInfo{} +} + +// OnAppInit implements launchr.Plugin interface. +func (p Plugin) OnAppInit(app launchr.App) error { + // Get services. + var cfg launchr.Config + var am action.Manager + app.GetService(&cfg) + app.GetService(&am) + + // Load naming configuration. + var launchrConfig *launchrCfg + // @todo refactor yaml property position. + err := cfg.Get("launchrctl", &launchrConfig) + if err != nil { + return err + } + // Override action id provider. + if launchrConfig == nil || len(launchrConfig.ActionsNaming) == 0 { + // No need to override. + return nil + } + am.SetActionIDProvider(&ConfigActionIDProvider{ + parent: am.GetActionIDProvider(), + naming: launchrConfig.ActionsNaming, + }) + return nil +} + +// ConfigActionIDProvider is an ID provider based on launchr global configuration. +type ConfigActionIDProvider struct { + parent action.IDProvider + naming []actionsNaming +} + +// GetID implements action.IDProvider interface. +func (idp *ConfigActionIDProvider) GetID(a *action.Action) string { + id := idp.parent.GetID(a) + newID := id + for _, an := range idp.naming { + newID = strings.ReplaceAll(newID, an.Search, an.Replace) + } + return newID +} diff --git a/plugins/builder/builder.go b/plugins/builder/builder.go index 8bf1ae1..6b97a9c 100644 --- a/plugins/builder/builder.go +++ b/plugins/builder/builder.go @@ -301,7 +301,7 @@ func (b *Builder) preBuild(ctx context.Context) error { // move assets from tmp dir to assets folder. log.Debug("moving assets from tmp to build folder") - err = cp.Copy(tmpPath, pluginAssetsPath, cp.Options{OnDirExists: func(src, dest string) cp.DirExistsAction { + err = cp.Copy(tmpPath, pluginAssetsPath, cp.Options{OnDirExists: func(_, _ string) cp.DirExistsAction { return cp.Merge }}) if err != nil { diff --git a/plugins/builder/plugin.go b/plugins/builder/plugin.go index f69f7bb..68c4a9a 100644 --- a/plugins/builder/plugin.go +++ b/plugins/builder/plugin.go @@ -13,14 +13,14 @@ import ( ) func init() { - launchr.RegisterPlugin(&Plugin{}) + launchr.RegisterPlugin(Plugin{}) } // Plugin is a plugin to build launchr application. type Plugin struct{} // PluginInfo implements launchr.Plugin interface. -func (p *Plugin) PluginInfo() launchr.PluginInfo { +func (p Plugin) PluginInfo() launchr.PluginInfo { return launchr.PluginInfo{} } @@ -36,14 +36,14 @@ type builderInput struct { } // CobraAddCommands implements launchr.CobraPlugin interface to provide build functionality. -func (p *Plugin) CobraAddCommands(rootCmd *cobra.Command) error { +func (p Plugin) CobraAddCommands(rootCmd *cobra.Command) error { // Flag options. flags := builderInput{} - var buildCmd = &cobra.Command{ + buildCmd := &cobra.Command{ Use: "build", Short: "Rebuilds application with specified configuration", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { // Don't show usage help on a runtime error. cmd.SilenceUsage = true return Execute(cmd.Context(), &flags) diff --git a/plugins/default.go b/plugins/default.go index 2182d65..f969462 100644 --- a/plugins/default.go +++ b/plugins/default.go @@ -3,6 +3,7 @@ package plugins import ( // Default launchr plugins to include for launchr functionality. + _ "github.com/launchrctl/launchr/plugins/actionnaming" _ "github.com/launchrctl/launchr/plugins/builder" _ "github.com/launchrctl/launchr/plugins/verbosity" _ "github.com/launchrctl/launchr/plugins/yamldiscovery" diff --git a/plugins/verbosity/plugin.go b/plugins/verbosity/plugin.go index 1cf5bdb..7db109a 100644 --- a/plugins/verbosity/plugin.go +++ b/plugins/verbosity/plugin.go @@ -18,17 +18,18 @@ func init() { type Plugin struct{} // PluginInfo implements launchr.Plugin interface. -func (p *Plugin) PluginInfo() launchr.PluginInfo { +func (p Plugin) PluginInfo() launchr.PluginInfo { return launchr.PluginInfo{} } // CobraAddCommands implements launchr.CobraPlugin interface to set app verbosity. -func (p *Plugin) CobraAddCommands(rootCmd *cobra.Command) error { - var verbosity = 0 - var quiet = false +func (p Plugin) CobraAddCommands(rootCmd *cobra.Command) error { + verbosity := 0 + quiet := false rootCmd.PersistentFlags().CountVarP(&verbosity, "verbose", "v", "log verbosity level, use -vvv DEBUG, -vv WARN, -v INFO") rootCmd.PersistentFlags().BoolVarP(&quiet, "quiet", "q", false, "disable stdOut") - rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + rootCmd.PersistentPreRunE = func(_ *cobra.Command, _ []string) error { + // @todo logger is not set on preflight like "--help", but needed if fails to boot for debugging. if quiet { // @todo it doesn't really work for cli and docker output, only for logging. return nil diff --git a/plugins/yamldiscovery/embed/tar.go b/plugins/yamldiscovery/embed/tar.go index 039b232..ac3af1c 100644 --- a/plugins/yamldiscovery/embed/tar.go +++ b/plugins/yamldiscovery/embed/tar.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "context" "io" "io/fs" "os" @@ -19,7 +20,7 @@ func createActionTar(cfs fs.FS, buildPath string) (string, []*action.Action, err target := filepath.Join(buildPath, name) // Discover actions. ad := action.NewYamlDiscovery(action.NewDiscoveryFS(cfs, "")) - actions, err := ad.Discover() + actions, err := ad.Discover(context.Background()) if err != nil { return name, nil, err } diff --git a/plugins/yamldiscovery/plugin.go b/plugins/yamldiscovery/plugin.go index 8e9651a..a5e912e 100644 --- a/plugins/yamldiscovery/plugin.go +++ b/plugins/yamldiscovery/plugin.go @@ -3,65 +3,35 @@ package yamldiscovery import ( - "github.com/spf13/cobra" + "context" "github.com/launchrctl/launchr/internal/launchr" "github.com/launchrctl/launchr/pkg/action" - "github.com/launchrctl/launchr/pkg/cli" ) func init() { - launchr.RegisterPlugin(&Plugin{}) + launchr.RegisterPlugin(Plugin{}) } // Plugin is a plugin to discover actions defined in yaml. -type Plugin struct { - app launchr.App -} +type Plugin struct{} // PluginInfo implements launchr.Plugin interface. -func (p *Plugin) PluginInfo() launchr.PluginInfo { +func (p Plugin) PluginInfo() launchr.PluginInfo { return launchr.PluginInfo{} } // OnAppInit implements launchr.Plugin interface to provide discovered actions. -func (p *Plugin) OnAppInit(app launchr.App) error { - p.app = app +func (p Plugin) OnAppInit(_ launchr.App) error { return nil } -// DiscoverActions implements launchr.ActionDiscoveryPlugin interface. -func (p *Plugin) DiscoverActions(fs launchr.ManagedFS) ([]*action.Action, error) { +// DiscoverActions implements action.DiscoveryPlugin interface. +func (p Plugin) DiscoverActions(ctx context.Context, fs launchr.ManagedFS, idp action.IDProvider) ([]*action.Action, error) { if fs, ok := fs.(action.DiscoveryFS); ok { - return action.NewYamlDiscovery(fs).Discover() + d := action.NewYamlDiscovery(fs) + d.SetActionIDProvider(idp) + return d.Discover(ctx) } return nil, nil } - -// CobraAddCommands implements launchr.CobraPlugin interface to provide discovered actions. -func (p *Plugin) CobraAddCommands(rootCmd *cobra.Command) error { - var discoverCmd = &cobra.Command{ - Use: "discover", - Short: "Discovers available actions in filesystem", - RunE: func(cmd *cobra.Command, args []string) error { - var actions []*action.Action - for _, fs := range p.app.GetRegisteredFS() { - res, err := p.DiscoverActions(fs) - if err != nil { - return err - } - actions = append(actions, res...) - } - - // @todo cache discovery to read fs only once. - for _, a := range actions { - cli.Println("%s", a.ID) - } - - return nil - }, - } - // Discover actions. - rootCmd.AddCommand(discoverCmd) - return nil -} diff --git a/types.go b/types.go index 571ec17..0e675ab 100644 --- a/types.go +++ b/types.go @@ -34,6 +34,8 @@ type ( OnAppInitPlugin = launchr.OnAppInitPlugin // ActionDiscoveryPlugin is an interface to implement a plugin to discover actions. ActionDiscoveryPlugin = action.DiscoveryPlugin + // ActionsAlterPlugin is in interface to implement a plugin to alter registered actions. + ActionsAlterPlugin = action.AlterActionsPlugin // CobraPlugin is an interface to implement a plugin for cobra. CobraPlugin = launchr.CobraPlugin // PluginGeneratedData is a struct containing a result information of plugin generation. From fc5c47da54f6cb83b210bba388c8403c77c76393 Mon Sep 17 00:00:00 2001 From: Aleksandr Britvin Date: Wed, 21 Aug 2024 18:14:41 +0200 Subject: [PATCH 2/3] #63: Fix incorrect discovering and parsing when in root or home directory. Update linter. --- app.go | 14 ++++++++------ pkg/action/discover.go | 3 +++ pkg/action/manager.go | 32 +++++++++++++++++++++++++------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/app.go b/app.go index 09c40f4..10c5059 100644 --- a/app.go +++ b/app.go @@ -38,7 +38,7 @@ type appImpl struct { workDir string cfgDir string services map[ServiceInfo]Service - actionMngr action.ManagerUnsafe + actionMngr action.Manager pluginMngr PluginManager config Config mFS []ManagedFS @@ -205,7 +205,7 @@ func (app *appImpl) init() error { action.WithDefaultRunEnvironment, action.WithContainerRunEnvironmentConfig(app.config, name+"_"), action.WithValueProcessors(), - ).(action.ManagerUnsafe) // Ensure we can use unsafe functionality. + ) // Register services for other modules. app.AddService(app.actionMngr) @@ -270,13 +270,12 @@ func (app *appImpl) exec() error { if app.skipActions { app.cmd.SetVersionTemplate(Version().Full()) } - // Convert actions to cobra commands. - actions := app.actionMngr.AllUnsafe() // Check the requested command to see what actions we must actually load. + var actions map[string]*action.Action if app.reqCmd != "" { // Check if an alias was provided to find the real action. app.reqCmd = app.actionMngr.GetIDFromAlias(app.reqCmd) - a, ok := actions[app.reqCmd] + a, ok := app.actionMngr.Get(app.reqCmd) if ok { // Use only the requested action. actions = map[string]*action.Action{a.ID: a} @@ -284,14 +283,17 @@ func (app *appImpl) exec() error { // Action was not requested, no need to load them. app.skipActions = true } + } else { + // Load all. + actions = app.actionMngr.All() } + // Convert actions to cobra commands. // @todo consider cobra completion and caching between runs. if !app.skipActions { if len(actions) > 0 { app.cmd.AddGroup(ActionsGroup) } for _, a := range actions { - a, _ = app.actionMngr.Get(a.ID) cmd, err := action.CobraImpl(a, app.Streams()) if err != nil { fmt.Fprintf(os.Stdout, "[WARNING] Action %q was skipped:\n%v\n", a.ID, err) diff --git a/pkg/action/discover.go b/pkg/action/discover.go index a70256f..187d14b 100644 --- a/pkg/action/discover.go +++ b/pkg/action/discover.go @@ -60,6 +60,8 @@ type DiscoveryStrategy interface { } // IDProvider provides an ID for an action. +// It is used to generate an ID from an action declaration. +// DefaultIDProvider is the default implementation based on action filepath. type IDProvider interface { GetID(a *Action) string } @@ -206,6 +208,7 @@ func (ad *Discovery) SetActionIDProvider(idp IDProvider) { } // DefaultIDProvider is a default action id provider. +// It generates action id by a filepath. type DefaultIDProvider struct{} // GetID implements IDProvider interface. diff --git a/pkg/action/manager.go b/pkg/action/manager.go index f7b326c..e569176 100644 --- a/pkg/action/manager.go +++ b/pkg/action/manager.go @@ -16,8 +16,14 @@ type Manager interface { launchr.Service // All returns all actions copied and decorated. All() map[string]*Action + // AllRef returns all original action values from the storage. + // Deprecated: use ManagerUnsafe.AllUnsafe instead. + AllRef() map[string]*Action // Get returns a copy of an action from the manager with default decorators. Get(id string) (*Action, bool) + // GetRef returns an original action value from the storage. + // Deprecated: use ManagerUnsafe.GetUnsafe instead. + GetRef(id string) (*Action, bool) // Add saves an action in the manager. Add(*Action) // Delete deletes the action from the manager. @@ -31,6 +37,7 @@ type Manager interface { // GetActionIDProvider returns global application action id provider. GetActionIDProvider() IDProvider // SetActionIDProvider sets global application action id provider. + // This id provider will be used as default on Action discovery process. SetActionIDProvider(p IDProvider) // AddValueProcessor adds processor to list of available processors @@ -50,16 +57,18 @@ type Manager interface { RunInfoByID(id string) (RunInfo, bool) } -// ManagerUnsafe is an extension of the Manager interface for unsafe access to actions. -// Consider twice before using it! +// ManagerUnsafe is an extension of the Manager interface that provides unsafe access to actions. +// Warning: Use this with caution! type ManagerUnsafe interface { Manager // AllUnsafe returns all original action values from the storage. - // Use it only if you need to read-only actions without allocations. - // It is unsafe to operate the actions as they are the original and affect the whole app. - // If you need to run actions, use Get or All, it will provide configured for run Action. + // Use this method only if you need read-only access to the actions without allocating new memory. + // Warning: It is unsafe to manipulate these actions directly as they are the original instances + // affecting the entire application. + // Normally, for action execution you should use the Manager.Get or Manager.All methods, + // which provide actions configured for execution. AllUnsafe() map[string]*Action - // GetUnsafe returns an original action value from the storage. + // GetUnsafe returns the original action value from the storage. GetUnsafe(id string) (*Action, bool) } @@ -118,6 +127,11 @@ func (m *actionManagerMap) AllUnsafe() map[string]*Action { return copyMap(m.actionStore) } +// Deprecated: use AllUnsafe instead. +func (m *actionManagerMap) AllRef() map[string]*Action { + return m.AllUnsafe() +} + func (m *actionManagerMap) GetIDFromAlias(alias string) string { if id, ok := m.actionAliases[alias]; ok { return id @@ -161,11 +175,15 @@ func (m *actionManagerMap) GetUnsafe(id string) (*Action, bool) { return a, ok } +// Deprecated: use GetUnsafe instead. +func (m *actionManagerMap) GetRef(id string) (*Action, bool) { + return m.GetUnsafe(id) +} + func (m *actionManagerMap) AddValueProcessor(name string, vp ValueProcessor) { if _, ok := m.processors[name]; ok { panic(fmt.Sprintf("processor `%q` with the same name already exists", name)) } - m.processors[name] = vp } From 2428b70b3aedb15fa3756fc9ea2c691f92c78936 Mon Sep 17 00:00:00 2001 From: Aleksandr Britvin Date: Thu, 22 Aug 2024 13:00:18 +0200 Subject: [PATCH 3/3] #63: Fix incorrect discovering and parsing when in root or home directory. Update linter. --- pkg/action/discover.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/discover.go b/pkg/action/discover.go index 187d14b..002b1ea 100644 --- a/pkg/action/discover.go +++ b/pkg/action/discover.go @@ -123,7 +123,7 @@ func (ad *Discovery) findFiles(ctx context.Context) chan string { } // Skip OS specific directories to prevent going too deep. // Skip hidden directories. - if d.IsDir() && (isHiddenPath(path) || skipSystemDirs(ad.fsDir, path)) { + if d != nil && d.IsDir() && (isHiddenPath(path) || skipSystemDirs(ad.fsDir, path)) { return fs.SkipDir } if err != nil {