Skip to content

Commit

Permalink
Revert "🐛 hasLabels and matchingLabels step on each other (kubernetes…
Browse files Browse the repository at this point in the history
…-sigs#2363)"

This reverts commit eb78e57.
  • Loading branch information
timflannagan committed Oct 6, 2023
1 parent 8361246 commit 59beef7
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 60 deletions.
20 changes: 5 additions & 15 deletions pkg/client/options.go
Expand Up @@ -513,15 +513,8 @@ type MatchingLabels map[string]string
// ApplyToList applies this configuration to the given list options.
func (m MatchingLabels) ApplyToList(opts *ListOptions) {
// TODO(directxman12): can we avoid reserializing this over and over?
if opts.LabelSelector == nil {
opts.LabelSelector = labels.NewSelector()
}
// If there's already a selector, we need to AND the two together.
noValidSel := labels.SelectorFromValidatedSet(map[string]string(m))
reqs, _ := noValidSel.Requirements()
for _, req := range reqs {
opts.LabelSelector = opts.LabelSelector.Add(req)
}
sel := labels.SelectorFromValidatedSet(map[string]string(m))
opts.LabelSelector = sel
}

// ApplyToDeleteAllOf applies this configuration to the given an List options.
Expand All @@ -535,17 +528,14 @@ type HasLabels []string

// ApplyToList applies this configuration to the given list options.
func (m HasLabels) ApplyToList(opts *ListOptions) {
if opts.LabelSelector == nil {
opts.LabelSelector = labels.NewSelector()
}
// TODO: ignore invalid labels will result in an empty selector.
// This is inconsistent to the that of MatchingLabels.
sel := labels.NewSelector()
for _, label := range m {
r, err := labels.NewRequirement(label, selection.Exists, nil)
if err == nil {
opts.LabelSelector = opts.LabelSelector.Add(*r)
sel = sel.Add(*r)
}
}
opts.LabelSelector = sel
}

// ApplyToDeleteAllOf applies this configuration to the given an List options.
Expand Down
45 changes: 0 additions & 45 deletions pkg/client/options_test.go
Expand Up @@ -237,19 +237,6 @@ var _ = Describe("MatchingLabels", func() {
expectedErrMsg := `values[0][k]: Invalid value: "axahm2EJ8Phiephe2eixohbee9eGeiyees1thuozi1xoh0GiuH3diewi8iem7Nui": must be no more than 63 characters`
Expect(err.Error()).To(Equal(expectedErrMsg))
})

It("Should add matchingLabels to existing selector", func() {
listOpts := &client.ListOptions{}

matchingLabels := client.MatchingLabels(map[string]string{"k": "v"})
matchingLabels2 := client.MatchingLabels(map[string]string{"k2": "v2"})

matchingLabels.ApplyToList(listOpts)
Expect(listOpts.LabelSelector.String()).To(Equal("k=v"))

matchingLabels2.ApplyToList(listOpts)
Expect(listOpts.LabelSelector.String()).To(Equal("k=v,k2=v2"))
})
})

var _ = Describe("FieldOwner", func() {
Expand Down Expand Up @@ -305,35 +292,3 @@ var _ = Describe("ForceOwnership", func() {
Expect(*o.Force).To(BeTrue())
})
})

var _ = Describe("HasLabels", func() {
It("Should produce hasLabels in given order", func() {
listOpts := &client.ListOptions{}

hasLabels := client.HasLabels([]string{"labelApe", "labelFox"})
hasLabels.ApplyToList(listOpts)
Expect(listOpts.LabelSelector.String()).To(Equal("labelApe,labelFox"))
})

It("Should add hasLabels to existing hasLabels selector", func() {
listOpts := &client.ListOptions{}

hasLabel := client.HasLabels([]string{"labelApe"})
hasLabel.ApplyToList(listOpts)

hasOtherLabel := client.HasLabels([]string{"labelFox"})
hasOtherLabel.ApplyToList(listOpts)
Expect(listOpts.LabelSelector.String()).To(Equal("labelApe,labelFox"))
})

It("Should add hasLabels to existing matchingLabels", func() {
listOpts := &client.ListOptions{}

matchingLabels := client.MatchingLabels(map[string]string{"k": "v"})
matchingLabels.ApplyToList(listOpts)

hasLabel := client.HasLabels([]string{"labelApe"})
hasLabel.ApplyToList(listOpts)
Expect(listOpts.LabelSelector.String()).To(Equal("k=v,labelApe"))
})
})

0 comments on commit 59beef7

Please sign in to comment.