filters: add basic pattern matching for label keys i.e --filter label=<pattern>
#12295
Conversation
flouthoc
force-pushed
the
filter-label-pattern
branch
from
ca61170 to
877fe45
Compare
Following PR adds basic pattern matching to filter by labels for `keys`. Adds support for use-cases like `--filter label=some.prefix.com/key/*` where end-users want to match a pattern for keys as compared to exact value. Signed-off-by: Aditya Rajan <[email protected]>
flouthoc
force-pushed
the
filter-label-pattern
branch
from
877fe45 to
c050f05
Compare
| func matchPattern(pattern string, value string) bool { | ||
| if strings.Contains(pattern, "*") { | ||
| result, _ := regexp.MatchString(wildCardToRegexp(pattern), value) | ||
| return result | ||
| } | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
does it need to handle only *?
Perhaps you could use filepath.Match?
There was a problem hiding this comment.
Yes I think that would be much better. otherwise you might end up with more regex failures.
There was a problem hiding this comment.
I thought of it first but we want to match label keys where filepath.Match does not works for cases like. Filepath is very restricted to matching valid path. However my idea was to match for patterns.
This fails with filepath.Match and few more cases.
fmt.Println(filepath.Match("/home/c*", "/home/catch/foo"))
There was a problem hiding this comment.
how does Docker do it?
If Docker doesn't do it, could we just use a regex here?
There was a problem hiding this comment.
I don't think docker does it. This was a new feature request. Afaik people mostly use pipe and grep with docker for that.
There was a problem hiding this comment.
I tend to agree that a regex is better - it's definitely more complicated but attempting to treat labels as a path seems very fragile?
Besides, Podman does absolutely no standardization of label formats, so there's neither guarantee nor obligation for labels to include slashes as separators.
There was a problem hiding this comment.
@giuseppe I think regex like regexp.MatchString("hello", key) would match all keys starting with hello so --filter label= becomes hard to use. But I'll try it out.
There was a problem hiding this comment.
My problem with regex is almost no human beings understand it and it has a lot more symbols.
People understand globing , well at least () a lot better then (.)
There was a problem hiding this comment.
Docker has a very small documentation but it only talks about * in examples https://docs.docker.com/engine/reference/commandline/images/#filtering . We could do a regex but lot of tests would need to be readjusted.
https://kliushnikov.medium.com/filtering-docker-images-5eb5aee358df
There was a problem hiding this comment.
Nvm found a fix also confirmed with @giuseppe I'll amend the PR.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@rhatdan I wasn't able to amend the PR yet. I still wanted to make the change but no issues i am creating a follow up PR. |
github-actions
bot
added
the
locked - please file new issue/PR
Following PR adds basic pattern matching to filter by labels for
keys.Adds support for use-cases like
--filter label=some.prefix.com/key/*where end-users want to match a pattern for keys as compared to exact
value.
Closes: #12199