Skip to content

Commit

Permalink
Fix controller issue in handling empty pod labels for labelidentity (#…
Browse files Browse the repository at this point in the history
…5404)

When a Pod is created without any labels at all, its labelidentity
will include "pod:<none>" as defined by FormatLabels in apimachinery.
This case is not handled correctly by the label group index and can
cause the controller to crash.

Signed-off-by: Dyanngg <dingyang@vmware.com>
  • Loading branch information
Dyanngg authored Aug 22, 2023
1 parent bd3c008 commit 3e2e829
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func GetNormalizedLabel(nsLabels, podLabels map[string]string, ns string) string
// label is guaranteed to have Namespace name information.
nsLabels[v1.LabelMetadataName] = ns
}
return "ns:" + labels.FormatLabels(nsLabels) + "&pod:" + labels.FormatLabels(podLabels)
return "ns:" + labels.Set(nsLabels).String() + "&pod:" + labels.Set(podLabels).String()
}

// getResourceExportNameForLabelIdentity retrieves the ResourceExport name for exporting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,13 @@ func TestGetNormalizedLabel(t *testing.T) {
map[string]string{"region": "west"},
"ns:kubernetes.io/metadata.name=test-ns,region=west&pod:purpose=test",
},
{
"no Pod label",
"test-ns",
map[string]string{},
map[string]string{"region": "west"},
"ns:kubernetes.io/metadata.name=test-ns,region=west&pod:",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/controller/labelidentity/label_group_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ func (l *labelIdentityMatch) matches(s *selectorItem) bool {
// constructMapFromLabelString parses label string of format "app=client,env=dev" into a map.
func constructMapFromLabelString(s string) map[string]string {
m := map[string]string{}
if s == "" {
return m
}
kvs := strings.Split(s, ",")
for _, kv := range kvs {
kvpair := strings.Split(kv, "=")
Expand Down
44 changes: 44 additions & 0 deletions pkg/controller/labelidentity/label_group_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ var (
selectorC = types.NewGroupSelector("", pSelDB, nil, nil, nil)
selectorD = types.NewGroupSelector("testing", pSelDB, nil, nil, nil)
selectorE = types.NewGroupSelector("random", pSelDB, nil, nil, nil)
selectorF = types.NewGroupSelector("", nil, nsSelTest, nil, nil)
selectorG = types.NewGroupSelector("testing", nil, nil, nil, nil)
selectorItemA = &selectorItem{
selector: selectorA,
}
Expand All @@ -57,9 +59,16 @@ var (
selectorItemE = &selectorItem{
selector: selectorE,
}
selectorItemF = &selectorItem{
selector: selectorF,
}
selectorItemG = &selectorItem{
selector: selectorG,
}
labelA = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=web"
labelB = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=db"
labelC = "ns:kubernetes.io/metadata.name=nomatch,purpose=nomatch&pod:app=db"
labelD = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:"
)

func TestLabelIdentityMatch(t *testing.T) {
Expand Down Expand Up @@ -128,6 +137,41 @@ func TestLabelIdentityMatch(t *testing.T) {
selector: selectorItemE,
expectMatch: false,
},
{
label: labelD,
selector: selectorItemA,
expectMatch: false,
},
{
label: labelD,
selector: selectorItemB,
expectMatch: false,
},
{
label: labelD,
selector: selectorItemD,
expectMatch: false,
},
{
label: labelD,
selector: selectorItemF,
expectMatch: true,
},
{
label: labelA,
selector: selectorItemG,
expectMatch: true,
},
{
label: labelC,
selector: selectorItemG,
expectMatch: false,
},
{
label: labelD,
selector: selectorItemG,
expectMatch: true,
},
}
for _, tt := range tests {
labelMatch := newLabelIdentityMatch(tt.label, 1)
Expand Down

0 comments on commit 3e2e829

Please sign in to comment.