Skip to content

Commit

Permalink
Improve cli-utils manifest reader logic (#92)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michaeljguarino authored Dec 13, 2023
1 parent 96808d4 commit 4254ef6
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 20 deletions.
2 changes: 1 addition & 1 deletion pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
78 changes: 78 additions & 0 deletions pkg/manifests/template/common.go
Original file line number Diff line number Diff line change
@@ -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
}
19 changes: 1 addition & 18 deletions pkg/manifests/template/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 16 additions & 1 deletion pkg/manifests/template/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
}

0 comments on commit 4254ef6

Please sign in to comment.