Skip to content

Commit 17e577a

Browse files
nicksnicks
authored
k8s: fix filter_yaml on crds (#6613)
fixes #6611 Signed-off-by: Nick Santos <[email protected]>
1 parent 33fc9d7 commit 17e577a

File tree

5 files changed

+115
-14
lines changed

5 files changed

+115
-14
lines changed

‎internal/k8s/extract.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"reflect"
66

77
v1 "k8s.io/api/core/v1"
8+
"k8s.io/apimachinery/pkg/api/meta"
89
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
)
1011

@@ -42,22 +43,30 @@ func ExtractPodTemplateSpec(obj interface{}) ([]*v1.PodTemplateSpec, error) {
4243
return result, nil
4344
}
4445

45-
func extractObjectMetas(obj interface{}, filter func(v reflect.Value) bool) ([]*metav1.ObjectMeta, error) {
46+
func extractObjectMetas(obj interface{}, filter func(v reflect.Value) bool) ([]metav1.Object, error) {
4647
extracted, err := newExtractor(reflect.TypeOf(metav1.ObjectMeta{})).
4748
withFilter(filter).
4849
extractPointersFrom(obj)
4950
if err != nil {
5051
return nil, err
5152
}
5253

53-
result := make([]*metav1.ObjectMeta, len(extracted))
54+
result := make([]metav1.Object, len(extracted))
5455
for i, e := range extracted {
5556
c, ok := e.(*metav1.ObjectMeta)
5657
if !ok {
5758
return nil, fmt.Errorf("ExtractObjectMetas: expected ObjectMeta, actual %T", e)
5859
}
5960
result[i] = c
6061
}
62+
63+
// if this is unstructured, use the built-in accessors
64+
if len(result) == 0 {
65+
accessor, err := meta.Accessor(obj)
66+
if err == nil {
67+
result = append(result, accessor)
68+
}
69+
}
6170
return result, nil
6271
}
6372

‎internal/k8s/label.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
appsv1beta2 "k8s.io/api/apps/v1beta2"
88
v1 "k8s.io/api/core/v1"
99
extv1beta1 "k8s.io/api/extensions/v1beta1"
10+
"k8s.io/apimachinery/pkg/api/meta"
1011
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1112
"k8s.io/apimachinery/pkg/runtime"
1213

@@ -52,15 +53,20 @@ func injectLabels(entity K8sEntity, labels []model.LabelPair, overwrite bool) (K
5253
// Don't modify persistent volume claims
5354
// because they're supposed to be immutable.
5455
pvc := reflect.TypeOf(v1.PersistentVolumeClaim{})
55-
metas, err := extractObjectMetas(&entity, func(v reflect.Value) bool {
56+
metas, err := extractObjectMetas(entity.Obj, func(v reflect.Value) bool {
5657
return v.Type() != pvc
5758
})
5859
if err != nil {
5960
return K8sEntity{}, err
6061
}
6162

6263
for _, meta := range metas {
63-
applyLabelsToMap(&meta.Labels, labels, overwrite, true)
64+
metaLabels := meta.GetLabels()
65+
if metaLabels == nil {
66+
metaLabels = make(map[string]string, 0)
67+
}
68+
applyLabelsToMap(&metaLabels, labels, overwrite, true)
69+
meta.SetLabels(metaLabels)
6470
}
6571

6672
switch obj := entity.Obj.(type) {
@@ -200,22 +206,18 @@ func (e K8sEntity) SelectorMatchesLabels(labels map[string]string) bool {
200206
// MatchesMetadataLabels indicates whether the given label(s) are a subset
201207
// of metadata labels for the given entity.
202208
func (e K8sEntity) MatchesMetadataLabels(labels map[string]string) (bool, error) {
203-
// TODO(nick): This doesn't seem right.
204-
// This should only look at the top-level obj meta, not all of them.
205-
metas, err := extractObjectMetas(&e, NoFilter)
209+
meta, err := meta.Accessor(e.Obj)
206210
if err != nil {
207211
return false, err
208212
}
209213

210-
for _, meta := range metas {
211-
for k, v := range labels {
212-
realVal, ok := meta.Labels[k]
213-
if !ok || realVal != v {
214-
return false, nil
215-
}
214+
metaLabels := meta.GetLabels()
215+
for k, v := range labels {
216+
realVal, ok := metaLabels[k]
217+
if !ok || realVal != v {
218+
return false, nil
216219
}
217220
}
218221

219222
return true, nil
220-
221223
}

‎internal/k8s/label_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
extbeta1 "k8s.io/api/extensions/v1beta1"
7+
"k8s.io/apimachinery/pkg/api/meta"
78

89
"k8s.io/api/apps/v1beta1"
910

@@ -274,6 +275,32 @@ func TestInjectService(t *testing.T) {
274275
})
275276
}
276277

278+
func TestInjectLabelAPIService(t *testing.T) {
279+
entity := parseOneEntity(t, testyaml.APIServiceYAML)
280+
lps := []model.LabelPair{
281+
{
282+
Key: "tier",
283+
Value: "test",
284+
},
285+
}
286+
newEntity, err := InjectLabels(entity, lps)
287+
if err != nil {
288+
t.Fatal(err)
289+
}
290+
291+
meta, err := meta.Accessor(newEntity.Obj)
292+
require.NoError(t, err)
293+
labels := meta.GetLabels()
294+
assert.Equal(t, map[string]string{
295+
"app.kubernetes.io/instance": "metrics-server",
296+
"app.kubernetes.io/managed-by": "Helm",
297+
"app.kubernetes.io/name": "metrics-server",
298+
"app.kubernetes.io/version": "0.7.1",
299+
"helm.sh/chart": "metrics-server-3.12.1",
300+
"tier": "test",
301+
}, labels)
302+
}
303+
277304
func TestSelectorMatchesLabels(t *testing.T) {
278305
entities, err := ParseYAMLFromString(testyaml.BlorgBackendYAML)
279306
if err != nil {

‎internal/k8s/testyaml/testyaml.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,3 +1697,18 @@ spec:
16971697
image: gcr.io/knative-releases/knative.dev/serving/cmd/queue@sha256:713bd548700bf7fe5452969611d1cc987051bd607d67a4e7623e140f06c209b2
16981698
16991699
`
1700+
1701+
const APIServiceYAML = `
1702+
apiVersion: apiregistration.k8s.io/v1
1703+
kind: APIService
1704+
metadata:
1705+
labels:
1706+
app.kubernetes.io/instance: metrics-server
1707+
app.kubernetes.io/managed-by: Helm
1708+
app.kubernetes.io/name: metrics-server
1709+
app.kubernetes.io/version: 0.7.1
1710+
helm.sh/chart: metrics-server-3.12.1
1711+
name: v1beta1.metrics.k8s.io
1712+
spec:
1713+
group: metrics.k8s.io
1714+
`

‎internal/tiltfile/tiltfile_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,54 @@ k8s_yaml(doggos)
15011501
f.assertNoMoreManifests()
15021502
}
15031503

1504+
func TestFilterYamlByLabelCustomResource(t *testing.T) {
1505+
f := newFixture(t)
1506+
f.file("Tiltfile", `
1507+
test="""
1508+
apiVersion: apiregistration.k8s.io/v1
1509+
kind: APIService
1510+
metadata:
1511+
labels:
1512+
app.kubernetes.io/instance: metrics-server
1513+
app.kubernetes.io/managed-by: Helm
1514+
app.kubernetes.io/name: metrics-server
1515+
app.kubernetes.io/version: 0.7.1
1516+
helm.sh/chart: metrics-server-3.12.1
1517+
name: v1beta1.metrics.k8s.io
1518+
spec:
1519+
group: metrics.k8s.io
1520+
groupPriorityMinimum: 100
1521+
insecureSkipTLSVerify: true
1522+
service:
1523+
name: metrics-server
1524+
namespace: kube-system
1525+
port: 443
1526+
version: v1beta1
1527+
versionPriority: 100
1528+
---
1529+
apiVersion: v1
1530+
kind: Namespace
1531+
metadata:
1532+
labels:
1533+
app.kubernetes.io/instance: longhorn
1534+
app.kubernetes.io/managed-by: Helm
1535+
app.kubernetes.io/name: longhorn
1536+
name: longhorn-system
1537+
"""
1538+
a, b = filter_yaml(
1539+
blob(test),
1540+
labels={"app.kubernetes.io/name": "longhorn"}
1541+
)
1542+
print('a: %d' % len(decode_yaml_stream(a)))
1543+
print('b: %d' % len(decode_yaml_stream(b)))
1544+
`)
1545+
1546+
f.load()
1547+
1548+
require.Contains(t, f.out.String(), "a: 1")
1549+
require.Contains(t, f.out.String(), "b: 1")
1550+
}
1551+
15041552
func TestFilterYamlByName(t *testing.T) {
15051553
f := newFixture(t)
15061554
f.file("k8s.yaml", yaml.ConcatYAML(

0 commit comments

Comments
 (0)