Skip to content

Commit

Permalink
Fix handling of 'has' selectors (ko-build#473)
Browse files Browse the repository at this point in the history
* Add tests to surface issue 467

* Fall back to empty map if object has no labels
  • Loading branch information
antoineco authored Dec 14, 2021
1 parent 84e8ab6 commit 66a77a9
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 9 deletions.
15 changes: 12 additions & 3 deletions pkg/resolve/selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ func MatchesSelector(doc *yaml.Node, selector labels.Selector) (bool, error) {
if err != nil {
return false, err
}
if kind == "" {
return false, nil
}

if kind == "List" {
return listMatchesSelector(doc, selector)
Expand Down Expand Up @@ -96,11 +99,12 @@ func objMatchesSelector(doc *yaml.Node, selector labels.Selector) bool {

node, ok := it()

if ok && selector.Matches(labelsNode{node}) {
return true
// Object has no metadata.labels, verify matching against an empty set.
if !ok {
node = emptyMapNode
}

return false
return selector.Matches(labelsNode{node})
}

func listMatchesSelector(doc *yaml.Node, selector labels.Selector) (bool, error) {
Expand Down Expand Up @@ -133,6 +137,11 @@ func listMatchesSelector(doc *yaml.Node, selector labels.Selector) (bool, error)
return len(matches) != 0, nil
}

var emptyMapNode = &yaml.Node{
Kind: yaml.MappingNode,
Tag: "!!map",
}

type labelsNode struct {
*yaml.Node
}
Expand Down
63 changes: 57 additions & 6 deletions pkg/resolve/selector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ var (
webSelector = selector(`app=web`)
notWebSelector = selector(`app!=web`)
nopSelector = selector(`foo!=bark`)

hasSelector = selector(`app`)
notHasSelector = selector(`!app`)
)

const (
Expand All @@ -44,6 +47,11 @@ metadata:
# comments should be preserved
app: db
name: rss-db
`
podNoLabel = `apiVersion: v1
kind: Pod
metadata:
name: rss-site
`
podList = `apiVersion: v1
kind: List
Expand Down Expand Up @@ -89,6 +97,21 @@ items:
labels:
app: db
name: rss-db
`
podListNoLabel = `apiVersion: v1
kind: List
metadata:
resourceVersion: ""
selfLink: ""
items:
- apiVersion: v1
kind: Pod
metadata:
name: rss-site
- apiVersion: v1
kind: Pod
metadata:
name: rss-db
`
)

Expand Down Expand Up @@ -116,6 +139,28 @@ func TestMatchesSelector(t *testing.T) {
selector: nopSelector,
output: dbPod,
matches: true,
}, {
desc: "single object with has selector",
input: webPod,
selector: hasSelector,
output: webPod,
matches: true,
}, {
desc: "single object with not-has selector",
input: webPod,
selector: notHasSelector,
matches: false,
}, {
desc: "single non-labeled object with has selector",
input: podNoLabel,
selector: hasSelector,
matches: false,
}, {
desc: "single non-labeled object with not-has selector",
input: podNoLabel,
selector: notHasSelector,
output: podNoLabel,
matches: true,
}, {
desc: "selector matching elements of list object",
input: podList,
Expand All @@ -128,22 +173,28 @@ func TestMatchesSelector(t *testing.T) {
selector: notWebSelector,
output: dbPodList,
matches: true,
}, {
desc: "has selector matching no non-labeled element of list object",
input: podListNoLabel,
selector: hasSelector,
matches: false,
}, {
desc: "not-has selector matching all non-labeled elements of list object",
input: podListNoLabel,
selector: notHasSelector,
output: podListNoLabel,
matches: true,
}, {
desc: "selector matching all elements of list object",
input: podList,
selector: labels.Everything(),
output: podList,
matches: true,
}, {
desc: "selector matching no elements of list object",
desc: "selector matching no element of list object",
input: podList,
selector: labels.Nothing(),
matches: false,
}, {
desc: "null node",
input: "!!null",
selector: labels.Everything(),
matches: false,
}}

for _, test := range tests {
Expand Down

0 comments on commit 66a77a9

Please sign in to comment.