From 4254ef614a5ceb8e0289d51a802e2d095dd6cfc6 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Tue, 12 Dec 2023 23:32:38 -0500 Subject: [PATCH] Improve cli-utils manifest reader logic (#92) cli-utils currently bails if a cluster scoped resource has a namespace field, which is not in-line with common helm behavior (many charts put namespaces in there even if they shouldn't). This reimplements the reader and namespace setting logic to just set the namespace to empty string and log that it was wrong originally. --- pkg/agent/agent.go | 2 +- pkg/manifests/template/common.go | 78 ++++++++++++++++++++++++++++++++ pkg/manifests/template/helm.go | 19 +------- pkg/manifests/template/reader.go | 17 ++++++- 4 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 pkg/manifests/template/common.go diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 26c84743..fdba55f0 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -101,7 +101,7 @@ func (agent *Agent) Run() { } }() - err := wait.PollInfinite(agent.refresh, func() (done bool, err error) { + err := wait.PollImmediateInfinite(agent.refresh, func() (done bool, err error) { if err := agent.socket.Join(); err != nil { log.Error(err, "could not establish websocket to upstream") } diff --git a/pkg/manifests/template/common.go b/pkg/manifests/template/common.go new file mode 100644 index 00000000..a30e00a1 --- /dev/null +++ b/pkg/manifests/template/common.go @@ -0,0 +1,78 @@ +package template + +import ( + "errors" + "fmt" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/cli-utils/pkg/inventory" + "sigs.k8s.io/cli-utils/pkg/manifestreader" + "sigs.k8s.io/cli-utils/pkg/object" +) + +func setNamespaces(mapper meta.RESTMapper, objs []*unstructured.Unstructured, + defaultNamespace string, enforceNamespace bool) error { + var crdObjs []*unstructured.Unstructured + + // find any crds in the set of resources. + for _, obj := range objs { + if object.IsCRD(obj) { + crdObjs = append(crdObjs, obj) + } + } + + var unknownGVKs []schema.GroupVersionKind + for _, obj := range objs { + // Exclude any inventory objects here since we don't want to change + // their namespace. + if inventory.IsInventoryObject(obj) { + continue + } + + // Look up the scope of the resource so we know if the resource + // should have a namespace set or not. + scope, err := object.LookupResourceScope(obj, crdObjs, mapper) + if err != nil { + var unknownTypeError *object.UnknownTypeError + if errors.As(err, &unknownTypeError) { + // If no scope was found, just add the resource type to the list + // of unknown types. + unknownGVKs = append(unknownGVKs, unknownTypeError.GroupVersionKind) + continue + } + // If something went wrong when looking up the scope, just + // give up. + return err + } + + switch scope { + case meta.RESTScopeNamespace: + if obj.GetNamespace() == "" { + obj.SetNamespace(defaultNamespace) + } else { + ns := obj.GetNamespace() + if enforceNamespace && ns != defaultNamespace { + return &manifestreader.NamespaceMismatchError{ + Namespace: ns, + RequiredNamespace: defaultNamespace, + } + } + } + case meta.RESTScopeRoot: + if ns := obj.GetNamespace(); ns != "" { + obj.SetNamespace("") + fmt.Printf("Found cluster scoped resource %s with namespace %s, coerced to un-namespaced\n", obj.GetName(), ns) + } + default: + return fmt.Errorf("unknown RESTScope %q", scope.Name()) + } + } + if len(unknownGVKs) > 0 { + return &manifestreader.UnknownTypesError{ + GroupVersionKinds: unknownGVKs, + } + } + return nil +} diff --git a/pkg/manifests/template/helm.go b/pkg/manifests/template/helm.go index a133e66f..1b55094f 100644 --- a/pkg/manifests/template/helm.go +++ b/pkg/manifests/template/helm.go @@ -27,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/util/homedir" "k8s.io/kubectl/pkg/cmd/util" - "sigs.k8s.io/cli-utils/pkg/manifestreader" "sigs.k8s.io/yaml" "github.com/samber/lo" @@ -89,28 +88,12 @@ func (h *helm) Render(svc *console.ServiceDeploymentExtended, utilFactory util.F } r := bytes.NewReader(out) - mapper, err := utilFactory.ToRESTMapper() if err != nil { return nil, err } - readerOptions := manifestreader.ReaderOptions{ - Mapper: mapper, - Namespace: svc.Namespace, - EnforceNamespace: false, - } - mReader := &manifestreader.StreamManifestReader{ - ReaderName: "helm", - Reader: r, - ReaderOptions: readerOptions, - } - - items, err := mReader.Read() - if err != nil { - return nil, err - } - return items, nil + return streamManifests(r, mapper, "helm", svc.Namespace) } func (h *helm) values(svc *console.ServiceDeploymentExtended) (map[string]interface{}, error) { diff --git a/pkg/manifests/template/reader.go b/pkg/manifests/template/reader.go index cdcb6094..e2ca726f 100644 --- a/pkg/manifests/template/reader.go +++ b/pkg/manifests/template/reader.go @@ -29,6 +29,21 @@ type StreamManifestReader struct { ReaderOptions } +func streamManifests(in io.Reader, mapper meta.RESTMapper, name, namespace string) ([]*unstructured.Unstructured, error) { + readerOptions := ReaderOptions{ + Mapper: mapper, + Namespace: namespace, + EnforceNamespace: false, + } + mReader := &StreamManifestReader{ + ReaderName: name, + Reader: in, + ReaderOptions: readerOptions, + } + + return mReader.Read([]*unstructured.Unstructured{}) +} + // Read reads the manifests and returns them as Info objects. func (r *StreamManifestReader) Read(objs []*unstructured.Unstructured) ([]*unstructured.Unstructured, error) { nodes, err := (&kio.ByteReader{ @@ -52,6 +67,6 @@ func (r *StreamManifestReader) Read(objs []*unstructured.Unstructured) ([]*unstr objs = manifestreader.FilterLocalConfig(objs) - err = manifestreader.SetNamespaces(r.Mapper, objs, r.Namespace, r.EnforceNamespace) + err = setNamespaces(r.Mapper, objs, r.Namespace, r.EnforceNamespace) return objs, err }