Add prefix-based matching for namespaces and excludedNamespaces

[julianKatz]

Add prefix-based glob matching support across
Gatekeeper's various match sections.

These include:

• Constraints (via the regolib match)
• Config resource (via the process excluder)
• Mutations (via the match package)

Fixes #1009

[codecov-commenter]

Codecov Report

Merging #1404 (d96060f) into master (b791188) will decrease coverage by 0.07%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##           master    #1404      +/-   ##
==========================================
- Coverage   50.06%   49.99%   -0.08%     
==========================================
  Files          73       75       +2     
  Lines        5019     5061      +42     
==========================================
+ Hits         2513     2530      +17     
- Misses       2155     2180      +25     
  Partials      351      351              

Flag	Coverage Δ
unittests	49.99% <79.16%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/controller/config/config_controller.go	65.56% <ø> (ø)
pkg/controller/config/process/excluder.go	10.00% <37.50%> (ø)
pkg/mutation/match/match.go	88.75% <100.00%> (+3.03%)	⬆️
pkg/target/target.go	68.14% <100.00%> (+0.72%)	⬆️
pkg/util/prefix_wildcard.go	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b791188...d96060f. Read the comment docs.

[maxsmythe]

Rego basically LGTM, with some comments.

We will need to have some way to ensure wildcards only show up at the end of the match schema... either a validation check in the JSONSchema, or as part of the validating webhook.

JSONSchema is preferable, if possible.

[julianKatz]

either a validation check in the JSONSchema

@maxsmythe I've added a regex to the JSONSchema

[maxsmythe]

LGTM with one nit

@shomron @sozercan LGTY?

[maxsmythe]

New changes LGTM, 1 nit

[julianKatz]

ping :) @sozercan @shomron

[sozercan]

minor suggestion, LGTM

[sozercan]

should we add a wildcard only and empty test here too?

[julianKatz]

Good catch. I think the best way to deal with this is to implement a pattern requirement on the excludedNamespace field. I believe I can do that with a kubebuilder annotation. Will research now.

[julianKatz]

I've added a pattern check on excludedNamespaces in the config resource's match section. This will prevent both "*" and "" from being added under excludedNamespaces.

@sozercan @maxsmythe is that a reasonable way to solve this problem?

[julianKatz]

Does this change count as backwards incompatible? At least at the level of the CRD, those were valid entries before. They weren't valid namespaces, so perhaps it's alright...

[maxsmythe]

IMO it's fine. "*" and "" never would have matched an existing namespace (invalid namespace names), so there would be no behavioral change that would impact users.

If, for some reason, these values were in user's constraints, they weren't doing anything and they should probably be notified of that as it was likely a mistake.

[maxsmythe]

Looks like there are some failing tests --- type error?

cannot use make([]string, len(*in)) (value of type []string) as []validWilcardNamespace value in assignment (typecheck)

[maxsmythe]

Looks like the type error could be fixed by running make generate

[julianKatz]

Looks like the type error could be fixed by running make generate

Done

[maxsmythe]

This description should be targeted at consumers of the API, so no "PrefixWildCard", maybe some guidance as to correct inputs and what they do.

[julianKatz]

So far I haven't been able to find a way to do this with kubebuilder. It defaults to using the description that's written on the custom PrefixWildcard type in pkg/util/prefix_wildcard.go.

Removing PrefixWildcard is a from this string causes a lint error:

pkg/util/prefix_wildcard.go:7:1: exported: comment on exported type PrefixWildcard should be of the form "PrefixWildcard ..." (with optional leading article) (revive)
// a string that supports globbing at its end.
^

Rather than add to the jumbled mess of kustomize, I'm going to add a //nolint:revive to the comment and call it a day.

LMK what you think of the more in-depth description I added.

[maxsmythe]

nit: drop val, return an explicit true, val has no relation to whether a given field matches.

[julianKatz]

If the boolMap's values don't matter (i.e., we always return true), should it be refactored to be a map of struct{}? If it's always returning true, then it's essentially functioning as a set.

[maxsmythe]

Yep, it's a set. bool is a little easier to work with for sets, as it allows you to test for membership just by accessing the value. Example: isMember["a"] returns true if "a" is a member, but the default value (false) if it's not.

[julianKatz]

rodger that. I've updated the function to always return true.

[maxsmythe]

LGTM with one nit

[erezo9]

Is there a way to add an astrix at the beginning of the namespace name?
Im trying to do match namespaces like -myenv-
but it doesnt work cause the astrix is only allowed at the end of the regular expression

[maxsmythe]

It looks like currently we only support prefix or suffix matching, not midfix. Can you open a bug so we can discuss the feature? I'd imagine there may be some concerns about potentially making future performance optimizations harder, but better to hash that out in a dedicated spot.

gatekeeper/pkg/util/wildcard.go

Lines 16 to 26 in abe7fe8

	func (w Wildcard) Matches(candidate string) bool {
	wStr := string(w)
	switch {
	case strings.HasPrefix(wStr, "*"):
	return strings.HasSuffix(candidate, strings.TrimPrefix(wStr, "*"))
	case strings.HasSuffix(wStr, "*"):
	return strings.HasPrefix(candidate, strings.TrimSuffix(wStr, "*"))
	default:
	return wStr == candidate
	}
	}