fix: Helm - only default to release namespace for namespaced resources

pkg/collector/helm.go

@@ -2,12 +2,16 @@ package collector
 
 import (
 	"fmt"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/discovery"
 
 	"github.com/ghodss/yaml"
+	"github.com/rs/zerolog/log"
 	"helm.sh/helm/v3/pkg/releaseutil"
 )
 
-func parseManifests(manifest string, defaultNamespace string) ([]map[string]interface{}, error) {
+func parseManifests(manifest string, defaultNamespace string, discoveryClient discovery.DiscoveryInterface) ([]map[string]interface{}, error) {
 	var results []map[string]interface{}
 
 	manifests := releaseutil.SplitManifests(manifest)
@@ -20,22 +24,35 @@ func parseManifests(manifest string, defaultNamespace string) ([]map[string]inte
 			return nil, err
 		}
 
-		fixNamespace(&manifest, defaultNamespace)
+		fixNamespace(&unstructured.Unstructured{Object: manifest}, defaultNamespace, discoveryClient)
 
 		results = append(results, manifest)
 	}
 
 	return results, nil
 }
 
-func fixNamespace(manifest *map[string]interface{}, defaultNamespace string) {
+func fixNamespace(resource *unstructured.Unstructured, defaultNamespace string, discoveryClient discovery.DiscoveryInterface) {
 	// Default to the release namespace if the manifest doesn't have the namespace set
-	if meta, ok := (*manifest)["metadata"]; ok {
-		switch v := meta.(type) {
-		case map[string]interface{}:
-			if val, ok := v["namespace"]; !ok || val == nil {
-				v["namespace"] = defaultNamespace
-			}
+	if resource.GetNamespace() == "" && isResourceNamespaced(discoveryClient, resource.GroupVersionKind()) {
+		resource.SetNamespace(defaultNamespace)
+	}
+}
+
+func isResourceNamespaced(discoveryClient discovery.DiscoveryInterface, gvk schema.GroupVersionKind) bool {
+	rs, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
+	// It seems discovery client fails with error if resource is not found
+	// this can happen, but is not fatal, we should notify user but continue
+	if err != nil {
+		log.Warn().Msgf("failed to discover supported resources for %s: %v", gvk.GroupVersion(), err)
+		return false
+	}
+
+	for _, r := range rs.APIResources {
+		if r.Kind == gvk.Kind {
+			return r.Namespaced
 		}
 	}
+
+	return false
 }

pkg/collector/helm2.go

@@ -68,7 +68,7 @@ func (c *HelmV2Collector) Get() ([]map[string]interface{}, error) {
 	var results []map[string]interface{}
 
 	for _, r := range releases {
-		if manifests, err := parseManifests(r.Manifest, r.Namespace); err != nil {
+		if manifests, err := parseManifests(r.Manifest, r.Namespace, c.discoveryClient); err != nil {
 			log.Warn().Msgf("failed to parse release %s/%s: %v", r.Namespace, r.Name, err)
 		} else {
 			results = append(results, manifests...)

pkg/collector/helm3.go

@@ -67,7 +67,7 @@ func (c *HelmV3Collector) Get() ([]map[string]interface{}, error) {
 	var results []map[string]interface{}
 
 	for _, r := range releases {
-		if manifests, err := parseManifests(r.Manifest, r.Namespace); err != nil {
+		if manifests, err := parseManifests(r.Manifest, r.Namespace, c.discoveryClient); err != nil {
 			log.Warn().Msgf("failed to parse release %s/%s: %v", r.Namespace, r.Name, err)
 		} else {
 			results = append(results, manifests...)

pkg/collector/helm_test.go

@@ -1,10 +1,24 @@
 package collector
 
 import (
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"reflect"
 	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/fake"
 )
 
+var FAKE_API_RESOURCES = &metav1.APIResourceList{
+	GroupVersion: "v1",
+	APIResources: []metav1.APIResource{
+		{Name: "pods", Namespaced: true, Kind: "Pod"},
+		{Name: "services", Namespaced: true, Kind: "Service"},
+		{Name: "namespaces", Namespaced: false, Kind: "Namespace"},
+	},
+}
+
 func TestSplitManifests(t *testing.T) {
 	testCases := []struct {
 		name     string
@@ -24,7 +38,8 @@ func TestSplitManifests(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			manifests, err := parseManifests(tc.input, "default")
+			fcs := fake.NewSimpleClientset()
+			manifests, err := parseManifests(tc.input, "default", fcs.Discovery())
 
 			if err != nil {
 				t.Fatalf("Expected to successfully parse manifests: %v", err)
@@ -45,23 +60,25 @@ func TestFixNamespace(t *testing.T) {
 		expected string                 // kinds of objects
 	}{
 		{"present",
-			map[string]interface{}{"metadata": map[string]interface{}{"namespace": "some-namespace"}},
+			map[string]interface{}{"apiVersion": "v1", "kind": "Pod", "metadata": map[string]interface{}{"namespace": "some-namespace"}},
 			"some-namespace",
 		},
 		{"missing",
-			map[string]interface{}{"metadata": map[string]interface{}{}},
+			map[string]interface{}{"apiVersion": "v1", "kind": "Pod", "metadata": map[string]interface{}{}},
 			"default-namespace",
 		},
 		{"nil",
-			map[string]interface{}{"metadata": map[string]interface{}{"namespace": nil}},
+			map[string]interface{}{"apiVersion": "v1", "kind": "Pod", "metadata": map[string]interface{}{"namespace": nil}},
 			"default-namespace",
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			fixNamespace(&tc.input, tc.expected)
+			fcs := fake.NewSimpleClientset()
+			fcs.Resources = []*metav1.APIResourceList{FAKE_API_RESOURCES}
+			fixNamespace(&unstructured.Unstructured{tc.input}, tc.expected, fcs.Discovery())
 
 			meta, ok := tc.input["metadata"]
 			if !ok {
@@ -85,3 +102,29 @@ func TestFixNamespace(t *testing.T) {
 		})
 	}
 }
+
+func Test_isResourceNamespaced(t *testing.T) {
+	tests := []struct {
+		name string
+		gvk  string
+		want bool
+	}{
+		{"true-pod", "Pod.v1.", true},
+		{"false-namespace", "Namespace.v1.", false},
+		{"false-non-existent", "XXX.v1.", false},
+	}
+
+	fcs := fake.NewSimpleClientset()
+	fcs.Resources = []*metav1.APIResourceList{FAKE_API_RESOURCES}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gvk, _ := schema.ParseKindArg(tt.gvk)
+
+			got := isResourceNamespaced(fcs.Discovery(), *gvk)
+			if got != tt.want {
+				t.Errorf("isResourceNamespaced() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}