change: make -error-on-replace more cooperating

With this change, the proxy wouldn't necessary return an error if
`-error-on-replace` is set and the incoming query has a label matcher
overlapping with the enforced label.

For instance when the enforced label is `namespace="foo"`, the following
expressions aren't rejected anymore:
- `up{namespace!="bar"}`
- `up{namespace=~"foo|bar"}`
- `up{namespace!~"bar"}`

The following expressions are rejected as before:
- `up{namespace=""}`
- `up{namespace="bar"}`
- `up{namespace=~"bar|fred"}`
- `up{namespace!~"foo"}`

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

README.md

@@ -157,7 +157,7 @@ prom-label-proxy \
 
 > :warning: The above feature is experimental. Be careful when using this option, it may expose sensitive metrics if you use a too permissive expression.
 
-To error out when the query already contains a label matcher that differs from the one the proxy would inject, you can use the `-error-on-replace` option. For example:
+To error out when the query already contains a label matcher that conflicts with the one the proxy would inject, you can use the `-error-on-replace` option. For example:
 
 ```
 prom-label-proxy \

injectproxy/enforce.go

@@ -152,30 +152,126 @@ func (ms PromQLEnforcer) EnforceNode(node parser.Node) error {
 	return nil
 }
 
-// EnforceMatchers appends the configured label matcher if not present.
-// If the label matcher that is to be injected is present (by labelname) but
-// different (either by match type or value) the behavior depends on the
-// errorOnReplace variable and the enforced matcher(s):
-// * if errorOnReplace is true, an error is returned,
-// * if errorOnReplace is false and the label matcher type is '=', the existing matcher is silently replaced.
-// * otherwise the existing matcher is preserved.
+// EnforceMatchers appends the enforced label matcher(s) to the list of matchers
+// if not already present.
+//
+// If the label matcher that is to be injected is present (by labelname), the
+// behavior depends on the errorOnReplace variable and the enforced matcher(s):
+// * If errorOnReplace is false
+//   - And the label matcher type is '=', the existing matcher is silently
+//     discarded whatever is the original value.
+//   - Otherwise the existing matcher is preserved.
+//
+// * if errorOnReplace is true
+//   - And the label matcher and the enforced matcher are disjoint, the function returns an error.
+//   - Otherwise the existing matcher is preserved.
 func (ms PromQLEnforcer) EnforceMatchers(targets []*labels.Matcher) ([]*labels.Matcher, error) {
 	var res []*labels.Matcher
 
 	for _, target := range targets {
-		if matcher, ok := ms.labelMatchers[target.Name]; ok {
-			// matcher.String() returns something like "labelfoo=value"
-			if ms.errorOnReplace && matcher.String() != target.String() {
-				return res, fmt.Errorf("%w: label matcher value %q conflicts with injected value %q", ErrIllegalLabelMatcher, matcher.String(), target.String())
+		matcher, ok := ms.labelMatchers[target.Name]
+		if !ok {
+			res = append(res, target)
+			continue
+		}
+
+		if ms.errorOnReplace {
+			var ok bool
+
+			// Ensure that the expression's matcher combined with the
+			// enforced matchers can return some result. If the combined
+			// matchers return no result, the function returns an error.
+			//
+			// For instance, when te enforced matcher is 'tenant="bar"':
+			// * and the expression's selector is 'tenant="foo"' then the
+			// result is always empty.
+			// * and the expression's selector is 'tenant!="foo"' then the
+			// matchers don't conflict.
+			switch matcher.Type {
+
+			case labels.MatchEqual:
+				switch target.Type {
+				case labels.MatchEqual:
+					ok = matcher.Value == target.Value
+				case labels.MatchNotEqual:
+					ok = matcher.Value != target.Value
+				case labels.MatchRegexp:
+					ok = target.Matches(matcher.Value)
+				case labels.MatchNotRegexp:
+					ok = target.Matches(matcher.Value)
+				}
+
+			case labels.MatchNotEqual:
+				switch target.Type {
+				case labels.MatchEqual:
+					ok = target.Value == "" || matcher.Matches(target.Value)
+				case labels.MatchNotEqual:
+					ok = true
+				case labels.MatchRegexp:
+					frm, _ := labels.NewFastRegexMatcher(target.Value)
+					ok = (frm == nil || len(frm.SetMatches()) == 0)
+					if !ok {
+						for _, sm := range frm.SetMatches() {
+							if sm != matcher.Value {
+								ok = true
+								break
+							}
+						}
+					}
+				case labels.MatchNotRegexp:
+					ok = true
+				}
+
+			case labels.MatchRegexp:
+				switch target.Type {
+				case labels.MatchEqual:
+					ok = matcher.Matches(target.Value)
+				case labels.MatchNotEqual:
+					ok = true
+				case labels.MatchRegexp:
+					ok = target.Value != ""
+				case labels.MatchNotRegexp:
+					ok = target.Value != matcher.Value
+				}
+
+			case labels.MatchNotRegexp:
+				switch target.Type {
+				case labels.MatchEqual:
+					ok = target.Value != "" || !matcher.Matches("")
+				case labels.MatchNotEqual:
+					ok = true
+				case labels.MatchRegexp:
+					frm, _ := labels.NewFastRegexMatcher(target.Value)
+					if frm != nil {
+						for _, sm := range frm.SetMatches() {
+							if matcher.Matches(sm) {
+								ok = true
+								break
+							}
+						}
+					}
+					ok = ok && !(target.Value == "" && matcher.Matches(""))
+					ok = ok && target.Value != matcher.Value
+				case labels.MatchNotRegexp:
+					ok = true
+				}
 			}
 
-			// Drop the existing matcher only if the enforced matcher is an
-			// equal matcher.
-			if matcher.Type == labels.MatchEqual {
-				continue
+			if !ok {
+				return res, fmt.Errorf("%w: label matcher %q conflicts with injected matcher %q", ErrIllegalLabelMatcher, target.String(), matcher.String())
 			}
 		}
 
+		// Always drop the expression matcher if:
+		// * the enforced matcher is an equal matcher because it will be
+		// added after iterating on all the expression's matchers.
+		// * or it is equal to the enforced matcher.
+		// In both cases, the enforced matcher will be added after
+		// iterating on all the expression's matchers.
+		if matcher.Type == labels.MatchEqual || matcher.String() == target.String() {
+			continue
+		}
+
 		res = append(res, target)
 	}