use guuid for the instance id (#17)

pkg/controllers/landscaperdeployments/controller.go

@@ -9,6 +9,8 @@ import (
 	"fmt"
 	"reflect"
 
+	guuid "github.com/google/uuid"
+
 	kutils "github.com/gardener/landscaper/controller-utils/pkg/kubernetes"
 	"github.com/go-logr/logr"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -26,11 +28,28 @@ import (
 // Controller is the landscaperdeployments controller
 type Controller struct {
 	operation.Operation
+
+	UniqueIDFunc func() string
+}
+
+// NewUniqueID creates a new unique id string with a length of 8.
+func (c *Controller) NewUniqueID() string {
+	id := c.UniqueIDFunc()
+	if len(id) > 8 {
+		id = id[:8]
+	}
+	return id
+}
+
+func defaultUniqueIdFunc() string {
+	return guuid.New().String()
 }
 
 // NewController returns a new landscaperdeployments controller
 func NewController(log logr.Logger, c client.Client, scheme *runtime.Scheme, config *coreconfig.LandscaperServiceConfiguration) (reconcile.Reconciler, error) {
-	ctrl := &Controller{}
+	ctrl := &Controller{
+		UniqueIDFunc: defaultUniqueIdFunc,
+	}
 	op := operation.NewOperation(log, c, scheme, config)
 	ctrl.Operation = *op
 	return ctrl, nil
@@ -39,7 +58,8 @@ func NewController(log logr.Logger, c client.Client, scheme *runtime.Scheme, con
 // NewTestActuator creates a new controller for testing purposes.
 func NewTestActuator(op operation.Operation) *Controller {
 	ctrl := &Controller{
-		Operation: op,
+		Operation:    op,
+		UniqueIDFunc: defaultUniqueIdFunc,
 	}
 	return ctrl
 }

pkg/controllers/landscaperdeployments/reconcile.go

@@ -6,19 +6,19 @@ package landscaperdeployments
 
 import (
 	"context"
-	"crypto/sha1"
-	"encoding/hex"
 	"fmt"
 	"sort"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/labels"
-
-	"github.com/gardener/landscaper/controller-utils/pkg/kubernetes"
 	"github.com/go-logr/logr"
+
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
+	"github.com/gardener/landscaper/controller-utils/pkg/kubernetes"
+
 	lssv1alpha1 "github.com/gardener/landscaper-service/pkg/apis/core/v1alpha1"
 	lsserrors "github.com/gardener/landscaper-service/pkg/apis/errors"
 	"github.com/gardener/landscaper-service/pkg/utils"
@@ -97,13 +97,22 @@ func (c *Controller) mutateInstance(ctx context.Context, log logr.Logger, deploy
 		instance.Spec.ServiceTargetConfigRef.Namespace = serviceTargetConf.GetNamespace()
 	}
 
-	// The instance has a generated name and therefore may be not set yet.
-	// The hash is therefore calculated with the name of the deployment.
-	// Since the name of the instance is derived from the deployment name, which is unique in a namespace,
-	// the hash value is also unique.
-	h := sha1.New()
-	id := h.Sum([]byte(deployment.Name))
-	instance.Spec.ID = hex.EncodeToString(id[:4])
+	if len(instance.Spec.ID) == 0 {
+		instanceList := &lssv1alpha1.InstanceList{}
+		if err := c.Client().List(ctx, instanceList, &client.ListOptions{Namespace: deployment.Namespace}); err != nil {
+			return fmt.Errorf("unable to list instances in namespace %s: %w", deployment.Namespace, err)
+		}
+
+		existingIds := sets.NewString()
+		for _, i := range instanceList.Items {
+			existingIds.Insert(i.Spec.ID)
+		}
+
+		var id string
+		for id = c.NewUniqueID(); existingIds.Has(id); id = c.NewUniqueID() {
+		}
+		instance.Spec.ID = id
+	}
 
 	instance.Spec.TenantId = deployment.Spec.TenantId
 	instance.Spec.LandscaperConfiguration = deployment.Spec.LandscaperConfiguration

pkg/controllers/landscaperdeployments/reconcile_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	lssv1alpha1 "github.com/gardener/landscaper-service/pkg/apis/core/v1alpha1"
 	deploymentscontroller "github.com/gardener/landscaper-service/pkg/controllers/landscaperdeployments"
@@ -170,7 +169,7 @@ var _ = Describe("SortServiceTargetConfigs", func() {
 var _ = Describe("Reconcile", func() {
 	var (
 		op    *operation.Operation
-		ctrl  reconcile.Reconciler
+		ctrl  *deploymentscontroller.Controller
 		ctx   context.Context
 		state *envtest.State
 	)
@@ -222,6 +221,7 @@ var _ = Describe("Reconcile", func() {
 		Expect(instance.Spec.LandscaperConfiguration).To(Equal(deployment.Spec.LandscaperConfiguration))
 		Expect(instance.Spec.TenantId).To(Equal(deployment.Spec.TenantId))
 		Expect(instance.Spec.ID).To(MatchRegexp("[a-f0-9]+"))
+		Expect(instance.Spec.ID).To(HaveLen(8))
 
 		Expect(testenv.Client.Get(ctx, kutil.ObjectKeyFromObject(config), config)).To(Succeed())
 		Expect(config.Status.InstanceRefs).To(HaveLen(1))
@@ -270,4 +270,39 @@ var _ = Describe("Reconcile", func() {
 		Expect(instance.Spec.LandscaperConfiguration).To(Equal(deployment.Spec.LandscaperConfiguration))
 		Expect(instance.Spec.ID).To(Equal(uid))
 	})
+
+	It("should not create instances with duplicated ids", func() {
+		var err error
+
+		state, err = testenv.InitResources(ctx, "./testdata/reconcile/test5")
+		Expect(err).ToNot(HaveOccurred())
+
+		deployment := state.GetDeployment("test")
+		existingInstance := state.GetInstance("existing")
+
+		callCount := 0
+		uniqueId := "eb08fabb"
+		ctrl.UniqueIDFunc = func() string {
+			var id string
+			if callCount == 0 {
+				id = existingInstance.Spec.ID
+			} else {
+				id = uniqueId
+			}
+			callCount += 1
+			return id
+		}
+
+		testutils.ShouldReconcile(ctx, ctrl, testutils.RequestFromObject(deployment))
+		Expect(testenv.Client.Get(ctx, kutil.ObjectKeyFromObject(deployment), deployment)).To(Succeed())
+		testutils.ShouldReconcile(ctx, ctrl, testutils.RequestFromObject(deployment))
+		Expect(testenv.Client.Get(ctx, kutil.ObjectKeyFromObject(deployment), deployment)).To(Succeed())
+		Expect(deployment.Status.InstanceRef).ToNot(BeNil())
+
+		instance := &lssv1alpha1.Instance{}
+		err = testenv.Client.Get(ctx, types.NamespacedName{Name: deployment.Status.InstanceRef.Name, Namespace: deployment.Status.InstanceRef.Namespace}, instance)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(instance.Spec.ID).To(Equal(uniqueId))
+	})
 })

pkg/controllers/landscaperdeployments/testdata/reconcile/test5/deployment.yaml

@@ -0,0 +1,18 @@
+# SPDX-FileCopyrightText: 2021 "SAP SE or an SAP affiliate company and Gardener contributors"
+#
+# SPDX-License-Identifier: Apache-2.0
+
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: LandscaperDeployment
+metadata:
+  name: "test"
+  namespace: {{ .Namespace }}
+spec:
+  tenantId: "12345"
+  purpose: "test"
+  region: "eu"
+  landscaperConfiguration:
+    deployers:
+      - helm
+      - manifest
+      - container

pkg/controllers/landscaperdeployments/testdata/reconcile/test5/instance.yaml

@@ -0,0 +1,20 @@
+# SPDX-FileCopyrightText: 2021 "SAP SE or an SAP affiliate company and Gardener contributors"
+#
+# SPDX-License-Identifier: Apache-2.0
+
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: Instance
+metadata:
+  name: "existing"
+  namespace: {{ .Namespace }}
+spec:
+  tenantId: "12345"
+  id: "aabbccdd"
+  landscaperConfiguration:
+    deployers:
+      - helm
+      - manifest
+      - container
+  serviceTargetConfigRef:
+    name: "config1"
+    namespace: {{ .Namespace }}

pkg/controllers/landscaperdeployments/testdata/reconcile/test5/servicetargetconf.yaml

@@ -0,0 +1,89 @@
+# SPDX-FileCopyrightText: 2021 "SAP SE or an SAP affiliate company and Gardener contributors"
+#
+# SPDX-License-Identifier: Apache-2.0
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: target
+  namespace: {{ .Namespace }}
+type: Opaque
+stringData:
+  kubeconfig: |
+    dummy
+---
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: ServiceTargetConfig
+
+metadata:
+  name: config1
+  namespace: {{ .Namespace }}
+  labels:
+    config.landscaper-service.gardener.cloud/visible: "true"
+    config.landscaper-service.gardener.cloud/region: eu
+
+spec:
+  providerType: gcp
+  priority: 10
+
+  secretRef:
+    name: target
+    namespace: {{ .Namespace }}
+    key: kubeconfig
+---
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: ServiceTargetConfig
+
+metadata:
+  name: config2
+  namespace: {{ .Namespace }}
+  labels:
+    config.landscaper-service.gardener.cloud/visible: "true"
+    config.landscaper-service.gardener.cloud/region: eu
+
+spec:
+  providerType: gcp
+  priority: 20
+
+  secretRef:
+    name: target
+    namespace: {{ .Namespace }}
+    key: kubeconfig
+---
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: ServiceTargetConfig
+
+metadata:
+  name: config3
+  namespace: {{ .Namespace }}
+  labels:
+    config.landscaper-service.gardener.cloud/visible: "true"
+    config.landscaper-service.gardener.cloud/region: us
+
+spec:
+  providerType: gcp
+  priority: 30
+
+  secretRef:
+    name: target
+    namespace: {{ .Namespace }}
+    key: kubeconfig
+---
+apiVersion: landscaper-service.gardener.cloud/v1alpha1
+kind: ServiceTargetConfig
+
+metadata:
+  name: configt4
+  namespace: {{ .Namespace }}
+  labels:
+    config.landscaper-service.gardener.cloud/visible: "false"
+    config.landscaper-service.gardener.cloud/region: eu
+
+spec:
+  providerType: gcp
+  priority: 30
+
+  secretRef:
+    name: target
+    namespace: {{ .Namespace }}
+    key: kubeconfig