From 8cdc9348d74f9729426a842adfeb61ebec81cb3b Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Mon, 13 Jun 2022 11:03:22 +0200 Subject: [PATCH] Substituting variables into the devfile from the CLI (#5749) * Define var and and-file flags * vars package * Vars for deploy * Add integration tests for dev * Update dev mock * New devfile library version * Add doc * Fix validate * Review * Apply suggestions from code review Co-authored-by: Parthvi Vala * review Co-authored-by: Parthvi Vala --- .../version-3.0.0/command-reference/deploy.md | 7 + .../version-3.0.0/command-reference/dev.md | 49 +++ go.mod | 4 +- go.sum | 4 + pkg/dev/dev.go | 3 +- pkg/dev/interface.go | 2 +- pkg/dev/mock.go | 8 +- pkg/devfile/devfile.go | 10 + pkg/odo/cli/deploy/deploy.go | 19 +- pkg/odo/cli/dev/dev.go | 22 +- pkg/odo/genericclioptions/context.go | 9 +- pkg/vars/doc.go | 2 + pkg/vars/errors.go | 15 + pkg/vars/vars.go | 113 ++++++ pkg/vars/vars_test.go | 363 ++++++++++++++++++ pkg/watch/watch.go | 2 + tests/integration/devfile/cmd_dev_test.go | 67 ++++ .../workspaces/v1alpha2/import_reference.go | 8 + .../v1alpha2/zz_generated.parent_overrides.go | 8 + .../devfile/library/pkg/devfile/parse.go | 10 + .../data/v2/2.2.0/devfileJsonSchema220.go | 5 + .../library/pkg/devfile/parser/parse.go | 17 +- .../devfile/library/pkg/util/util.go | 3 - vendor/modules.txt | 4 +- 24 files changed, 726 insertions(+), 28 deletions(-) create mode 100644 pkg/vars/doc.go create mode 100644 pkg/vars/errors.go create mode 100644 pkg/vars/vars.go create mode 100644 pkg/vars/vars_test.go diff --git a/docs/website/versioned_docs/version-3.0.0/command-reference/deploy.md b/docs/website/versioned_docs/version-3.0.0/command-reference/deploy.md index 287b2a65fd6..488f74907f9 100644 --- a/docs/website/versioned_docs/version-3.0.0/command-reference/deploy.md +++ b/docs/website/versioned_docs/version-3.0.0/command-reference/deploy.md @@ -64,3 +64,10 @@ components: - name: main image: {{CONTAINER_IMAGE}} ``` + +## Sustituting variables + +The Devfile can define variables to make the Devfile parameterizable. The Devfile can define values for these variables, and you +can override the values for variables from the command line when running `odo deploy`, using the `--var` and `--var-file` options. + +See [Sustituting variables in odo dev](dev.md#sustituting-variables) for more information. diff --git a/docs/website/versioned_docs/version-3.0.0/command-reference/dev.md b/docs/website/versioned_docs/version-3.0.0/command-reference/dev.md index 3ebb7c19b75..dd930c6882a 100644 --- a/docs/website/versioned_docs/version-3.0.0/command-reference/dev.md +++ b/docs/website/versioned_docs/version-3.0.0/command-reference/dev.md @@ -43,6 +43,55 @@ In the above example, three things have happened: You can press Ctrl-c at any time to terminate the development session. The command can take a few moment to terminate, as it will first delete all resources deployed into the cluster for this session before terminating. +### Sustituting variables + +The Devfile can define variables to make the Devfile parameterizable. The Devfile can define values for these variables, and you +can override the values for variables from the command line when running `odo dev`, using the `--var` and `--var-file` options. + +The `--var` option is a repeatable option that takes a `variable=value` pair, where `=value` is optional. If the `=value` is omitted, the value is extracted from the environment variable named `variable`. In this case, if the environment variable with this name is not defined, the value defined into the Devfile will be used. + +The `--var-file` option takes a filename as argument. The file contains a line-separated list of `variable=value` pairs, with the same behaviour as before. + +Note that the values passed with the `--var` option overrides the values obtained with the `--var-file` option. + +#### Examples + +Considering the Devfile contains this `variables` field: + +``` +variables: + USER: anonymous + DEBUG: false +``` + + +This command will override the `USER` Devfile variable with the value of the `USER` environment variable, if it is defined. +It will also override the value of the `DEBUG` Devfile variable with the `true` value. + +```shell +$ odo dev --var USER --var DEBUG=true +``` + +If you create a file `config.vars` containing: + +``` +USER +DEBUG=true +``` + +The following command will have the same behaviour as the previous one: + +```shell +$ odo dev --var-file config.vars +``` + +The following command will override the `USER` Devfile variable with the `john` value: + + +```shell +$ odo dev --var USER=john --var-file config.vars +``` + ## Devfile (Advanced Usage) ### Devfile Overview diff --git a/go.mod b/go.mod index 4a444ad2f53..ebac6ce5ac4 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,8 @@ require ( github.com/Netflix/go-expect v0.0.0-20201125194554-85d881c3777e github.com/Xuanwo/go-locale v1.0.0 github.com/blang/semver v3.5.1+incompatible - github.com/devfile/api/v2 v2.0.0-20220117162434-6e6e6a8bc14c - github.com/devfile/library v1.2.1-0.20220217161036-0f5995513e92 + github.com/devfile/api/v2 v2.0.0-20220309195345-48ebbf1e51cf + github.com/devfile/library v1.2.1-0.20220602130922-85a4805bd59c github.com/devfile/registry-support/index/generator v0.0.0-20220222194908-7a90a4214f3e github.com/devfile/registry-support/registry-library v0.0.0-20220504150710-21de53798172 github.com/fatih/color v1.13.0 diff --git a/go.sum b/go.sum index ba560cee6b4..c0a746838c6 100644 --- a/go.sum +++ b/go.sum @@ -353,10 +353,14 @@ github.com/devfile/api/v2 v2.0.0-20210910153124-da620cd1a7a1/go.mod h1:kLX/nW93g github.com/devfile/api/v2 v2.0.0-20211021164004-dabee4e633ed/go.mod h1:d99eTN6QxgzihOOFyOZA+VpUyD4Q1pYRYHZ/ci9J96Q= github.com/devfile/api/v2 v2.0.0-20220117162434-6e6e6a8bc14c h1:sjghKUov/WT71dBreHYQcTgQctxHT/p4uAhUFdGQfSU= github.com/devfile/api/v2 v2.0.0-20220117162434-6e6e6a8bc14c/go.mod h1:d99eTN6QxgzihOOFyOZA+VpUyD4Q1pYRYHZ/ci9J96Q= +github.com/devfile/api/v2 v2.0.0-20220309195345-48ebbf1e51cf h1:FkwAOQtepscB5B0j++9S/eoicXj707MaP5HPIScz0sA= +github.com/devfile/api/v2 v2.0.0-20220309195345-48ebbf1e51cf/go.mod h1:kLX/nW93gigOHXK3NLeJL2fSS/sgEe+OHu8bo3aoOi4= github.com/devfile/library v1.1.1-0.20210910214722-7c5ff63711ec/go.mod h1:svPWwWb+BP15SXCHl0dyOeE4Sohrjl5a2BaOzc/riLc= github.com/devfile/library v1.2.1-0.20211104222135-49d635cb492f/go.mod h1:uFZZdTuRqA68FVe/JoJHP92CgINyQkyWnM2Qyiim+50= github.com/devfile/library v1.2.1-0.20220217161036-0f5995513e92 h1:RIrd0pBAET9vLHEZGyaG1PSjp/lJXa+CZfuWiih2p6g= github.com/devfile/library v1.2.1-0.20220217161036-0f5995513e92/go.mod h1:GSPfJaBg0+bBjBHbwBE5aerJLH6tWGQu2q2rHYd9czM= +github.com/devfile/library v1.2.1-0.20220602130922-85a4805bd59c h1:Puqg/NJ6pP1LPFxPYBX9c0Oh0Ttu5PVdNJL5WR/CRTk= +github.com/devfile/library v1.2.1-0.20220602130922-85a4805bd59c/go.mod h1:kpkIA9YI7gkfglRDsKJ3n5QgVyyBmnasr2U2djYwg6s= github.com/devfile/registry-support/index/generator v0.0.0-20211012185733-0a73f866043f h1:fKNUmoOPh7yAs69uMRZWHvev+m3e7T4jBL/hOXZB9ys= github.com/devfile/registry-support/index/generator v0.0.0-20211012185733-0a73f866043f/go.mod h1:bLGagbW2SFn7jo5+kUPlCMehIGqWkRtLKc5O0OyJMJM= github.com/devfile/registry-support/index/generator v0.0.0-20220222194908-7a90a4214f3e h1:3WAjUoyAmCBzUpx+sO0dSgkH74uSW1y886wGbojD2D8= diff --git a/pkg/dev/dev.go b/pkg/dev/dev.go index 54bb925a758..b27b6ef2302 100644 --- a/pkg/dev/dev.go +++ b/pkg/dev/dev.go @@ -56,7 +56,7 @@ func (o *DevClient) Start(devfileObj parser.DevfileObj, platformContext kubernet return nil } -func (o *DevClient) Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool) error { +func (o *DevClient) Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool, variables map[string]string) error { envSpecificInfo, err := envinfo.NewEnvSpecificInfo(path) if err != nil { return err @@ -72,6 +72,7 @@ func (o *DevClient) Watch(devfileObj parser.DevfileObj, path string, ignorePaths InitialDevfileObj: devfileObj, Debug: debug, DebugPort: envSpecificInfo.GetDebugPort(), + Variables: variables, } return o.watchClient.WatchAndPush(out, watchParameters, ctx) diff --git a/pkg/dev/interface.go b/pkg/dev/interface.go index c0c244c77e2..134683faa11 100644 --- a/pkg/dev/interface.go +++ b/pkg/dev/interface.go @@ -20,7 +20,7 @@ type Client interface { // It logs messages to out and uses the Handler h to perform push operation when anything changes in path. // It uses devfileObj to notify user to restart odo dev if they change endpoint information in the devfile. // If debug is true, the debug command will be started after a sync, or the run command by default - Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool) error + Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool, variables map[string]string) error } type Handler interface { diff --git a/pkg/dev/mock.go b/pkg/dev/mock.go index 5279e2f2700..a062816e445 100644 --- a/pkg/dev/mock.go +++ b/pkg/dev/mock.go @@ -54,17 +54,17 @@ func (mr *MockClientMockRecorder) Start(devfileObj, platformContext, ignorePaths } // Watch mocks base method. -func (m *MockClient) Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool) error { +func (m *MockClient) Watch(devfileObj parser.DevfileObj, path string, ignorePaths []string, out io.Writer, h Handler, ctx context.Context, debug bool, variables map[string]string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Watch", devfileObj, path, ignorePaths, out, h, ctx, debug) + ret := m.ctrl.Call(m, "Watch", devfileObj, path, ignorePaths, out, h, ctx, debug, variables) ret0, _ := ret[0].(error) return ret0 } // Watch indicates an expected call of Watch. -func (mr *MockClientMockRecorder) Watch(devfileObj, path, ignorePaths, out, h, ctx, debug interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) Watch(devfileObj, path, ignorePaths, out, h, ctx, debug, variables interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Watch", reflect.TypeOf((*MockClient)(nil).Watch), devfileObj, path, ignorePaths, out, h, ctx, debug) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Watch", reflect.TypeOf((*MockClient)(nil).Watch), devfileObj, path, ignorePaths, out, h, ctx, debug, variables) } // MockHandler is a mock of Handler interface. diff --git a/pkg/devfile/devfile.go b/pkg/devfile/devfile.go index 9fddeb5ced8..c00963465b5 100644 --- a/pkg/devfile/devfile.go +++ b/pkg/devfile/devfile.go @@ -47,6 +47,16 @@ func ParseAndValidateFromFile(devfilePath string) (parser.DevfileObj, error) { return parseDevfile(parser.ParserArgs{Path: devfilePath}) } +// ParseAndValidateFromFileWithVariables reads, parses and validates devfile from a file +// variables are used to override devfile variables +// if there are warning it logs them on stdout +func ParseAndValidateFromFileWithVariables(devfilePath string, variables map[string]string) (parser.DevfileObj, error) { + return parseDevfile(parser.ParserArgs{ + Path: devfilePath, + ExternalVariables: variables, + }) +} + func variableWarning(section string, variable string, messages []string) string { quotedVars := []string{} for _, v := range messages { diff --git a/pkg/odo/cli/deploy/deploy.go b/pkg/odo/cli/deploy/deploy.go index 816f60587e1..28d853c1389 100644 --- a/pkg/odo/cli/deploy/deploy.go +++ b/pkg/odo/cli/deploy/deploy.go @@ -16,6 +16,7 @@ import ( "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" odoutil "github.com/redhat-developer/odo/pkg/odo/util" scontext "github.com/redhat-developer/odo/pkg/segment/context" + "github.com/redhat-developer/odo/pkg/vars" "github.com/redhat-developer/odo/pkg/version" "os" @@ -36,6 +37,13 @@ type DeployOptions struct { // Clients clientset *clientset.Clientset + // Flags + varFileFlag string + varsFlag []string + + // Variables to override Devfile variables + variables map[string]string + // working directory contextDir string } @@ -85,7 +93,12 @@ func (o *DeployOptions) Complete(cmdline cmdline.Cmdline, args []string) (err er return err } - o.Context, err = genericclioptions.New(genericclioptions.NewCreateParameters(cmdline).NeedDevfile(o.contextDir).CreateAppIfNeeded()) + o.variables, err = vars.GetVariables(o.clientset.FS, o.varFileFlag, o.varsFlag, os.LookupEnv) + if err != nil { + return err + } + + o.Context, err = genericclioptions.New(genericclioptions.NewCreateParameters(cmdline).NeedDevfile(o.contextDir).WithVariables(o.variables).CreateAppIfNeeded()) if err != nil { return err } @@ -161,7 +174,9 @@ func NewCmdDeploy(name, fullName string) *cobra.Command { genericclioptions.GenericRun(o, cmd, args) }, } - clientset.Add(deployCmd, clientset.INIT, clientset.DEPLOY) + deployCmd.Flags().StringArrayVar(&o.varsFlag, "var", []string{}, "Variable to override Devfile variable and variables in var-file") + deployCmd.Flags().StringVar(&o.varFileFlag, "var-file", "", "File containing variables to override Devfile variables") + clientset.Add(deployCmd, clientset.INIT, clientset.DEPLOY, clientset.FILESYSTEM) // Add a defined annotation in order to appear in the help menu deployCmd.Annotations["command"] = "main" diff --git a/pkg/odo/cli/dev/dev.go b/pkg/odo/cli/dev/dev.go index 92d0ede14d3..495c10410a2 100644 --- a/pkg/odo/cli/dev/dev.go +++ b/pkg/odo/cli/dev/dev.go @@ -32,6 +32,7 @@ import ( odoutil "github.com/redhat-developer/odo/pkg/odo/util" scontext "github.com/redhat-developer/odo/pkg/segment/context" "github.com/redhat-developer/odo/pkg/util" + "github.com/redhat-developer/odo/pkg/vars" "github.com/redhat-developer/odo/pkg/version" "github.com/redhat-developer/odo/pkg/watch" ) @@ -65,6 +66,11 @@ type DevOptions struct { noWatchFlag bool randomPortsFlag bool debugFlag bool + varFileFlag string + varsFlag []string + + // Variables to override Devfile variables + variables map[string]string } type Handler struct{} @@ -122,7 +128,12 @@ func (o *DevOptions) Complete(cmdline cmdline.Cmdline, args []string) error { return err } - o.Context, err = genericclioptions.New(genericclioptions.NewCreateParameters(cmdline).NeedDevfile("")) + o.variables, err = vars.GetVariables(o.clientset.FS, o.varFileFlag, o.varsFlag, os.LookupEnv) + if err != nil { + return err + } + + o.Context, err = genericclioptions.New(genericclioptions.NewCreateParameters(cmdline).NeedDevfile("").WithVariables(o.variables)) if err != nil { return fmt.Errorf("unable to create context: %v", err) } @@ -257,7 +268,7 @@ func (o *DevOptions) Run(ctx context.Context) (err error) { err = o.clientset.WatchClient.CleanupDevResources(devFileObj, log.GetStdout()) } else { d := Handler{} - err = o.clientset.DevClient.Watch(devFileObj, path, o.ignorePaths, o.out, &d, o.ctx, o.debugFlag) + err = o.clientset.DevClient.Watch(devFileObj, path, o.ignorePaths, o.out, &d, o.ctx, o.debugFlag, o.variables) } return err } @@ -280,7 +291,7 @@ func (o *Handler) RegenerateAdapterAndPush(pushParams common.PushParameters, wat } func regenerateComponentAdapterFromWatchParams(parameters watch.WatchParameters) (common.ComponentAdapter, error) { - devObj, err := ododevfile.ParseAndValidateFromFile(location.DevfileLocation("")) + devObj, err := ododevfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), parameters.Variables) if err != nil { return nil, err } @@ -321,8 +332,9 @@ It forwards endpoints with exposure values 'public' or 'internal' to a port on l devCmd.Flags().BoolVar(&o.noWatchFlag, "no-watch", false, "Do not watch for file changes") devCmd.Flags().BoolVar(&o.randomPortsFlag, "random-ports", false, "Assign random ports to redirected ports") devCmd.Flags().BoolVar(&o.debugFlag, "debug", false, "Execute the debug command within the component") - - clientset.Add(devCmd, clientset.DEV, clientset.INIT, clientset.KUBERNETES, clientset.STATE) + devCmd.Flags().StringArrayVar(&o.varsFlag, "var", []string{}, "Variable to override Devfile variable and variables in var-file") + devCmd.Flags().StringVar(&o.varFileFlag, "var-file", "", "File containing variables to override Devfile variables") + clientset.Add(devCmd, clientset.DEV, clientset.INIT, clientset.KUBERNETES, clientset.STATE, clientset.FILESYSTEM) // Add a defined annotation in order to appear in the help menu devCmd.Annotations["command"] = "main" devCmd.SetUsageTemplate(odoutil.CmdUsageTemplate) diff --git a/pkg/odo/genericclioptions/context.go b/pkg/odo/genericclioptions/context.go index f745f5dc732..9add468ec64 100644 --- a/pkg/odo/genericclioptions/context.go +++ b/pkg/odo/genericclioptions/context.go @@ -60,6 +60,8 @@ type CreateParameters struct { devfile bool offline bool appIfNeeded bool + // variables override devfile variables + variables map[string]string } func NewCreateParameters(cmdline cmdline.Cmdline) CreateParameters { @@ -82,6 +84,11 @@ func (o CreateParameters) CreateAppIfNeeded() CreateParameters { return o } +func (o CreateParameters) WithVariables(variables map[string]string) CreateParameters { + o.variables = variables + return o +} + // New creates a context based on the given parameters func New(parameters CreateParameters) (*Context, error) { ctx := internalCxt{} @@ -124,7 +131,7 @@ func New(parameters CreateParameters) (*Context, error) { isDevfile := odoutil.CheckPathExists(ctx.devfilePath) if isDevfile { // Parse devfile and validate - devObj, err := devfile.ParseAndValidateFromFile(ctx.devfilePath) + devObj, err := devfile.ParseAndValidateFromFileWithVariables(ctx.devfilePath, parameters.variables) if err != nil { return nil, fmt.Errorf("failed to parse the devfile %s, with error: %s", ctx.devfilePath, err) } diff --git a/pkg/vars/doc.go b/pkg/vars/doc.go new file mode 100644 index 00000000000..7e5fefd70b8 --- /dev/null +++ b/pkg/vars/doc.go @@ -0,0 +1,2 @@ +// The var package analyzes variables defined in env file and as flags +package vars diff --git a/pkg/vars/errors.go b/pkg/vars/errors.go new file mode 100644 index 00000000000..99b0ca20b4d --- /dev/null +++ b/pkg/vars/errors.go @@ -0,0 +1,15 @@ +package vars + +import "fmt" + +type ErrBadKey struct { + msg string +} + +func NewErrBadKey(msg string) ErrBadKey { + return ErrBadKey{msg: msg} +} + +func (e ErrBadKey) Error() string { + return fmt.Sprintf("poorly formatted environment: %s", e.msg) +} diff --git a/pkg/vars/vars.go b/pkg/vars/vars.go new file mode 100644 index 00000000000..2122a8c8bab --- /dev/null +++ b/pkg/vars/vars.go @@ -0,0 +1,113 @@ +package vars + +import ( + "bufio" + "fmt" + "strings" + "unicode" + + "github.com/redhat-developer/odo/pkg/testingutil/filesystem" +) + +// GetVariables returns a map of key/value from pairs defined in the file and in the list of strings in the format KEY=VALUE or KEY +// For a KEY entry, the value will be obtained from the environment, and if the value is not defined in the environment, the entry KEY will be ignored +// An empty filename will skip the extraction of pairs from file +func GetVariables(fs filesystem.Filesystem, filename string, override []string, lookupEnv func(string) (string, bool)) (map[string]string, error) { + + result := map[string]string{} + var err error + if len(filename) > 0 { + result, err = parseKeyValueFile(fs, filename, lookupEnv) + if err != nil { + return nil, err + } + } + overrideVars, err := parseKeyValueStrings(override, lookupEnv) + if err != nil { + return nil, err + } + + for k, v := range overrideVars { + result[k] = v + } + + return result, nil +} + +// parseKeyValueFile parses a file for "KEY=VALUE" lines and returns a map of keys/values +// If a key is defined without a value as "KEY", the value is searched into the environment with lookupEnv function +// Note that "KEY=" defines an empty value for KEY, but "KEY" indicates to search for value in environment +// If the KEY environment variable is not defined, this entry will be skipped +func parseKeyValueFile(fs filesystem.Filesystem, filename string, lookupEnv func(string) (string, bool)) (map[string]string, error) { + f, err := fs.Open(filename) + if err != nil { + return nil, err + } + defer f.Close() + + result := map[string]string{} + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + scannedText := scanner.Text() + + key, value, err := parseKeyValueString(scannedText, lookupEnv) + if err != nil { + return nil, err + } + if len(key) == 0 { + continue + } + result[key] = value + } + + return result, nil +} + +func parseKeyValueStrings(strs []string, lookupEnv func(string) (string, bool)) (map[string]string, error) { + result := map[string]string{} + + for _, str := range strs { + key, value, err := parseKeyValueString(str, lookupEnv) + if err != nil { + return nil, err + } + if len(key) == 0 { + continue + } + result[key] = value + } + return result, nil +} + +// parseKeyValueString parses a string to extract a key and its associated value +// if a line is empty or a comment, a nil error and an empty key are returned +// if a key does not define a value, the value will be obtained from the environment +// in this case, if the environment does not define the variable, the entry will be ignored +func parseKeyValueString(s string, lookupEnv func(string) (string, bool)) (string, string, error) { + line := strings.TrimLeftFunc(s, unicode.IsSpace) + if len(line) == 0 || strings.HasPrefix(line, "#") { + return "", "", nil + } + parts := strings.SplitN(line, "=", 2) + key := parts[0] + + // TODO validate key format + + if len(key) == 0 { + return "", "", NewErrBadKey(fmt.Sprintf("no key defined in line %q", s)) + } + + var value string + if len(parts) > 1 { + value = parts[1] + } else { + var found bool + value, found = lookupEnv(key) + if !found { + return "", "", nil + } + } + + return key, value, nil +} diff --git a/pkg/vars/vars_test.go b/pkg/vars/vars_test.go new file mode 100644 index 00000000000..e59113e1fae --- /dev/null +++ b/pkg/vars/vars_test.go @@ -0,0 +1,363 @@ +package vars + +import ( + "reflect" + "testing" + + "github.com/redhat-developer/odo/pkg/testingutil/filesystem" +) + +func Test_parseKeyValueFile(t *testing.T) { + type args struct { + fileContent string + lookupEnv func(string) (string, bool) + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "no error", + args: args{ + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + fileContent: `A=aze +# a comment + +B=zerty +# a line beginning with spaces + C=cvb +# a value beginning and ending with spaces + D= dfg + +# an empty value +E= + +# a key with no value +F + +# not defined in environment +G +`, + }, + want: map[string]string{ + "A": "aze", + "B": "zerty", + "C": "cvb", + "D": " dfg ", + "E": "", + "F": "a value for F from env", + }, + }, + { + name: "works with Windows EOL", + args: args{ + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + fileContent: "A=aze\r\nB=qsd", + }, + want: map[string]string{ + "A": "aze", + "B": "qsd", + }, + }, + { + name: "line without key", + args: args{ + fileContent: `# a comment +=aze`, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filename := "vars.txt" + fs := filesystem.NewFakeFs() + _ = fs.WriteFile(filename, []byte(tt.args.fileContent), 0444) + + got, err := parseKeyValueFile(fs, filename, tt.args.lookupEnv) + if (err != nil) != tt.wantErr { + t.Errorf("parseKeyValueFile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("parseKeyValueFile() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_parseKeyValueStrings(t *testing.T) { + type args struct { + strs []string + lookupEnv func(string) (string, bool) + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "no error", + args: args{ + strs: []string{ + "A=aze", + "# a comment", + "", + "B=zerty", + "# a line beginning with spaces", + " C=cvb", + "# a value beginning and ending with spaces", + " D= dfg ", + "# an empty value", + "E=", + "# a key with no value", + "F", + "# not defined in environment", + "G", + }, + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "aze", + "B": "zerty", + "C": "cvb", + "D": " dfg ", + "E": "", + "F": "a value for F from env", + }, + }, + { + name: "string without key", + args: args{ + strs: []string{ + "# a comment", + "=aze", + }, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseKeyValueStrings(tt.args.strs, tt.args.lookupEnv) + if (err != nil) != tt.wantErr { + t.Errorf("parseKeyValueStrings() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("parseKeyValueStrings() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGetVariables(t *testing.T) { + type args struct { + fileContent string + override []string + lookupEnv func(string) (string, bool) + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "overrides file", + args: args{ + fileContent: `A=aze`, + + override: []string{ + "A=qsd", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "qsd", + }, + }, + { + name: "no override between file and override", + args: args{ + fileContent: `A=aze`, + + override: []string{ + "B=qsd", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "aze", + "B": "qsd", + }, + }, + { + name: "override file with env var", + args: args{ + fileContent: `A=aze`, + + override: []string{ + "A", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "A": "a value for A from env", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "a value for A from env", + }, + }, + { + name: "no override file with not defined env var", + args: args{ + fileContent: `A=aze`, + + override: []string{ + "A", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{}[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "aze", + }, + }, + { + name: "override file with empty defined env var", + args: args{ + fileContent: `A=aze`, + + override: []string{ + "A", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "A": "", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "", + }, + }, + { + name: "error parsing file", + args: args{ + fileContent: `=aze`, + }, + wantErr: true, + }, + { + name: "error parsing override strings", + args: args{ + fileContent: `A=aze`, + override: []string{ + "=aze", + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filename := "vars.txt" + fs := filesystem.NewFakeFs() + _ = fs.WriteFile(filename, []byte(tt.args.fileContent), 0444) + + got, err := GetVariables(fs, filename, tt.args.override, tt.args.lookupEnv) + if (err != nil) != tt.wantErr { + t.Errorf("GetVariables() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetVariables() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGetVariablesEmptyFilename(t *testing.T) { + type args struct { + override []string + lookupEnv func(string) (string, bool) + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "get override value", + args: args{ + override: []string{ + "A=qsd", + }, + + lookupEnv: func(s string) (string, bool) { + res, ok := map[string]string{ + "F": "a value for F from env", + }[s] + return res, ok + }, + }, + want: map[string]string{ + "A": "qsd", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetVariables(filesystem.NewFakeFs(), "", tt.args.override, tt.args.lookupEnv) + if (err != nil) != tt.wantErr { + t.Errorf("GetVariables() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetVariables() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/watch/watch.go b/pkg/watch/watch.go index db3b765a246..618c0d1c83d 100644 --- a/pkg/watch/watch.go +++ b/pkg/watch/watch.go @@ -74,6 +74,8 @@ type WatchParameters struct { Debug bool // DebugPort indicates which debug port to use for pushing after sync DebugPort int + // Variables override Devfile variables + Variables map[string]string } // evaluateChangesFunc evaluates any file changes for the events by ignoring the files in fileIgnores slice and removes diff --git a/tests/integration/devfile/cmd_dev_test.go b/tests/integration/devfile/cmd_dev_test.go index 50dcfc9aa1f..0eb8dc127cc 100644 --- a/tests/integration/devfile/cmd_dev_test.go +++ b/tests/integration/devfile/cmd_dev_test.go @@ -489,6 +489,73 @@ var _ = Describe("odo dev command tests", func() { Expect(envVars["FOO"]).To(Equal("bar")) }) }) + + When("doing odo dev with --var flag", func() { + var session helper.DevSession + BeforeEach(func() { + var err error + session, _, _, _, err = helper.StartDevMode("--var", "VALUE_TEST=baz") + Expect(err).ToNot(HaveOccurred()) + }) + AfterEach(func() { + session.Stop() + session.WaitEnd() + }) + + It("should check if the env variable has a correct value", func() { + envVars := commonVar.CliRunner.GetEnvsDevFileDeployment(devfileCmpName, "app", commonVar.Project) + // check if the env variable has a correct value. This value was substituted from in devfile from variable + Expect(envVars["FOO"]).To(Equal("baz")) + }) + }) + + When("doing odo dev with --var-file flag", func() { + var session helper.DevSession + varfilename := "vars.txt" + BeforeEach(func() { + var err error + err = helper.CreateFileWithContent(varfilename, "VALUE_TEST=baz") + Expect(err).ToNot(HaveOccurred()) + session, _, _, _, err = helper.StartDevMode("--var-file", "vars.txt") + Expect(err).ToNot(HaveOccurred()) + }) + AfterEach(func() { + session.Stop() + session.WaitEnd() + helper.DeleteFile(varfilename) + }) + + It("should check if the env variable has a correct value", func() { + envVars := commonVar.CliRunner.GetEnvsDevFileDeployment(devfileCmpName, "app", commonVar.Project) + // check if the env variable has a correct value. This value was substituted from in devfile from variable + Expect(envVars["FOO"]).To(Equal("baz")) + }) + }) + + When("doing odo dev with --var-file flag and setting value in env", func() { + var session helper.DevSession + varfilename := "vars.txt" + BeforeEach(func() { + var err error + _ = os.Setenv("VALUE_TEST", "baz") + err = helper.CreateFileWithContent(varfilename, "VALUE_TEST") + Expect(err).ToNot(HaveOccurred()) + session, _, _, _, err = helper.StartDevMode("--var-file", "vars.txt") + Expect(err).ToNot(HaveOccurred()) + }) + AfterEach(func() { + session.Stop() + session.WaitEnd() + helper.DeleteFile(varfilename) + _ = os.Unsetenv("VALUE_TEST") + }) + + It("should check if the env variable has a correct value", func() { + envVars := commonVar.CliRunner.GetEnvsDevFileDeployment(devfileCmpName, "app", commonVar.Project) + // check if the env variable has a correct value. This value was substituted from in devfile from variable + Expect(envVars["FOO"]).To(Equal("baz")) + }) + }) }) When("running odo dev and single env var is set", func() { diff --git a/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/import_reference.go b/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/import_reference.go index 342b7975c19..65414c10aa9 100644 --- a/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/import_reference.go +++ b/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/import_reference.go @@ -50,4 +50,12 @@ type ImportReference struct { // it is recommended to always specify the `registryUrl` when `id` is used. // +optional RegistryUrl string `json:"registryUrl,omitempty"` + + // Specific stack/sample version to pull the parent devfile from, when using id in the parent reference. + // To specify `version`, `id` must be defined and used as the import reference source. + // `version` can be either a specific stack version, or `latest`. + // If no `version` specified, default version will be used. + // +optional + // +kubebuilder:validation:Pattern=^(latest)|(([1-9])\.([0-9]+)\.([0-9]+)(\-[0-9a-z-]+(\.[0-9a-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)$ + Version string `json:"version,omitempty"` } diff --git a/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/zz_generated.parent_overrides.go b/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/zz_generated.parent_overrides.go index 754777f1fe9..466f0fb2a4d 100644 --- a/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/zz_generated.parent_overrides.go +++ b/vendor/github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2/zz_generated.parent_overrides.go @@ -552,6 +552,14 @@ type ImportReferenceParentOverride struct { // it is recommended to always specify the `registryUrl` when `id` is used. // +optional RegistryUrl string `json:"registryUrl,omitempty"` + + // Specific stack/sample version to pull the parent devfile from, when using id in the parent reference. + // To specify `version`, `id` must be defined and used as the import reference source. + // `version` can be either a specific stack version, or `latest`. + // If no `version` specified, default version will be used. + // +optional + // +kubebuilder:validation:Pattern=^(latest)|(([1-9])\.([0-9]+)\.([0-9]+)(\-[0-9a-z-]+(\.[0-9a-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)$ + Version string `json:"version,omitempty"` } type PluginOverridesParentOverride struct { diff --git a/vendor/github.com/devfile/library/pkg/devfile/parse.go b/vendor/github.com/devfile/library/pkg/devfile/parse.go index 6eba767f78d..64c2890af11 100644 --- a/vendor/github.com/devfile/library/pkg/devfile/parse.go +++ b/vendor/github.com/devfile/library/pkg/devfile/parse.go @@ -80,6 +80,16 @@ func ParseDevfileAndValidate(args parser.ParserArgs) (d parser.DevfileObj, varWa } if d.Data.GetSchemaVersion() != "2.0.0" { + + // add external variables to spec variables + if d.Data.GetDevfileWorkspaceSpec().Variables == nil { + d.Data.GetDevfileWorkspaceSpec().Variables = map[string]string{} + } + allVariables := d.Data.GetDevfileWorkspaceSpec().Variables + for key, val := range args.ExternalVariables { + allVariables[key] = val + } + // replace the top level variable keys with their values in the devfile varWarning = variables.ValidateAndReplaceGlobalVariable(d.Data.GetDevfileWorkspaceSpec()) } diff --git a/vendor/github.com/devfile/library/pkg/devfile/parser/data/v2/2.2.0/devfileJsonSchema220.go b/vendor/github.com/devfile/library/pkg/devfile/parser/data/v2/2.2.0/devfileJsonSchema220.go index 54e36df54f8..d729f7cfd98 100644 --- a/vendor/github.com/devfile/library/pkg/devfile/parser/data/v2/2.2.0/devfileJsonSchema220.go +++ b/vendor/github.com/devfile/library/pkg/devfile/parser/data/v2/2.2.0/devfileJsonSchema220.go @@ -1855,6 +1855,11 @@ const JsonSchema220 = `{ "additionalProperties": { "type": "string" } + }, + "version": { + "description": "Specific stack/sample version to pull the parent devfile from, when using id in the parent reference. To specify 'version', 'id' must be defined and used as the import reference source. 'version' can be either a specific stack version, or 'latest'. If no 'version' specified, default version will be used.", + "type": "string", + "pattern": "^(latest)|(([1-9])\\.([0-9]+)\\.([0-9]+)(\\-[0-9a-z-]+(\\.[0-9a-z-]+)*)?(\\+[0-9A-Za-z-]+(\\.[0-9A-Za-z-]+)*)?)$" } }, "additionalProperties": false diff --git a/vendor/github.com/devfile/library/pkg/devfile/parser/parse.go b/vendor/github.com/devfile/library/pkg/devfile/parser/parse.go index 066e14a1ccc..a4ba3820218 100644 --- a/vendor/github.com/devfile/library/pkg/devfile/parser/parse.go +++ b/vendor/github.com/devfile/library/pkg/devfile/parser/parse.go @@ -4,13 +4,14 @@ import ( "context" "encoding/json" "fmt" + "net/url" + "path" + "strings" + "github.com/devfile/library/pkg/util" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/clientcmd" - "net/url" - "path" "sigs.k8s.io/controller-runtime/pkg/client" - "strings" devfileCtx "github.com/devfile/library/pkg/devfile/parser/context" "github.com/devfile/library/pkg/devfile/parser/data" @@ -81,6 +82,8 @@ type ParserArgs struct { Context context.Context // K8sClient is the Kubernetes client instance used for interacting with a cluster K8sClient client.Client + // ExternalVariables override variables defined in the Devfile + ExternalVariables map[string]string } // ParseDevfile func populates the devfile data, parses and validates the devfile integrity. @@ -378,7 +381,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio id := importReference.Id registryURL := importReference.RegistryUrl if registryURL != "" { - devfileContent, err := getDevfileFromRegistry(id, registryURL) + devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version) if err != nil { return DevfileObj{}, err } @@ -392,7 +395,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio } else if tool.registryURLs != nil { for _, registryURL := range tool.registryURLs { - devfileContent, err := getDevfileFromRegistry(id, registryURL) + devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version) if devfileContent != nil && err == nil { d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent) if err != nil { @@ -411,12 +414,12 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio return DevfileObj{}, fmt.Errorf("failed to get id: %s from registry URLs provided", id) } -func getDevfileFromRegistry(id, registryURL string) ([]byte, error) { +func getDevfileFromRegistry(id, registryURL, version string) ([]byte, error) { if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") { return nil, fmt.Errorf("the provided registryURL: %s is not a valid URL", registryURL) } param := util.HTTPRequestParams{ - URL: fmt.Sprintf("%s/devfiles/%s", registryURL, id), + URL: fmt.Sprintf("%s/devfiles/%s/%s", registryURL, id, version), } return util.HTTPGetRequest(param, 0) } diff --git a/vendor/github.com/devfile/library/pkg/util/util.go b/vendor/github.com/devfile/library/pkg/util/util.go index cb97c23fc04..7d10a9b78d6 100644 --- a/vendor/github.com/devfile/library/pkg/util/util.go +++ b/vendor/github.com/devfile/library/pkg/util/util.go @@ -845,9 +845,6 @@ func GetAndExtractZip(zipURL string, destination string, pathToUnzip string) err if zipURL == "" { return errors.Errorf("Empty zip url: %s", zipURL) } - if !strings.Contains(zipURL, ".zip") { - return errors.Errorf("Invalid zip url: %s", zipURL) - } var pathToZip string if strings.HasPrefix(zipURL, "file://") { diff --git a/vendor/modules.txt b/vendor/modules.txt index 2919639aed4..90a4463edf2 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -118,7 +118,7 @@ github.com/danieljoos/wincred github.com/davecgh/go-spew/spew # github.com/deislabs/oras v0.8.1 ## explicit; go 1.13 -# github.com/devfile/api/v2 v2.0.0-20220117162434-6e6e6a8bc14c +# github.com/devfile/api/v2 v2.0.0-20220309195345-48ebbf1e51cf ## explicit; go 1.13 github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2 github.com/devfile/api/v2/pkg/attributes @@ -127,7 +127,7 @@ github.com/devfile/api/v2/pkg/utils/overriding github.com/devfile/api/v2/pkg/utils/unions github.com/devfile/api/v2/pkg/validation github.com/devfile/api/v2/pkg/validation/variables -# github.com/devfile/library v1.2.1-0.20220217161036-0f5995513e92 +# github.com/devfile/library v1.2.1-0.20220602130922-85a4805bd59c ## explicit; go 1.15 github.com/devfile/library/pkg/devfile github.com/devfile/library/pkg/devfile/generator