From 3758fc6866f4adfc777565e58e644d1580923308 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Mon, 15 Mar 2021 16:57:04 -0700 Subject: [PATCH 1/2] Testing: structured arguments This should make it a bit easier to manage argument overrides than the set of templates we have now, especially while we wait for componentconfig to be everywhere. Arguments can be defaulted, and those defaults can be overriden or appended to, and then finally render to a slice of strings to be passed as arguments and such. This should make it easier to configure the API server without needing to splat out the existing default arguments, maybe splice some stuff out, etc. For example: ```go apiServer.Configure(). Disable("insecure-port"). Append("vmodule", "httplog.go=10"). Set("v", "5") ``` --- pkg/internal/testing/process/arguments.go | 215 ++++++++++++++++++ .../testing/process/arguments_test.go | 210 +++++++++++++++++ 2 files changed, 425 insertions(+) create mode 100644 pkg/internal/testing/process/arguments_test.go diff --git a/pkg/internal/testing/process/arguments.go b/pkg/internal/testing/process/arguments.go index c336f02fa7..4d9edd6f5b 100644 --- a/pkg/internal/testing/process/arguments.go +++ b/pkg/internal/testing/process/arguments.go @@ -6,6 +6,8 @@ import ( ) // RenderTemplates returns an []string to render the templates +// +// Deprecated: this should be removed when we remove APIServer.Args. func RenderTemplates(argTemplates []string, data interface{}) (args []string, err error) { var t *template.Template @@ -27,3 +29,216 @@ func RenderTemplates(argTemplates []string, data interface{}) (args []string, er return } + +// EmptyArguments constructs an empty set of flags with no defaults. +func EmptyArguments() *Arguments { + return &Arguments{ + values: make(map[string]Arg), + } +} + +// Arguments are structured, overridable arguments. +// Each Arguments object contains some set of default arguments, which may +// be appended to, or overridden. +// +// When ready, you can serialize them to pass to exec.Command and friends using +// AsStrings. +// +// All flag-setting methods return the *same* instance of Arguments so that you +// can chain calls. +type Arguments struct { + // values contains the user-set values for the arguments. + // `values[key] = dontPass` means "don't pass this flag" + // `values[key] = passAsName` means "pass this flag without args like --key` + // `values[key] = []string{a, b, c}` means "--key=a --key=b --key=c` + // any values not explicitly set here will be copied from defaults on final rendering. + values map[string]Arg +} + +// Arg is an argument that has one or more values, +// and optionally falls back to default values. +type Arg interface { + // Append adds new values to this argument, returning + // a new instance contain the new value. The intermediate + // argument should generally be assumed to be consumed. + Append(vals ...string) Arg + // Get returns the full set of values, optionally including + // the passed in defaults. If it returns nil, this will be + // skipped. If it returns a non-nil empty slice, it'll be + // assumed that the argument should be passed as name-only. + Get(defaults []string) []string +} + +type userArg []string + +func (a userArg) Append(vals ...string) Arg { + return userArg(append(a, vals...)) +} +func (a userArg) Get(_ []string) []string { + return []string(a) +} + +type defaultedArg []string + +func (a defaultedArg) Append(vals ...string) Arg { + return defaultedArg(append(a, vals...)) +} +func (a defaultedArg) Get(defaults []string) []string { + res := append([]string(nil), defaults...) + return append(res, a...) +} + +type dontPassArg struct{} + +func (a dontPassArg) Append(vals ...string) Arg { + return userArg(vals) +} +func (dontPassArg) Get(_ []string) []string { + return nil +} + +type passAsNameArg struct{} + +func (a passAsNameArg) Append(_ ...string) Arg { + return passAsNameArg{} +} +func (passAsNameArg) Get(_ []string) []string { + return []string{} +} + +var ( + // DontPass indicates that the given argument will not actually be + // rendered. + DontPass Arg = dontPassArg{} + // PassAsName indicates that the given flag will be passed as `--key` + // without any value. + PassAsName Arg = passAsNameArg{} +) + +// AsStrings serializes this set of arguments to a slice of strings appropriate +// for passing to exec.Command and friends, making use of the given defaults +// as indicated for each particular argument. +// +// - Any flag in defaults that's not in Arguments will be present in the output +// - Any flag that's present in Arguments will be passed the corresponding +// defaults to do with as it will (ignore, append-to, suppress, etc). +func (a *Arguments) AsStrings(defaults map[string][]string) []string { + // sort for deterministic ordering + keysInOrder := make([]string, 0, len(defaults)+len(a.values)) + for key := range defaults { + if _, userSet := a.values[key]; userSet { + continue + } + keysInOrder = append(keysInOrder, key) + } + for key := range a.values { + keysInOrder = append(keysInOrder, key) + } + sort.Strings(keysInOrder) + + var res []string + for _, key := range keysInOrder { + vals := a.Get(key).Get(defaults[key]) + switch { + case vals == nil: // don't pass + continue + case len(vals) == 0: // pass as name + res = append(res, "--"+key) + default: + for _, val := range vals { + res = append(res, "--"+key+"="+val) + } + } + } + + return res +} + +// Get returns the value of the given flag. If nil, +// it will not be passed in AsString, otherwise: +// +// len == 0 --> `--key`, len > 0 --> `--key=val1 --key=val2 ...` +func (a *Arguments) Get(key string) Arg { + if vals, ok := a.values[key]; ok { + return vals + } + return defaultedArg(nil) +} + +// Enable configures the given key to be passed as a "name-only" flag, +// like, `--key`. +func (a *Arguments) Enable(key string) *Arguments { + a.values[key] = PassAsName + return a +} + +// Disable prevents this flag from be passed. +func (a *Arguments) Disable(key string) *Arguments { + a.values[key] = DontPass + return a +} + +// Append adds additional values to this flag. If this flag has +// yet to be set, initial values will include defaults. If you want +// to intentionally ignore defaults/start from scratch, call AppendNoDefaults. +// +// Multiple values will look like `--key=value1 --key=value2 ...`. +func (a *Arguments) Append(key string, values ...string) *Arguments { + vals, present := a.values[key] + if !present { + vals = defaultedArg{} + } + a.values[key] = vals.Append(values...) + return a +} + +// AppendNoDefaults adds additional values to this flag. However, +// unlike Append, it will *not* copy values from defaults. +func (a *Arguments) AppendNoDefaults(key string, values ...string) *Arguments { + vals, present := a.values[key] + if !present { + vals = userArg{} + } + a.values[key] = vals.Append(values...) + return a +} + +// Set resets the given flag to the specified values, ignoring any existing +// values or defaults. +func (a *Arguments) Set(key string, values ...string) *Arguments { + a.values[key] = userArg(values) + return a +} + +// SetRaw sets the given flag to the given Arg value directly. Use this if +// you need to do some complicated deferred logic or something. +// +// Otherwise behaves like Set. +func (a *Arguments) SetRaw(key string, val Arg) *Arguments { + a.values[key] = val + return a +} + +// FuncArg is a basic implementation of Arg that can be used for custom argument logic, +// like pulling values out of APIServer, or dynamically calculating values just before +// launch. +// +// The given function will be mapped directly to Arg#Get, and will generally be +// used in conjunction with SetRaw. For example, to set `--some-flag` to the +// API server's CertDir, you could do: +// +// server.Configure().SetRaw("--some-flag", FuncArg(func(defaults []string) []string { +// return []string{server.CertDir} +// })) +// +// FuncArg ignores Appends; if you need to support appending values too, consider implementing +// Arg directly. +type FuncArg func([]string) []string + +// Append is a no-op for FuncArg, and just returns itself. +func (a FuncArg) Append(vals ...string) Arg { return a } + +// Get delegates functionality to the FuncArg function itself. +func (a FuncArg) Get(defaults []string) []string { + return a(defaults) +} diff --git a/pkg/internal/testing/process/arguments_test.go b/pkg/internal/testing/process/arguments_test.go new file mode 100644 index 0000000000..fb7da9ed82 --- /dev/null +++ b/pkg/internal/testing/process/arguments_test.go @@ -0,0 +1,210 @@ +package process_test + +import ( + "net/url" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + . "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" +) + +var _ = Describe("Arguments Templates", func() { + It("templates URLs", func() { + templates := []string{ + "plain URL: {{ .SomeURL }}", + "method on URL: {{ .SomeURL.Hostname }}", + "empty URL: {{ .EmptyURL }}", + "handled empty URL: {{- if .EmptyURL }}{{ .EmptyURL }}{{ end }}", + } + data := struct { + SomeURL *url.URL + EmptyURL *url.URL + }{ + &url.URL{Scheme: "https", Host: "the.host.name:3456"}, + nil, + } + + out, err := RenderTemplates(templates, data) + Expect(err).NotTo(HaveOccurred()) + Expect(out).To(BeEquivalentTo([]string{ + "plain URL: https://the.host.name:3456", + "method on URL: the.host.name", + "empty URL: <nil>", + "handled empty URL:", + })) + }) + + It("templates strings", func() { + templates := []string{ + "a string: {{ .SomeString }}", + "empty string: {{- .EmptyString }}", + } + data := struct { + SomeString string + EmptyString string + }{ + "this is some random string", + "", + } + + out, err := RenderTemplates(templates, data) + Expect(err).NotTo(HaveOccurred()) + Expect(out).To(BeEquivalentTo([]string{ + "a string: this is some random string", + "empty string:", + })) + }) + + It("has no access to unexported fields", func() { + templates := []string{ + "this is just a string", + "this blows up {{ .test }}", + } + data := struct{ test string }{"ooops private"} + + out, err := RenderTemplates(templates, data) + Expect(out).To(BeEmpty()) + Expect(err).To(MatchError( + ContainSubstring("is an unexported field of struct"), + )) + }) + + It("errors when field cannot be found", func() { + templates := []string{"this does {{ .NotExist }}"} + data := struct{ Unused string }{"unused"} + + out, err := RenderTemplates(templates, data) + Expect(out).To(BeEmpty()) + Expect(err).To(MatchError( + ContainSubstring("can't evaluate field"), + )) + }) +}) + +type plainDefaults map[string][]string + +func (d plainDefaults) DefaultArgs() map[string][]string { + return d +} + +var _ = Describe("Arguments", func() { + Context("when appending", func() { + It("should copy from defaults when appending for the first time", func() { + args := EmptyArguments(). + Append("some-key", "val3") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"val1", "val2", "val3"})) + }) + + It("should not copy from defaults if the flag has been disabled previously", func() { + args := EmptyArguments(). + Disable("some-key"). + Append("some-key", "val3") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"val3"})) + }) + + It("should only copy defaults the first time", func() { + args := EmptyArguments(). + Append("some-key", "val3", "val4"). + Append("some-key", "val5") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"val1", "val2", "val3", "val4", "val5"})) + }) + + It("should not copy from defaults if the flag has been previously overridden", func() { + args := EmptyArguments(). + Set("some-key", "vala"). + Append("some-key", "valb", "valc") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"vala", "valb", "valc"})) + }) + + Context("when explicitly overriding defaults", func() { + It("should not copy from defaults, but should append to previous calls", func() { + args := EmptyArguments(). + AppendNoDefaults("some-key", "vala"). + AppendNoDefaults("some-key", "valb", "valc") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"vala", "valb", "valc"})) + }) + + It("should not copy from defaults, but should respect previous appends' copies", func() { + args := EmptyArguments(). + Append("some-key", "vala"). + AppendNoDefaults("some-key", "valb", "valc") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"val1", "val2", "vala", "valb", "valc"})) + }) + + It("should not copy from defaults if the flag has been previously appended to ignoring defaults", func() { + args := EmptyArguments(). + AppendNoDefaults("some-key", "vala"). + Append("some-key", "valb", "valc") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"vala", "valb", "valc"})) + }) + }) + }) + + It("should ignore defaults when overriding", func() { + args := EmptyArguments(). + Set("some-key", "vala") + Expect(args.Get("some-key").Get([]string{"val1", "val2"})).To(Equal([]string{"vala"})) + }) + + It("should allow directly setting the argument value for custom argument types", func() { + args := EmptyArguments(). + SetRaw("custom-key", commaArg{"val3"}). + Append("custom-key", "val4") + Expect(args.Get("custom-key").Get([]string{"val1", "val2"})).To(Equal([]string{"val1,val2,val3,val4"})) + }) + + Context("when rendering flags", func() { + It("should not render defaults for disabled flags", func() { + defs := map[string][]string{ + "some-key": []string{"val1", "val2"}, + "other-key": []string{"val"}, + } + args := EmptyArguments(). + Disable("some-key") + Expect(args.AsStrings(defs)).To(ConsistOf("--other-key=val")) + }) + + It("should render name-only flags as --key", func() { + args := EmptyArguments(). + Enable("some-key") + Expect(args.AsStrings(nil)).To(ConsistOf("--some-key")) + }) + + It("should render multiple values as --key=val1, --key=val2", func() { + args := EmptyArguments(). + Append("some-key", "val1", "val2"). + Append("other-key", "vala", "valb") + Expect(args.AsStrings(nil)).To(ConsistOf("--other-key=valb", "--other-key=vala", "--some-key=val1", "--some-key=val2")) + }) + + It("should read from defaults if the user hasn't set a value for a flag", func() { + defs := map[string][]string{ + "some-key": []string{"val1", "val2"}, + } + args := EmptyArguments(). + Append("other-key", "vala", "valb") + Expect(args.AsStrings(defs)).To(ConsistOf("--other-key=valb", "--other-key=vala", "--some-key=val1", "--some-key=val2")) + }) + + It("should not render defaults if the user has set a value for a flag", func() { + defs := map[string][]string{ + "some-key": []string{"val1", "val2"}, + } + args := EmptyArguments(). + Set("some-key", "vala") + Expect(args.AsStrings(defs)).To(ConsistOf("--some-key=vala")) + }) + }) +}) + +type commaArg []string + +func (a commaArg) Get(defs []string) []string { + // not quite, but close enough + return []string{strings.Join(defs, ",") + "," + strings.Join(a, ",")} +} +func (a commaArg) Append(vals ...string) Arg { + return commaArg(append(a, vals...)) +} From ddfdfdff78f6ef84c514cf16308835bc23feb325 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Thu, 25 Mar 2021 15:38:47 -0700 Subject: [PATCH 2/2] Testing: use structured arguments This converts the internal testing package to use structured arguments, introducing a new helper to preserve compat when specifying legacy args. --- .../testing/controlplane/apiserver.go | 62 ++++++++-- pkg/internal/testing/controlplane/etcd.go | 48 ++++++-- pkg/internal/testing/process/arguments.go | 79 +++++++++++- .../testing/process/arguments_test.go | 113 ++++++++++++++++++ 4 files changed, 283 insertions(+), 19 deletions(-) diff --git a/pkg/internal/testing/controlplane/apiserver.go b/pkg/internal/testing/controlplane/apiserver.go index eae05eaa6a..56631e16c3 100644 --- a/pkg/internal/testing/controlplane/apiserver.go +++ b/pkg/internal/testing/controlplane/apiserver.go @@ -44,6 +44,11 @@ type APIServer struct { // // If not specified, the minimal set of arguments to run the APIServer will // be used. + // + // They will be loaded into the same argument set as Configure. Each flag + // will be Append-ed to the configured arguments just before launch. + // + // Deprecated: use Configure instead. Args []string // CertDir is a path to a directory containing whatever certificates the @@ -72,20 +77,34 @@ type APIServer struct { Err io.Writer processState *process.State + + // args contains the structured arguments to use for running the API server + // Lazily initialized by .Configure(), Defaulted eventually with .defaultArgs() + args *process.Arguments +} + +// Configure returns Arguments that may be used to customize the +// flags used to launch the API server. A set of defaults will +// be applied underneath. +func (s *APIServer) Configure() *process.Arguments { + if s.args == nil { + s.args = process.EmptyArguments() + } + return s.args } // Start starts the apiserver, waits for it to come up, and returns an error, // if occurred. func (s *APIServer) Start() error { if s.processState == nil { - if err := s.setState(); err != nil { + if err := s.setProcessState(); err != nil { return err } } return s.processState.Start(s.Out, s.Err) } -func (s *APIServer) setState() error { +func (s *APIServer) setProcessState() error { if s.EtcdURL == nil { return fmt.Errorf("expected EtcdURL to be configured") } @@ -133,15 +152,42 @@ func (s *APIServer) setState() error { return err } - args := s.Args - if len(args) == 0 { - args = APIServerDefaultArgs - } - - s.processState.Args, err = process.RenderTemplates(args, s) + s.processState.Args, err = process.TemplateAndArguments(s.Args, s.Configure(), process.TemplateDefaults{ + Data: s, + Defaults: s.defaultArgs(), + // as per kubernetes-sigs/controller-runtime#641, we need this (we + // probably need other stuff too, but this is the only thing that was + // previously considered a "minimal default") + MinimalDefaults: map[string][]string{ + "service-cluster-ip-range": []string{"10.0.0.0/24"}, + }, + }) return err } +func (s *APIServer) defaultArgs() map[string][]string { + args := map[string][]string{ + "advertise-address": []string{"127.0.0.1"}, + "service-cluster-ip-range": []string{"10.0.0.0/24"}, + "allow-privileged": []string{"true"}, + // we're keeping this disabled because if enabled, default SA is + // missing which would force all tests to create one in normal + // apiserver operation this SA is created by controller, but that is + // not run in integration environment + "disable-admission-plugins": []string{"ServiceAccount"}, + "cert-dir": []string{s.CertDir}, + "secure-port": []string{strconv.Itoa(s.SecurePort)}, + } + if s.EtcdURL != nil { + args["etcd-servers"] = []string{s.EtcdURL.String()} + } + if s.URL != nil { + args["insecure-port"] = []string{s.URL.Port()} + args["insecure-bind-address"] = []string{s.URL.Hostname()} + } + return args +} + func (s *APIServer) populateAPIServerCerts() error { _, statErr := os.Stat(filepath.Join(s.CertDir, "apiserver.crt")) if !os.IsNotExist(statErr) { diff --git a/pkg/internal/testing/controlplane/etcd.go b/pkg/internal/testing/controlplane/etcd.go index fde45e20ed..af28533f11 100644 --- a/pkg/internal/testing/controlplane/etcd.go +++ b/pkg/internal/testing/controlplane/etcd.go @@ -36,6 +36,11 @@ type Etcd struct { // // If not specified, the minimal set of arguments to run the Etcd will be // used. + // + // They will be loaded into the same argument set as Configure. Each flag + // will be Append-ed to the configured arguments just before launch. + // + // Deprecated: use Configure instead. Args []string // DataDir is a path to a directory in which etcd can store its state. @@ -59,22 +64,24 @@ type Etcd struct { // processState contains the actual details about this running process processState *process.State + + // args contains the structured arguments to use for running etcd. + // Lazily initialized by .Configure(), Defaulted eventually with .defaultArgs() + args *process.Arguments } // Start starts the etcd, waits for it to come up, and returns an error, if one // occoured. func (e *Etcd) Start() error { if e.processState == nil { - if err := e.setState(); err != nil { + if err := e.setProcessState(); err != nil { return err } } return e.processState.Start(e.Out, e.Err) } -func (e *Etcd) setState() error { - var err error - +func (e *Etcd) setProcessState() error { e.processState = &process.State{ Dir: e.DataDir, Path: e.Path, @@ -106,12 +113,11 @@ func (e *Etcd) setState() error { e.StartTimeout = e.processState.StartTimeout e.StopTimeout = e.processState.StopTimeout - args := e.Args - if len(args) == 0 { - args = EtcdDefaultArgs - } - - e.processState.Args, err = process.RenderTemplates(args, e) + var err error + e.processState.Args, err = process.TemplateAndArguments(e.Args, e.Configure(), process.TemplateDefaults{ + Data: e, + Defaults: e.defaultArgs(), + }) return err } @@ -121,6 +127,28 @@ func (e *Etcd) Stop() error { return e.processState.Stop() } +func (e *Etcd) defaultArgs() map[string][]string { + args := map[string][]string{ + "listen-peer-urls": []string{"http://localhost:0"}, + "data-dir": []string{e.DataDir}, + } + if e.URL != nil { + args["advertise-client-urls"] = []string{e.URL.String()} + args["listen-client-urls"] = []string{e.URL.String()} + } + return args +} + +// Configure returns Arguments that may be used to customize the +// flags used to launch etcd. A set of defaults will +// be applied underneath. +func (e *Etcd) Configure() *process.Arguments { + if e.args == nil { + e.args = process.EmptyArguments() + } + return e.args +} + // EtcdDefaultArgs exposes the default args for Etcd so that you // can use those to append your own additional arguments. var EtcdDefaultArgs = []string{ diff --git a/pkg/internal/testing/process/arguments.go b/pkg/internal/testing/process/arguments.go index 4d9edd6f5b..206fc2e902 100644 --- a/pkg/internal/testing/process/arguments.go +++ b/pkg/internal/testing/process/arguments.go @@ -3,11 +3,13 @@ package process import ( "bytes" "html/template" + "sort" + "strings" ) // RenderTemplates returns an []string to render the templates // -// Deprecated: this should be removed when we remove APIServer.Args. +// Deprecated: will be removed in favor of Arguments func RenderTemplates(argTemplates []string, data interface{}) (args []string, err error) { var t *template.Template @@ -30,6 +32,81 @@ func RenderTemplates(argTemplates []string, data interface{}) (args []string, er return } +// SliceToArguments converts a slice of arguments to structured arguments, +// appending each argument that starts with `--` and contains an `=` to the +// argument set, returning the rest. +// +// Deprecated: will be removed when RenderTemplates is removed +func SliceToArguments(sliceArgs []string, args *Arguments) []string { + var rest []string + for i, arg := range sliceArgs { + if arg == "--" { + rest = append(rest, sliceArgs[i:]...) + return rest + } + // skip non-flag arguments, skip arguments w/o equals because we + // can't tell if the next argument should take a value + if !strings.HasPrefix(arg, "--") || !strings.Contains(arg, "=") { + rest = append(rest, arg) + continue + } + + parts := strings.SplitN(arg[2:], "=", 2) + name := parts[0] + val := parts[1] + + args.AppendNoDefaults(name, val) + } + + return rest +} + +// TemplateDefaults specifies defaults to be used for joining structured arguments with templates. +// +// Deprecated: will be removed when RenderTemplates is removed +type TemplateDefaults struct { + // Data will be used to render the template. + Data interface{} + // Defaults will be used to default structured arguments if no template is passed. + Defaults map[string][]string + // MinimalDefaults will be used to default structured arguments if a template is passed. + // Use this for flags which *must* be present. + MinimalDefaults map[string][]string // for api server service-cluster-ip-range +} + +// TemplateAndArguments joins structured arguments and non-structured arguments, preserving existing +// behavior. Namely: +// +// 1. if templ has len > 0, it will be rendered against data +// 2. the rendered template values that look like `--foo=bar` will be split +// and appended to args, the rest will be kept around +// 3. the given args will be rendered as string form. If a template is given, +// no defaults will be used, otherwise defaults will be used +// 4. a result of [args..., rest...] will be returned +// +// Deprecated: will be removed when RenderTemplates is removed +func TemplateAndArguments(templ []string, args *Arguments, data TemplateDefaults) ([]string, error) { + if len(templ) == 0 { // 3 & 4 (no template case) + return args.AsStrings(data.Defaults), nil + } + + // 1: render the template + rendered, err := RenderTemplates(templ, data.Data) + if err != nil { + return nil, err + } + + // 2: filter out structured args and add them to args + rest := SliceToArguments(rendered, args) + + // 3 (template case): render structured args, no defaults (matching the + // legacy case where if Args was specified, no defaults were used) + res := args.AsStrings(data.MinimalDefaults) + + // 4: return the rendered structured args + all non-structured args + return append(res, rest...), nil +} + // EmptyArguments constructs an empty set of flags with no defaults. func EmptyArguments() *Arguments { return &Arguments{ diff --git a/pkg/internal/testing/process/arguments_test.go b/pkg/internal/testing/process/arguments_test.go index fb7da9ed82..9a49382fe6 100644 --- a/pkg/internal/testing/process/arguments_test.go +++ b/pkg/internal/testing/process/arguments_test.go @@ -81,6 +81,119 @@ var _ = Describe("Arguments Templates", func() { ContainSubstring("can't evaluate field"), )) }) + + Context("when joining with structured Arguments", func() { + var ( + args *Arguments + templ = []string{ + "--cheese=parmesean", + "-om", + "nom nom nom", + "--sharpness={{ .sharpness }}", + } + data = TemplateDefaults{ + Data: map[string]string{"sharpness": "extra"}, + Defaults: map[string][]string{ + "cracker": []string{"ritz"}, + "pickle": []string{"kosher-dill"}, + }, + MinimalDefaults: map[string][]string{ + "pickle": []string{"kosher-dill"}, + }, + } + ) + BeforeEach(func() { + args = EmptyArguments() + }) + + Context("when a template is given", func() { + It("should use minimal defaults", func() { + Expect(TemplateAndArguments(templ, args, data)).To(SatisfyAll( + Not(ContainElement("--cracker=ritz")), + ContainElement("--pickle=kosher-dill"), + )) + }) + + It("should render the template against the data", func() { + Expect(TemplateAndArguments(templ, args, data)).To(ContainElements( + "--sharpness=extra", + )) + }) + + It("should append the rendered template to structured arguments", func() { + args.Append("cheese", "cheddar") + + Expect(TemplateAndArguments(templ, args, data)).To(Equal([]string{ + "--cheese=cheddar", + "--cheese=parmesean", + "--pickle=kosher-dill", + "--sharpness=extra", + "-om", + "nom nom nom", + })) + }) + }) + + Context("when no template is given", func() { + It("should render the structured arguments with the given defaults", func() { + args. + Append("cheese", "cheddar", "parmesean"). + Append("cracker", "triscuit") + + Expect(TemplateAndArguments(nil, args, data)).To(Equal([]string{ + "--cheese=cheddar", + "--cheese=parmesean", + "--cracker=ritz", + "--cracker=triscuit", + "--pickle=kosher-dill", + })) + }) + }) + }) + + Context("when converting to structured Arguments", func() { + var args *Arguments + BeforeEach(func() { + args = EmptyArguments() + }) + + It("should skip arguments that don't start with `--`", func() { + rest := SliceToArguments([]string{"-first", "second", "--foo=bar"}, args) + Expect(rest).To(Equal([]string{"-first", "second"})) + Expect(args.AsStrings(nil)).To(Equal([]string{"--foo=bar"})) + }) + + It("should skip arguments that don't contain an `=` because they're ambiguous", func() { + rest := SliceToArguments([]string{"--first", "--second", "--foo=bar"}, args) + Expect(rest).To(Equal([]string{"--first", "--second"})) + Expect(args.AsStrings(nil)).To(Equal([]string{"--foo=bar"})) + }) + + It("should stop at the flag terminator (`--`)", func() { + rest := SliceToArguments([]string{"--first", "--second", "--", "--foo=bar"}, args) + Expect(rest).To(Equal([]string{"--first", "--second", "--", "--foo=bar"})) + Expect(args.AsStrings(nil)).To(BeEmpty()) + }) + + It("should split --foo=bar into Append(foo, bar)", func() { + rest := SliceToArguments([]string{"--foo=bar1", "--foo=bar2"}, args) + Expect(rest).To(BeEmpty()) + Expect(args.Get("foo").Get(nil)).To(Equal([]string{"bar1", "bar2"})) + }) + + It("should split --foo=bar=baz into Append(foo, bar=baz)", func() { + rest := SliceToArguments([]string{"--vmodule=file.go=3", "--vmodule=other.go=4"}, args) + Expect(rest).To(BeEmpty()) + Expect(args.Get("vmodule").Get(nil)).To(Equal([]string{"file.go=3", "other.go=4"})) + }) + + It("should append to existing arguments", func() { + args.Append("foo", "barA") + rest := SliceToArguments([]string{"--foo=bar1", "--foo=bar2"}, args) + Expect(rest).To(BeEmpty()) + Expect(args.Get("foo").Get([]string{"barI"})).To(Equal([]string{"barI", "barA", "bar1", "bar2"})) + }) + }) }) type plainDefaults map[string][]string