Remove old CRD stored versions when upgrading (#71)

* If upgrading CRD has different versions than the ones stored on the server, remove the previously stored versions as stored versions

* Support older github workflow for now which is missing chartVersion from the pkg download

Makefile

@@ -1,4 +1,4 @@
-VERSION ?= 0.3.0
+VERSION ?= 0.7.0
 
 COMMIT := $(shell git rev-parse --short HEAD)
 DATE := $(shell date +%Y%m%d)
@@ -14,7 +14,7 @@ SHELL = /usr/bin/env bash -o pipefail
 .SHELLFLAGS = -ec
 
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.28.x
+ENVTEST_K8S_VERSION = 1.31.x
 
 GO_FLAGS ?= -v
 
@@ -83,7 +83,7 @@ $(LOCALBIN):
 GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
 ENVTEST ?= $(LOCALBIN)/setup-envtest
 
-GOLINT_VERSION ?= 1.56.2
+GOLINT_VERSION ?= 1.61.0
 
 .PHONY: envtest
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.

pkg/helmutil/crds.go

@@ -8,12 +8,15 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	"github.com/charmbracelet/log"
 	"github.com/pkg/errors"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	deser "k8s.io/apimachinery/pkg/runtime/serializer/yaml"
 	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
 
@@ -54,7 +57,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, chartVersion string) ([]unstruct
 	}
 
 	if fs, err := os.Stat(chartDir); os.IsNotExist(err) {
-		log.Info("Downloading chart release from remote repository", "repoURL", u.repoURL, "chartName", u.chartName, "chartVersion", chartVersion)
+		log.Info("Downloading chart release from remote repository", "repoURL", u.repoURL, "chartName", u.chartName, "chartVersion", chartVersion, "chartDir", chartDir)
 		downloadDir, err := DownloadChartRelease(u.repoName, u.repoURL, u.chartName, chartVersion)
 		if err != nil {
 			return nil, err
@@ -102,6 +105,51 @@ func (u *Upgrader) Upgrade(ctx context.Context, chartVersion string) ([]unstruct
 		} else {
 			log.Debug("Updating CustomResourceDefinition", "name", obj.GetName())
 			obj.SetResourceVersion(existingCrd.GetResourceVersion())
+
+			// TODO We need to check which versions we have available here before updating
+			unstructured := obj.UnstructuredContent()
+			var definition apiextensionsv1.CustomResourceDefinition
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &definition); err != nil {
+				return nil, errors.Wrapf(err, "failed to convert unstructured to CustomResourceDefinition %s", obj.GetName())
+			}
+
+			updatedVersions := make([]string, 0, len(definition.Spec.Versions))
+			for _, version := range definition.Spec.Versions {
+				updatedVersions = append(updatedVersions, version.Name)
+			}
+			log.Debug("Read CustomResourceDefinition versions", "name", obj.GetName(), "versions", updatedVersions)
+
+			existing := existingCrd.UnstructuredContent()
+			var existingDefinition apiextensionsv1.CustomResourceDefinition
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(existing, &existingDefinition); err != nil {
+				return nil, errors.Wrapf(err, "failed to convert unstructured to CustomResourceDefinition %s", obj.GetName())
+			}
+
+			storedVersions := existingDefinition.Status.StoredVersions
+
+			if !slices.Equal(storedVersions, updatedVersions) {
+
+				// Check if storedVersion has any versions that are not in updatedVersions
+				// If so, we need to remove them from the storedVersions
+				removed := false
+				for _, storedVersion := range storedVersions {
+					if !slices.Contains(updatedVersions, storedVersion) {
+						log.Debug("Removing CustomResourceDefinition version", "name", obj.GetName(), "version", storedVersion)
+						// storedVersions = slices.DeleteFunc(storedVersions, func(e string) bool { return e == storedVersion })
+						removed = true
+					}
+				}
+
+				if removed {
+					log.Debug("Updating CustomResourceDefinition versions", "name", obj.GetName(), "storedVersions", storedVersions, "updatedVersions", updatedVersions)
+					existingDefinition.Status.StoredVersions = updatedVersions
+					if err := u.client.Status().Update(ctx, &existingDefinition); err != nil {
+						return nil, errors.Wrapf(err, "failed to update CRD storedVersions %s", obj.GetName())
+					}
+					obj.SetResourceVersion(existingDefinition.GetResourceVersion())
+				}
+			}
+
 			if err = u.client.Update(ctx, &obj); err != nil {
 				return nil, errors.Wrapf(err, "failed to update CRD %s", obj.GetName())
 			}

pkg/helmutil/crds_test.go

@@ -2,12 +2,17 @@ package helmutil_test
 
 import (
 	"context"
+	"fmt"
+	"io"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/k8ssandra/k8ssandra-client/pkg/helmutil"
+	"github.com/k8ssandra/k8ssandra-client/pkg/util"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -85,3 +90,156 @@ func cleanCache(repoName, chartName string) error {
 
 	return os.RemoveAll(chartDir)
 }
+
+func TestUpgradingStoredVersions(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode.")
+	}
+
+	require := require.New(t)
+	chartName := "test-chart"
+	namespace := env.CreateNamespace(t)
+	kubeClient := env.GetClientInNamespace(namespace)
+	require.NoError(cleanCache("k8ssandra", chartName))
+
+	// Copy testfiles
+	chartDir, err := helmutil.GetChartTargetDir(helmutil.K8ssandraRepoName, chartName)
+	require.NoError(err)
+
+	crdDir := filepath.Join(chartDir, "crds")
+	_, err = util.CreateIfNotExistsDir(crdDir)
+	require.NoError(err)
+	crdSrc := filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1alpha1.yaml")
+	require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml")))
+
+	testOptions := envtest.CRDInstallOptions{
+		PollInterval: 100 * time.Millisecond,
+		MaxTime:      10 * time.Second,
+	}
+
+	// creating new upgrader
+	u, err := helmutil.NewUpgrader(kubeClient, helmutil.K8ssandraRepoName, helmutil.StableK8ssandraRepoURL, chartName, []string{})
+	require.NoError(err)
+
+	crds, err := u.Upgrade(context.TODO(), "0.1.0")
+	require.NoError(err)
+
+	targetCrd := &apiextensions.CustomResourceDefinition{}
+	objs := []*apiextensions.CustomResourceDefinition{}
+	for _, crd := range crds {
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd)
+		require.NoError(err)
+		objs = append(objs, targetCrd)
+	}
+
+	require.NotEmpty(objs)
+	require.NotEmpty(targetCrd.GetName())
+	require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions))
+	require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd))
+
+	require.Equal([]string{"v1alpha1"}, targetCrd.Status.StoredVersions)
+
+	// Upgrade to 0.2.0
+
+	require.NoError(cleanCache("k8ssandra", chartName))
+	_, err = util.CreateIfNotExistsDir(crdDir)
+	require.NoError(err)
+	crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-both.yaml")
+	require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml")))
+
+	crds, err = u.Upgrade(context.TODO(), "0.2.0")
+	require.NoError(err)
+	for _, crd := range crds {
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd)
+		require.NoError(err)
+		objs = append(objs, targetCrd)
+	}
+	require.NotEmpty(objs)
+	require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions))
+	require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd))
+	require.Equal([]string{"v1alpha1", "v1beta1"}, targetCrd.Status.StoredVersions)
+
+	// Upgrade to 0.3.0
+
+	require.NoError(cleanCache("k8ssandra", chartName))
+	_, err = util.CreateIfNotExistsDir(crdDir)
+	require.NoError(err)
+	crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1beta1.yaml")
+	require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml")))
+
+	crds, err = u.Upgrade(context.TODO(), "0.3.0")
+	require.NoError(err)
+	for _, crd := range crds {
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd)
+		require.NoError(err)
+		objs = append(objs, targetCrd)
+	}
+	require.NotEmpty(objs)
+	require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions))
+	require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd))
+	require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions)
+
+	// Sanity check, install 0.2.0 and only update to 0.3.0 (there should be no storedVersion of v1alpha1)
+	require.NoError(kubeClient.Delete(context.TODO(), targetCrd))
+	require.Eventually(func() bool {
+		err = kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)
+		return err != nil && client.IgnoreNotFound(err) == nil
+	}, time.Second*5, time.Millisecond*100)
+
+	// Install 0.2.0
+
+	require.NoError(cleanCache("k8ssandra", chartName))
+	_, err = util.CreateIfNotExistsDir(crdDir)
+	require.NoError(err)
+	crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-both.yaml")
+	require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml")))
+
+	crds, err = u.Upgrade(context.TODO(), "0.2.0")
+	require.NoError(err)
+	for _, crd := range crds {
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd)
+		require.NoError(err)
+		objs = append(objs, targetCrd)
+	}
+	require.NotEmpty(objs)
+	require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions))
+	require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd))
+	require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions)
+
+	// Upgrade to 0.3.0
+
+	require.NoError(cleanCache("k8ssandra", chartName))
+	_, err = util.CreateIfNotExistsDir(crdDir)
+	require.NoError(err)
+	crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1beta1.yaml")
+	require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml")))
+
+	crds, err = u.Upgrade(context.TODO(), "0.3.0")
+	require.NoError(err)
+	for _, crd := range crds {
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd)
+		require.NoError(err)
+		objs = append(objs, targetCrd)
+	}
+	require.NotEmpty(objs)
+	require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions))
+	require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd))
+	require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions)
+}
+
+func copyFile(source, target string) error {
+	src, err := os.Open(source)
+	if err != nil {
+		return errors.Wrap(err, fmt.Sprintf("failed to open %s", source))
+	}
+	defer src.Close()
+
+	dst, err := os.Create(target)
+	if err != nil {
+		return errors.Wrap(err, fmt.Sprintf("failed to open %s", target))
+	}
+	defer dst.Close()
+
+	_, err = io.Copy(dst, src)
+	return err
+}

testfiles/crd-upgrader/multiversion-clientconfig-mockup-both.yaml

@@ -0,0 +1,103 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: v0.14.0
+  name: clientconfigmocks.config.k8ssandra.io
+spec:
+  group: config.k8ssandra.io
+  names:
+    kind: ClientConfigMock
+    listKind: ClientConfigMockList
+    plural: clientconfigmocks
+    singular: clientconfigmock
+  scope: Namespaced
+  versions:
+  - name: v1alpha1
+    schema:
+      openAPIV3Schema:
+        description: ClientConfig is the Schema for the kubeconfigs API
+        properties:
+          apiVersion:
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
+            type: string
+          kind:
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: ClientConfigSpec defines the desired state of KubeConfig
+            properties:
+              kubeConfigSecret:
+                description: |-
+                  KubeConfigSecret should reference an existing secret; the actual configuration will be read from
+                  this secret's "kubeconfig" key.
+                properties:
+                  name:
+                    description: |-
+                      Name of the referent.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      TODO: Add other useful fields. apiVersion, kind, uid?
+                    type: string
+                type: object
+                x-kubernetes-map-type: atomic
+            type: object
+        type: object
+    served: true
+    storage: false
+  - name: v1beta1
+    schema:
+      openAPIV3Schema:
+        description: ClientConfig is the Schema for the kubeconfigs API
+        properties:
+          apiVersion:
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
+            type: string
+          kind:
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: ClientConfigSpec defines the desired state of KubeConfig
+            properties:
+              contextName:
+                description: ContextName allows to override the object name for context-name.
+                  If not set, the ClientConfig.Name is used as context name
+                type: string
+              kubeConfigSecret:
+                description: |-
+                  KubeConfigSecret should reference an existing secret; the actual configuration will be read from
+                  this secret's "kubeconfig" key.
+                properties:
+                  name:
+                    description: |-
+                      Name of the referent.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      TODO: Add other useful fields. apiVersion, kind, uid?
+                    type: string
+                type: object
+                x-kubernetes-map-type: atomic
+            type: object
+        type: object
+    served: true
+    storage: true