Workaround obsolete API versions

This is a workaround for changes in API versions like
extensions/v1beta1 => apps/v1

Problem:
From Helm's perspective those are new Resources as the API version
cannot be changed. However there is a compat layer in k8s that will
return the old resources for the new API version. Helm will reject the
resource 'creation' as there is already a resource of the same kind and
version.

Solution:
In general ignore the Version.
Due to the fact that there might be clashes of Kind we track CRD and
none-CRD resources seperate.

Open issue:
Corner case with same Kind in different CRDs and a pre-existing resource
that would clash with the one created by Helm.

Signed-off-by: Sebastian Poehn <sebastian.poehn@gmail.com>

pkg/action/upgrade.go

@@ -202,19 +202,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea
 		return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest")
 	}
 
-	// Do a basic diff using gvk + name to figure out what new resources are being created so we can validate they don't already exist
+	toBeCreated := detectNewResources(&current, &target)
-	existingResources := make(map[string]bool)
-	for _, r := range current {
-		existingResources[objectKey(r)] = true
-	}
-
-	var toBeCreated kube.ResourceList
-	for _, r := range target {
-		if !existingResources[objectKey(r)] {
-			toBeCreated = append(toBeCreated, r)
-		}
-	}
-
 	if err := existingResourceConflict(toBeCreated); err != nil {
 		return nil, errors.Wrap(err, "rendered manifests contain a new resource that already exists. Unable to continue with update")
 	}
@@ -425,6 +413,54 @@ func recreate(cfg *Configuration, resources kube.ResourceList) error {
 }
 
 func objectKey(r *resource.Info) string {
-	gvk := r.Object.GetObjectKind().GroupVersionKind()
+	return fmt.Sprintf("%s/%s/%s", objectKind(r), r.Namespace, r.Name)
-	return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name)
+}
+
+func detectNewResources(current *kube.ResourceList, target *kube.ResourceList) kube.ResourceList {
+	existingResources := make(map[string]string)
+	for _, r := range *current {
+		existingResources[objectKey(r)] = objectGroupVersion(r)
+	}
+
+	var toBeCreated kube.ResourceList
+	for _, r := range *target {
+		e := existingResources[objectKey(r)]
+		// Generally we do not care about object versions
+		// Kubernetes has a compat layer that handles migrations
+		// However the same Kind might exist in diffent Groups
+		// In that case we have actually a new Resource
+		if e == "" || isCRD(e) != isCRD(objectGroupVersion(r)) {
+			toBeCreated = append(toBeCreated, r)
+		}
+	}
+
+	return toBeCreated
+}
+
+func objectGroupVersion(r *resource.Info) string {
+	return r.Object.GetObjectKind().GroupVersionKind().GroupVersion().String()
+}
+
+func objectKind(r *resource.Info) string {
+	return r.Object.GetObjectKind().GroupVersionKind().Kind
+}
+
+func isCRD(groupVersion string) bool {
+	s := strings.Split(groupVersion, "/")
+	if len(s) == 1 {
+		// "v1"
+		return false
+	}
+	group := s[0]
+	s = strings.Split(group, ".")
+	if len(s) == 1 {
+		// "apps/v1"
+		return false
+	}
+	if s[len(s)-2] == "k8s" && s[len(s)-1] == "io" {
+		// "networking.k8s.io/v1"
+		return false
+	}
+	// "crd.projectcalico.org/v1"
+	return true
 }

pkg/action/upgrade_test.go

@@ -18,9 +18,11 @@ package action
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/kube"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -253,3 +255,57 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) {
 		is.Equal(expectedValues, updatedRes.Config)
 	})
 }
+
+func TestIsCRD(t *testing.T) {
+	is := assert.New(t)
+	is.True(isCRD("crd.projectcalico.org/v1"))
+	is.True(isCRD("kafka.strimzi.io/v1alpha1"))
+	is.False(isCRD("v1"))
+	is.False(isCRD("app/v1"))
+	is.False(isCRD("banana/v17"))
+	is.False(isCRD("extensions/v1beta1"))
+	is.False(isCRD("networking.k8s.io/v1"))
+}
+
+func TestDetectNewResources(t *testing.T) {
+	t.Skip("This is a live test, comment this line to run")
+	is := assert.New(t)
+	c := kube.New(nil)
+	test1KubernetesNetworkPolicyV1, err := c.Build(strings.NewReader(test1KubernetesNetworkPolicyV1Manifest), false)
+	is.NoError(err)
+	test1KubernetesNetworkPolicyV1Beta1, err := c.Build(strings.NewReader(test1KubernetesNetworkPolicyV1Beta1Manifest), false)
+	is.NoError(err)
+	test2KubernetesNetworkPolicyV1, err := c.Build(strings.NewReader(test2KubernetesNetworkPolicyV1Manifest), false)
+	is.NoError(err)
+
+	// Update of resource
+	is.Nil(detectNewResources(&test1KubernetesNetworkPolicyV1, &test1KubernetesNetworkPolicyV1))
+	// Update of resource - API version changed
+	is.Nil(detectNewResources(&test1KubernetesNetworkPolicyV1Beta1, &test1KubernetesNetworkPolicyV1))
+	// New resource was added
+	is.Equal(test2KubernetesNetworkPolicyV1, detectNewResources(&test1KubernetesNetworkPolicyV1, &test2KubernetesNetworkPolicyV1))
+}
+
+const test1KubernetesNetworkPolicyV1Manifest = `
+kind: NetworkPolicy
+apiVersion: networking.k8s.io/v1
+metadata:
+  name: test1
+spec: {}
+`
+
+const test1KubernetesNetworkPolicyV1Beta1Manifest = `
+kind: NetworkPolicy
+apiVersion: extensions/v1beta1
+metadata:
+  name: test1
+spec: {}
+`
+
+const test2KubernetesNetworkPolicyV1Manifest = `
+kind: NetworkPolicy
+apiVersion: networking.k8s.io/v1
+metadata:
+  name: test2
+spec: {}
+`