From 72648427b304cf2852e2044af2f43c19154d5221 Mon Sep 17 00:00:00 2001 From: Fredrik Sommar Date: Thu, 27 Jun 2024 11:53:59 +0200 Subject: [PATCH 1/2] Propagate ResourceGroup labels and annotations When I declare a ResourceGroup on disk, I expect the labels and annotations defined on it to be propagated to the in-cluster inventory. That was previously not the case. kioutil annotations are removed (path, index, etc) since they are redundant for inventories that are typically defined in one place, and don't need to be reconstructed by a function pipeline. Signed-off-by: Fredrik Sommar --- internal/pkg/pkg.go | 24 ++++++++ internal/testutil/pkgbuilder/builder.go | 10 +++- pkg/live/load.go | 10 +++- pkg/live/load_test.go | 78 +++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 2 deletions(-) diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index f43a8bb191..0821ad5e36 100644 --- a/internal/pkg/pkg.go +++ b/internal/pkg/pkg.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "sort" + "strings" "github.com/GoogleContainerTools/kpt/internal/errors" "github.com/GoogleContainerTools/kpt/internal/types" @@ -736,9 +737,14 @@ func (p *Pkg) LocalInventory() (kptfilev1.Inventory, error) { if !hasKptfile { // Return the ResourceGroup object as inventory. if len(resources) == 1 { + labels := resources[0].GetLabels() + delete(labels, rgfilev1alpha1.RGInventoryIDLabel) + return kptfilev1.Inventory{ Name: resources[0].GetName(), Namespace: resources[0].GetNamespace(), + Annotations: resources[0].GetAnnotations(), + Labels: labels, InventoryID: resources[0].GetLabels()[rgfilev1alpha1.RGInventoryIDLabel], }, nil } @@ -764,9 +770,14 @@ func (p *Pkg) LocalInventory() (kptfilev1.Inventory, error) { // ResourceGroup stores the inventory and Kptfile does not contain inventory. if len(resources) == 1 { + labels := resources[0].GetLabels() + delete(labels, rgfilev1alpha1.RGInventoryIDLabel) + return kptfilev1.Inventory{ Name: resources[0].GetName(), Namespace: resources[0].GetNamespace(), + Annotations: resources[0].GetAnnotations(), + Labels: labels, InventoryID: resources[0].GetLabels()[rgfilev1alpha1.RGInventoryIDLabel], }, nil } @@ -788,8 +799,21 @@ func filterResourceGroups(input []*yaml.RNode) (output []*yaml.RNode, err error) continue } + ClearInternalAnnotations(meta.Annotations) + if err := r.SetAnnotations(meta.Annotations); err != nil { + return nil, fmt.Errorf("failed to set annotations for resource %w", err) + } + output = append(output, r) } return output, nil } + +func ClearInternalAnnotations(annotations map[string]string) { + for k := range annotations { + if strings.HasPrefix(k, "internal.config.kubernetes.io/") || strings.HasPrefix(k, "config.kubernetes.io/") { + delete(annotations, k) + } + } +} diff --git a/internal/testutil/pkgbuilder/builder.go b/internal/testutil/pkgbuilder/builder.go index 8132a25263..0c96e9e901 100644 --- a/internal/testutil/pkgbuilder/builder.go +++ b/internal/testutil/pkgbuilder/builder.go @@ -309,6 +309,7 @@ func (sp *SubPkg) WithSubPackages(ps ...*SubPkg) *SubPkg { // RGFile represents a minimal resourcegroup. type RGFile struct { Name, Namespace, ID string + Annotations, Labels map[string]string } func NewRGFile() *RGFile { @@ -547,8 +548,15 @@ func buildRGFile(pkg *pkg) string { tmp := rgfilev1alpha1.ResourceGroup{ResourceMeta: rgfilev1alpha1.DefaultMeta} tmp.ObjectMeta.Name = pkg.RGFile.Name tmp.ObjectMeta.Namespace = pkg.RGFile.Namespace + tmp.ObjectMeta.Annotations = pkg.RGFile.Annotations + tmp.ObjectMeta.Labels = pkg.RGFile.Labels + + if tmp.ObjectMeta.Labels == nil { + tmp.ObjectMeta.Labels = make(map[string]string) + } + if pkg.RGFile.ID != "" { - tmp.ObjectMeta.Labels = map[string]string{rgfilev1alpha1.RGInventoryIDLabel: pkg.RGFile.ID} + tmp.ObjectMeta.Labels[rgfilev1alpha1.RGInventoryIDLabel] = pkg.RGFile.ID } b, err := yaml.MarshalWithOptions(tmp, &yaml.EncoderOptions{SeqIndent: yaml.WideSequenceStyle}) diff --git a/pkg/live/load.go b/pkg/live/load.go index da1b587e10..e0282230fd 100644 --- a/pkg/live/load.go +++ b/pkg/live/load.go @@ -135,10 +135,18 @@ func readInvInfoFromStream(in io.Reader) (kptfilev1.Inventory, error) { } // Inventory found with ResourceGroup object. if len(rgFilter.Inventories) == 1 { - invID := rgFilter.Inventories[0].Labels[rgfilev1alpha1.RGInventoryIDLabel] + labels := rgFilter.Inventories[0].Labels + invID := labels[rgfilev1alpha1.RGInventoryIDLabel] + delete(labels, rgfilev1alpha1.RGInventoryIDLabel) + + annotations := rgFilter.Inventories[0].Annotations + pkg.ClearInternalAnnotations(annotations) + return kptfilev1.Inventory{ Name: rgFilter.Inventories[0].Name, Namespace: rgFilter.Inventories[0].Namespace, + Annotations: annotations, + Labels: labels, InventoryID: invID, }, nil } diff --git a/pkg/live/load_test.go b/pkg/live/load_test.go index 5c8f51305e..a21524a03a 100644 --- a/pkg/live/load_test.go +++ b/pkg/live/load_test.go @@ -209,6 +209,36 @@ func TestLoad_LocalDisk(t *testing.T) { }, rgFile: "resourcegroup.yaml", }, + "Inventory information taken from resourcegroup in file": { + pkg: pkgbuilder.NewRootPkg(). + WithKptfile( + pkgbuilder.NewKptfile(), + ).WithRGFile(&pkgbuilder.RGFile{ + Name: "foo", + Namespace: "bar", + Annotations: map[string]string{ + "additional-annotation": "true", + }, + Labels: map[string]string{ + "cli-utils.sigs.k8s.io/inventory-id": "foo-bar", + "additional-label": "true", + }, + }), + namespace: "foo", + expectedObjs: []object.ObjMetadata{}, + expectedInv: kptfile.Inventory{ + Name: "foo", + Namespace: "bar", + InventoryID: "foo-bar", + Annotations: map[string]string{ + "additional-annotation": "true", + }, + Labels: map[string]string{ + "additional-label": "true", + }, + }, + rgFile: "inventory.yaml", + }, } for tn, tc := range testCases { @@ -239,6 +269,14 @@ func TestLoad_LocalDisk(t *testing.T) { }) assert.Equal(t, tc.expectedObjs, objMetas) + // Simplify testing against empty labels and annotations + if len(inv.Labels) == 0 { + inv.Labels = nil + } + if len(inv.Annotations) == 0 { + inv.Annotations = nil + } + assert.Equal(t, tc.expectedInv, inv) }) } @@ -380,6 +418,39 @@ func TestLoad_StdIn(t *testing.T) { }, }, }, + "Inventory using STDIN resourcegroup file with annotations": { + pkg: pkgbuilder.NewRootPkg(). + WithKptfile( + pkgbuilder.NewKptfile(), + ). + WithFile("cm.yaml", configMap). + WithRGFile(&pkgbuilder.RGFile{ + Name: "foo", + Namespace: "bar", + ID: "foo-bar", + Annotations: map[string]string{ + "extra-annotation": "hello-world", + }, + }), + namespace: "foo", + expectedInv: kptfile.Inventory{ + Name: "foo", + Namespace: "bar", + InventoryID: "foo-bar", + Annotations: map[string]string{ + "extra-annotation": "hello-world", + }, + }, + expectedObjs: []object.ObjMetadata{ + { + GroupKind: schema.GroupKind{ + Kind: "ConfigMap", + }, + Name: "cm", + Namespace: "foo", + }, + }, + }, "Multiple inventories using STDIN resourcegroup and Kptfile is error": { pkg: pkgbuilder.NewRootPkg(). WithKptfile( @@ -479,6 +550,13 @@ func TestLoad_StdIn(t *testing.T) { }) assert.Equal(t, tc.expectedObjs, objMetas) + // Simplify testing against empty labels and annotations + if len(inv.Annotations) == 0 { + inv.Annotations = nil + } + if len(inv.Labels) == 0 { + inv.Labels = nil + } assert.Equal(t, tc.expectedInv, inv) }) } From feb91a693d3ebb179cc425428fb497aded23df9e Mon Sep 17 00:00:00 2001 From: Fredrik Sommar Date: Thu, 27 Jun 2024 13:52:56 +0200 Subject: [PATCH 2/2] Replace busybox with no-op function in test Signed-off-by: Fredrik Sommar --- internal/fnruntime/container_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/fnruntime/container_test.go b/internal/fnruntime/container_test.go index 9333e02d36..8e85670906 100644 --- a/internal/fnruntime/container_test.go +++ b/internal/fnruntime/container_test.go @@ -36,8 +36,9 @@ func TestContainerFn(t *testing.T) { err bool }{ { - name: "simple busybox", - image: "gcr.io/google-containers/busybox", + name: "no-op function", + image: "gcr.io/kpt-functions/no-op", + output: "apiVersion: v1\nkind: ResourceList\nmetadata:\n name: output\nitems: []\n", }, { name: "non-existing image",