From a0972c07523549aa78cb2c88d634fcc0947a00b4 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Thu, 22 Aug 2024 20:41:48 -0400 Subject: [PATCH] lang, engine: Add a metaparam for catching accidental dollar signs Let's make our life easier for users! --- docs/documentation.md | 23 +++++++++++++++++++ engine/metaparams.go | 12 ++++++++++ engine/resources.go | 7 ++++++ lang/ast/structs.go | 12 +++++++++- lang/interpolate/parse.rl | 1 + .../TestAstFunc1/resdupefields0.txtar | 1 + .../TestAstFunc1/resdupefields5.txtar | 3 +++ lang/interpret_test/TestAstFunc2/res4.txtar | 2 ++ .../interpret_test/TestAstFunc3/dollar1.txtar | 5 ++++ .../interpret_test/TestAstFunc3/dollar2.txtar | 7 ++++++ 10 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 lang/interpret_test/TestAstFunc3/dollar1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/dollar2.txtar diff --git a/docs/documentation.md b/docs/documentation.md index 82f82543..75d5194b 100644 --- a/docs/documentation.md +++ b/docs/documentation.md @@ -274,6 +274,29 @@ and it can't guarantee it if the resource is blocked because of a failed pre-requisite resource. *XXX: This is currently not implemented!* +#### Dollar + +Boolean. Dollar allows you to have a resource name that starts with a `$` sign. +This is false by default. This helps you catch cases when you write code like: + +```mcl +$foo = "/tmp/file1" +file "$foo" {} # incorrect! +``` + +The above code would ignore the `$foo` variable and attempt to make a file named +`$foo` which would obviously not work. To correctly interpolate a variable, you +need to surround the name with curly braces. + +```mcl +$foo = "/tmp/file1" +file "${foo}" {} # correct! +``` + +This meta param is a safety measure to make your life easier. It works for all +resources. If someone comes up with a resource which would routinely start with +a dollar sign, then we can revisit the default for this resource kind. + #### Reverse Boolean. Reverse is a property that some resources can implement that specifies diff --git a/engine/metaparams.go b/engine/metaparams.go index b0165706..015a94fa 100644 --- a/engine/metaparams.go +++ b/engine/metaparams.go @@ -52,6 +52,7 @@ var DefaultMetaParams = &MetaParams{ //Sema: []string{}, Rewatch: false, Realize: false, // true would be more awesome, but unexpected for users + Dollar: false, } // MetaRes is the interface a resource must implement to support meta params. @@ -132,6 +133,13 @@ type MetaParams struct { // the resource is blocked because of a failed pre-requisite resource. // XXX: Not implemented! Realize bool `yaml:"realize"` + + // Dollar allows you to name a resource to start with the dollar + // character. We don't allow this by default since it's probably not + // needed, and is more likely to be a typo where the user forgot to + // interpolate a variable name. In the rare case when it's needed, you + // can disable that check with this meta param. + Dollar bool `yaml:"dollar"` } // Cmp compares two AutoGroupMeta structs and determines if they're equivalent. @@ -178,6 +186,9 @@ func (obj *MetaParams) Cmp(meta *MetaParams) error { if obj.Realize != meta.Realize { return fmt.Errorf("values for Realize are different") } + if obj.Dollar != meta.Dollar { + return fmt.Errorf("values for Dollar are different") + } return nil } @@ -218,6 +229,7 @@ func (obj *MetaParams) Copy() *MetaParams { Sema: sema, Rewatch: obj.Rewatch, Realize: obj.Realize, + Dollar: obj.Dollar, } } diff --git a/engine/resources.go b/engine/resources.go index 9edcfe7c..69084b49 100644 --- a/engine/resources.go +++ b/engine/resources.go @@ -33,6 +33,7 @@ import ( "context" "encoding/gob" "fmt" + "strings" "github.com/purpleidea/mgmt/engine/local" "github.com/purpleidea/mgmt/pgraph" @@ -272,6 +273,12 @@ func Validate(res Res) error { return errwrap.Wrapf(err, "the Res has an invalid meta param") } + // TODO: pull dollar prefix from a constant + // This catches typos where the user meant to use ${var} interpolation. + if !res.MetaParams().Dollar && strings.HasPrefix(res.Name(), "$") { + return fmt.Errorf("the Res name starts with a $") + } + return res.Validate() } diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 32d8b3ac..b8bf17b9 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -1087,6 +1087,9 @@ func (obj *StmtRes) metaparams(table map[interfaces.Func]types.Value, res engine case "realize": meta.Realize = v.Bool() // must not panic + case "dollar": + meta.Dollar = v.Bool() // must not panic + case "reverse": if rm != nil { rm.Disabled = !v.Bool() // must not panic @@ -1150,6 +1153,9 @@ func (obj *StmtRes) metaparams(table map[interfaces.Func]types.Value, res engine if val, exists := v.Struct()["realize"]; exists { meta.Realize = val.Bool() // must not panic } + if val, exists := v.Struct()["dollar"]; exists { + meta.Dollar = val.Bool() // must not panic + } if val, exists := v.Struct()["reverse"]; exists && rm != nil { rm.Disabled = !val.Bool() // must not panic } @@ -1757,6 +1763,7 @@ func (obj *StmtResMeta) Init(data *interfaces.Data) error { case "sema": case "rewatch": case "realize": + case "dollar": case "reverse": case "autoedge": case "autogroup": @@ -1966,6 +1973,9 @@ func (obj *StmtResMeta) TypeCheck(kind string) ([]*interfaces.UnificationInvaria case "realize": typExpr = types.TypeBool + case "dollar": + typExpr = types.TypeBool + case "reverse": // TODO: We might want more parameters about how to reverse. typExpr = types.TypeBool @@ -1982,7 +1992,7 @@ func (obj *StmtResMeta) TypeCheck(kind string) ([]*interfaces.UnificationInvaria // FIXME: allow partial subsets of this struct, and in any order // FIXME: we might need an updated unification engine to do this wrap := func(reverse *types.Type) *types.Type { - return types.NewType(fmt.Sprintf("struct{noop bool; retry int; retryreset bool; delay int; poll int; limit float; burst int; reset bool; sema []str; rewatch bool; realize bool; reverse %s; autoedge bool; autogroup bool}", reverse.String())) + return types.NewType(fmt.Sprintf("struct{noop bool; retry int; retryreset bool; delay int; poll int; limit float; burst int; reset bool; sema []str; rewatch bool; realize bool; dollar bool; reverse %s; autoedge bool; autogroup bool}", reverse.String())) } // TODO: We might want more parameters about how to reverse. typExpr = wrap(types.TypeBool) diff --git a/lang/interpolate/parse.rl b/lang/interpolate/parse.rl index 97d0225f..18ef303d 100644 --- a/lang/interpolate/parse.rl +++ b/lang/interpolate/parse.rl @@ -41,6 +41,7 @@ import ( // Parse performs string interpolation on the input. It returns the list of // tokens found. It looks for variables of the format ${foo}. The curly braces // are required. +// XXX: Pull dollar sign and curly chars from VarPrefix and other constants. func Parse(data string) (out Stream, _ error) { var ( // variables used by Ragel diff --git a/lang/interpret_test/TestAstFunc1/resdupefields0.txtar b/lang/interpret_test/TestAstFunc1/resdupefields0.txtar index 9526801a..f6d136ed 100644 --- a/lang/interpret_test/TestAstFunc1/resdupefields0.txtar +++ b/lang/interpret_test/TestAstFunc1/resdupefields0.txtar @@ -14,6 +14,7 @@ test "test" { sema => ["foo:1", "bar:3",], rewatch => false, realize => true, + dollar => false, reverse => true, autoedge => true, autogroup => true, diff --git a/lang/interpret_test/TestAstFunc1/resdupefields5.txtar b/lang/interpret_test/TestAstFunc1/resdupefields5.txtar index acbbbbc9..fbdc155c 100644 --- a/lang/interpret_test/TestAstFunc1/resdupefields5.txtar +++ b/lang/interpret_test/TestAstFunc1/resdupefields5.txtar @@ -14,6 +14,7 @@ test "test" { sema => ["foo:1", "bar:3",], rewatch => false, realize => true, + dollar => false, reverse => true, autoedge => true, autogroup => true, @@ -31,6 +32,7 @@ Edge: const -> composite # autoedge Edge: const -> composite # autogroup Edge: const -> composite # burst Edge: const -> composite # delay +Edge: const -> composite # dollar Edge: const -> composite # limit Edge: const -> composite # noop Edge: const -> composite # poll @@ -60,3 +62,4 @@ Vertex: const Vertex: const Vertex: const Vertex: const +Vertex: const diff --git a/lang/interpret_test/TestAstFunc2/res4.txtar b/lang/interpret_test/TestAstFunc2/res4.txtar index 59d6757f..74a808c8 100644 --- a/lang/interpret_test/TestAstFunc2/res4.txtar +++ b/lang/interpret_test/TestAstFunc2/res4.txtar @@ -14,6 +14,7 @@ test "t1" { # sema => ["foo:1", "bar:3",], # rewatch => false, # realize => true, + # dollar => false, # reverse => true, # autoedge => true, # autogroup => true, @@ -29,6 +30,7 @@ test "t1" { Meta:sema => ["foo:1", "bar:3",], Meta:rewatch => false, Meta:realize => true, + Meta:dollar => false, Meta:reverse => true, Meta:autoedge => true, Meta:autogroup => true, diff --git a/lang/interpret_test/TestAstFunc3/dollar1.txtar b/lang/interpret_test/TestAstFunc3/dollar1.txtar new file mode 100644 index 00000000..18fa8038 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/dollar1.txtar @@ -0,0 +1,5 @@ +-- main.mcl -- +$foo = "/tmp/file1" +test "$foo" {} # catch simple typos! should be "${foo}" +-- OUTPUT -- +# err: errValidate: test[$foo] did not Validate: the Res name starts with a $ diff --git a/lang/interpret_test/TestAstFunc3/dollar2.txtar b/lang/interpret_test/TestAstFunc3/dollar2.txtar new file mode 100644 index 00000000..fb951fae --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/dollar2.txtar @@ -0,0 +1,7 @@ +-- main.mcl -- +$foo = "/tmp/file1" +test "$foo" { + Meta:dollar => true, # allow what would normally be a typo! +} +-- OUTPUT -- +Vertex: test[$foo]