Validate node_labels syntax at role creation time

Also improve error output and user validation.
Based on missed feedback in #4253 (review)

lib/services/role.go

@@ -710,6 +710,18 @@ func (r *RoleV3) CheckAndSetDefaults() error {
 		if key == Wildcard && !(len(val) == 1 && val[0] == Wildcard) {
 			return trace.BadParameter("selector *:<val> is not supported")
 		}
+		for _, l := range val {
+			if _, err := parse.NewMatcher(l); err != nil {
+				return trace.Wrap(err)
+			}
+		}
+	}
+	for _, val := range r.Spec.Deny.NodeLabels {
+		for _, l := range val {
+			if _, err := parse.NewMatcher(l); err != nil {
+				return trace.Wrap(err)
+			}
+		}
 	}
 	for i := range r.Spec.Allow.Rules {
 		err := r.Spec.Allow.Rules[i].CheckAndSetDefaults()

lib/services/user.go

@@ -492,7 +492,10 @@ func (u *UserV1) Check() error {
 	}
 	for _, login := range u.AllowedLogins {
 		e, err := parse.NewExpression(login)
-		if err == nil && e.Namespace() != parse.LiteralNamespace {
+		if err != nil {
+			return trace.Wrap(err)
+		}
+		if e.Namespace() != parse.LiteralNamespace {
 			return trace.BadParameter("role variables not allowed in allowed logins")
 		}
 	}

lib/utils/parse/parse.go

@@ -175,7 +175,12 @@ type Matcher interface {
 //
 // These expressions do not support variable interpolation (e.g.
 // `{{internal.logins}}`), like Expression does.
-func NewMatcher(value string) (Matcher, error) {
+func NewMatcher(value string) (m Matcher, err error) {
+	defer func() {
+		if err != nil {
+			err = trace.WrapWithMessage(err, "see supported syntax at https://gravitational.com/teleport/docs/enterprise/ssh-rbac/#rbac-for-hosts")
+		}
+	}()
 	match := reVariable.FindStringSubmatch(value)
 	if len(match) == 0 {
 		if strings.Contains(value, "{{") || strings.Contains(value, "}}") {
@@ -191,7 +196,7 @@ func NewMatcher(value string) (Matcher, error) {
 	// parse and get the ast of the expression
 	expr, err := parser.ParseExpr(variable)
 	if err != nil {
-		return nil, trace.BadParameter("failed to parse %q: %v", variable, err)
+		return nil, trace.BadParameter("failed to parse %q: %v", value, err)
 	}
 
 	// walk the ast tree and gather the variable parts
@@ -204,7 +209,7 @@ func NewMatcher(value string) (Matcher, error) {
 	// the matching logic. For example
 	// `{{regexp.match(external.allowed_env_trait)}}`.
 	if result.transform != nil || len(result.parts) > 0 {
-		return nil, trace.BadParameter("%q is not a valid matcher expression - no variables and transformations are allowed", variable)
+		return nil, trace.BadParameter("%q is not a valid matcher expression - no variables and transformations are allowed", value)
 	}
 	return newPrefixSuffixMatcher(prefix, suffix, result.match), nil
 }
@@ -345,8 +350,7 @@ func walk(node ast.Node) (*walkResult, error) {
 					if err != nil {
 						return nil, trace.Wrap(err)
 					}
-					// If this is not_match, wrap the regexpMatcher to invert
-					// it.
+					// If this is not_match, wrap the regexpMatcher to invert it.
 					if fn == RegexpNotMatchFnName {
 						result.match = notMatcher{result.match}
 					}