-
Notifications
You must be signed in to change notification settings - Fork 335
feat: add glob pattern support for --namespace flag #1247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: add glob pattern support for --namespace flag #1247
Conversation
Add support for glob-style wildcards (*, ?, [...]) in the --namespace flag, allowing users to match multiple namespaces with patterns like "main.*" or "*.gke". Fixes open-policy-agent#1141 Signed-off-by: majiayu000 <[email protected]>
Rename "Test command with namespace wildcard pattern" to "Test command with namespace exact match" since it tests exact namespace matching without wildcards. Signed-off-by: majiayu000 <[email protected]>
jalseth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking on this PR. Can you also add support to this for the verify command?
| // filterNamespaces filters the available namespaces using the given patterns. | ||
| // Patterns support glob-style matching with *, ?, and [...] syntax. | ||
| func filterNamespaces(available []string, patterns []string) []string { | ||
| seen := make(map[string]bool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary. engine.Namespaces() returns the unique list already.
| } | ||
| matched, err := path.Match(pattern, ns) | ||
| if err != nil { | ||
| continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should raise an error rather than silently continuing.
| } | ||
| } | ||
|
|
||
| func TestFilterNamespaces(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add t.Parallel().
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add t.Parallel().
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add t.Parallel().
| "testing" | ||
| ) | ||
|
|
||
| func TestHasWildcard(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add t.Parallel().
| return files, nil | ||
| } | ||
|
|
||
| // hasWildcard checks if any of the given patterns contain wildcard characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a note that this is based on what path.Match() supports.
| } | ||
|
|
||
| // filterNamespaces filters the available namespaces using the given patterns. | ||
| // Patterns support glob-style matching with *, ?, and [...] syntax. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a note that this is based on what path.Match() supports.
| // Patterns support glob-style matching with *, ?, and [...] syntax. | ||
| func filterNamespaces(available []string, patterns []string) []string { | ||
| seen := make(map[string]bool) | ||
| var result []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Call this namespaces rather than a generic variable name.
Fixes #1141
Changes