Apply suggestions from CR kyma-project#3

components/binding/charts/binding/templates/crds/targetkind.yaml

@@ -57,16 +57,24 @@ spec:
                 - labelsPath
               type: object
             status:
-              properties:
-                registered:
-                  type: boolean
               description: TargetKindStatus defines the observed state of TargetKind
+              properties:
+                lastProcessedTime:
+                  format: date-time
+                  type: string
+                phase:
+                  type: string
+                message:
+                  type: string
+              required:
+              - phase
+              - message
               type: object
           type: object
       served: true
       storage: true
       additionalPrinterColumns:
-      - name: Registered
-        type: boolean
+      - name: Status
+        type: string
         description: the field shows the current registration status of the resource
-        jsonPath: .status.registered
+        jsonPath: .status.phase

components/binding/internal/storage/storage.go

@@ -11,6 +11,7 @@ import (
 
 type ResourceData struct {
 	Schema      schema.GroupVersionResource
+	LabelsPath  string
 	LabelFields []string
 }
 
@@ -28,6 +29,7 @@ func NewKindStorage() *KindStorage {
 func newResourceData(gvr schema.GroupVersionResource, labelsPath string) *ResourceData {
 	return &ResourceData{
 		Schema:      gvr,
+		LabelsPath:  labelsPath,
 		LabelFields: strings.Split(labelsPath, "."),
 	}
 }
@@ -64,9 +66,19 @@ func (s *KindStorage) Get(kind v1alpha1.Kind) (*ResourceData, error) {
 	return concreteResourceData, nil
 }
 
-func (s *KindStorage) Exist(kind v1alpha1.Kind) bool {
+func (s *KindStorage) Exist(tk v1alpha1.TargetKind) bool {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
-	_, exists := s.registered[kind]
+	_, exists := s.registered[tk.Spec.Resource.Kind]
+	if !exists {
+		return false
+	}
 	return exists
 }
+
+func (s *KindStorage) Equal(tk v1alpha1.TargetKind, registeredTk *ResourceData) bool {
+	if tk.Spec.Resource.Group != registeredTk.Schema.Group || fmt.Sprintf("%s%s", tk.Spec.Resource.Kind, "s") != registeredTk.Schema.Resource || tk.Spec.Resource.Version != registeredTk.Schema.Version || tk.Spec.LabelsPath != registeredTk.LabelsPath {
+		return false
+	}
+	return true
+}

components/binding/internal/target/handler.go

@@ -9,7 +9,6 @@ import (
 	"github.com/kyma-project/kyma/components/binding/internal/worker"
 	"github.com/kyma-project/kyma/components/binding/pkg/apis/v1alpha1"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -29,15 +28,14 @@ func NewHandler(client dynamic.Interface, storage worker.TargetKindStorage) *Han
 }
 
 func (h *Handler) AddLabel(b *v1alpha1.Binding) error {
-	resourceData, err := h.storage.Get(b.Spec.Target.Kind)
+	resourceData, err := h.storage.Get(v1alpha1.Kind(b.Spec.Target.Kind))
 	if err != nil {
 		return errors.Wrapf(err, "while getting Kind %s from storage", b.Spec.Target.Kind)
 	}
 	resource, err := h.getResource(b, resourceData)
 	if err != nil {
 		return errors.Wrapf(err, "while getting resource for Binding %s", b.Name)
 	}
-	logrus.Infof("Got resource: %+v", resource)
 	labelsToApply := map[string]string{h.labelKey(b): uuid.New().String()}
 	if err := h.ensureLabelsAreApplied(resource, labelsToApply, resourceData.LabelFields); err != nil {
 		return errors.Wrap(err, "while ensuring labels are applied")
@@ -51,7 +49,7 @@ func (h *Handler) AddLabel(b *v1alpha1.Binding) error {
 }
 
 func (h *Handler) LabelExist(b *v1alpha1.Binding) (bool, error) {
-	resourceData, err := h.storage.Get(b.Spec.Target.Kind)
+	resourceData, err := h.storage.Get(v1alpha1.Kind(b.Spec.Target.Kind))
 	if err != nil {
 		return false, errors.Wrapf(err, "while getting Kind %s from storage", b.Spec.Target.Kind)
 	}
@@ -73,7 +71,7 @@ func (h *Handler) LabelExist(b *v1alpha1.Binding) (bool, error) {
 }
 
 func (h *Handler) RemoveOldAddNewLabel(b *v1alpha1.Binding) error {
-	resourceData, err := h.storage.Get(b.Spec.Target.Kind)
+	resourceData, err := h.storage.Get(v1alpha1.Kind(b.Spec.Target.Kind))
 	if err != nil {
 		return errors.Wrapf(err, "while getting Kind %s from storage", b.Spec.Target.Kind)
 	}
@@ -108,7 +106,7 @@ func (h *Handler) RemoveOldAddNewLabel(b *v1alpha1.Binding) error {
 }
 
 func (h *Handler) RemoveLabel(b *v1alpha1.Binding) error {
-	resourceData, err := h.storage.Get(b.Spec.Target.Kind)
+	resourceData, err := h.storage.Get(v1alpha1.Kind(b.Spec.Target.Kind))
 	if err != nil {
 		return errors.Wrapf(err, "while getting Kind %s from storage", b.Spec.Target.Kind)
 	}

components/binding/internal/worker/binding.go

@@ -115,7 +115,7 @@ func (b *BindingWorker) readyPhase(binding *v1alpha1.Binding, log log.FieldLogge
 	if binding.Status.Target != fmt.Sprintf("%s/%s", binding.Spec.Target.Kind, binding.Spec.Target.Name) {
 		log.Info("target was changed, removing label from old target")
 		bindingCopy := binding.DeepCopy()
-		bindingCopy.Spec.Target.Kind = v1alpha1.Kind(strings.Split(binding.Status.Target, "/")[0])
+		bindingCopy.Spec.Target.Kind = strings.Split(binding.Status.Target, "/")[0]
 		bindingCopy.Spec.Target.Name = strings.Split(binding.Status.Target, "/")[1]
 		err := b.kindManager.RemoveLabel(bindingCopy)
 		if err != nil {

components/binding/internal/worker/targetkind.go

@@ -1,11 +1,11 @@
 package worker
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/kyma-project/kyma/components/binding/internal/storage"
 	"github.com/kyma-project/kyma/components/binding/pkg/apis/v1alpha1"
+	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	"k8s.io/client-go/dynamic"
 )
@@ -14,7 +14,8 @@ type TargetKindStorage interface {
 	Register(tk v1alpha1.TargetKind) error
 	Unregister(tk v1alpha1.TargetKind) error
 	Get(kind v1alpha1.Kind) (*storage.ResourceData, error)
-	Exist(kind v1alpha1.Kind) bool
+	Exist(kind v1alpha1.TargetKind) bool
+	Equal(tk v1alpha1.TargetKind, registeredTk *storage.ResourceData) bool
 }
 
 type TargetKindWorker struct {
@@ -31,17 +32,51 @@ func NewTargetKindWorker(storage TargetKindStorage, dynamicClient dynamic.Interf
 func (w *TargetKindWorker) Process(targetKind *v1alpha1.TargetKind, log log.FieldLogger) (*v1alpha1.TargetKind, error) {
 	log.Info("start TargetKind process")
 
-	if targetKind.Status.Registered {
-		if w.storage.Exist(targetKind.Spec.Resource.Kind) {
-			log.Infof("TargetKind %q was already registered", targetKind.Name)
+	if targetKind.Status.IsRegistered() {
+		log.Infof("TargetKind %s was already registered", targetKind.Name)
+		if !w.storage.Exist(*targetKind) {
+			err := w.storage.Register(*targetKind)
+			if err != nil {
+				return targetKind, fmt.Errorf("while registering TargetKind %q", targetKind.Name)
+			}
+			err = targetKind.Status.Registered()
+			if err != nil {
+				return targetKind, errors.Wrapf(err, "while set TargetKind phase to %s", v1alpha1.TargetKindRegistered)
+			}
 			return targetKind, nil
 		}
+
+		hasChanged, err := w.isDifferentThanRegistered(targetKind)
+		if err != nil {
+			return targetKind, errors.Wrap(err, "while comparing processed TargetKind with registered one")
+		}
+		if !hasChanged {
+			log.Infof("TargetKind %s is not different than existing one", targetKind.Name)
+			return targetKind, nil
+		}
+
+		err = w.storage.Register(*targetKind)
+		if err != nil {
+			return targetKind, fmt.Errorf("while registering TargetKind %q", targetKind.Name)
+		}
+		err = targetKind.Status.Registered()
+		if err != nil {
+			return targetKind, errors.Wrapf(err, "while set TargetKind phase to %s", v1alpha1.TargetKindRegistered)
+		}
+
+		//TODO: mark old existing TargetKind CR as 'invalid'
+		return targetKind, nil
+
 	}
+	// TargetKind was not registered before
 	err := w.storage.Register(*targetKind)
 	if err != nil {
-		return &v1alpha1.TargetKind{}, errors.New(fmt.Sprintf("while registering TargetKind %q", targetKind.Spec.DisplayName))
+		return targetKind, errors.New(fmt.Sprintf("while registering TargetKind %q", targetKind.Name))
+	}
+	err = targetKind.Status.Registered()
+	if err != nil {
+		return targetKind, errors.Wrapf(err, "while set TargetKind phase to %s", v1alpha1.TargetKindRegistered)
 	}
-	targetKind.Status.Registered = true
 	log.Infof("TargetKind %q registered", targetKind.Name)
 	return targetKind, nil
 }
@@ -51,3 +86,14 @@ func (w *TargetKindWorker) RemoveProcess(targetKind *v1alpha1.TargetKind, log lo
 
 	return w.storage.Unregister(*targetKind)
 }
+
+func (w *TargetKindWorker) isDifferentThanRegistered(targetKind *v1alpha1.TargetKind) (bool, error) {
+	registered, err := w.storage.Get(targetKind.Spec.Resource.Kind)
+	if err != nil {
+		return false, errors.Wrap(err, "while getting ResourceData")
+	}
+	if !w.storage.Equal(*targetKind, registered) {
+		return true, nil
+	}
+	return false, nil
+}

components/binding/pkg/apis/v1alpha1/binding_types.go

@@ -36,7 +36,7 @@ type Source struct {
 }
 
 type Target struct {
-	Kind Kind `json:"kind"`
+	Kind string `json:"kind"`
 	Name string `json:"name"`
 }
 

components/binding/pkg/apis/v1alpha1/targetkind_types.go

@@ -1,28 +1,53 @@
 package v1alpha1
 
 import (
+	"time"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // Kind represents Kubernetes Kind name
 type Kind string
 
+// TargetKindPhase describes TargetKind phase
+type TargetKindPhase string
+
+const (
+	TargetKindRegistered = "Registered"
+)
+
 // TargetKindSpec defines the desired state of TargetKind
 type TargetKindSpec struct {
-	DisplayName string    `json:"displayName"`
+	DisplayName string   `json:"displayName"`
 	Resource    Resource `json:"resource"`
-	LabelsPath  string    `json:"labelsPath"`
+	LabelsPath  string   `json:"labelsPath"`
 }
 
 type Resource struct {
 	Group   string `json:"group"`
-	Kind    Kind `json:"kind"`
+	Kind    Kind   `json:"kind"`
 	Version string `json:"version"`
 }
 
 // TargetKindStatus defines the observed state of TargetKind
 type TargetKindStatus struct {
-	Registered bool `json:"registered"`
+	Phase             TargetKindPhase `json:"phase"`
+	Message           string          `json:"message"`
+	LastProcessedTime *metav1.Time    `json:"lastProcessedTime,omitempty"`
+}
+
+func (tks *TargetKindStatus) IsRegistered() bool {
+	if tks.Phase == TargetKindRegistered {
+		return true
+	}
+	return false
+}
+
+func (tks *TargetKindStatus) Registered() error {
+	tks.Phase = TargetKindRegistered
+	tks.LastProcessedTime = &metav1.Time{Time: time.Now()}
+
+	return nil
 }
 
 // +kubebuilder:object:root=true