diff --git a/apis/observedobjectcollection/v1alpha1/types.go b/apis/observedobjectcollection/v1alpha1/types.go index a8ef7fe0..3ac93c4a 100644 --- a/apis/observedobjectcollection/v1alpha1/types.go +++ b/apis/observedobjectcollection/v1alpha1/types.go @@ -33,6 +33,7 @@ import ( // +kubebuilder:printcolumn:name="READY",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" // +kubebuilder:resource:scope=Cluster,categories={crossplane,managed,kubernetes} +// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) < 64",message="metadata.name max length is 63" type ObservedObjectCollection struct { v1.TypeMeta `json:",inline"` v1.ObjectMeta `json:"metadata,omitempty"` @@ -109,14 +110,9 @@ type ObservedObjectTemplateMetadata struct { type ObservedObjectCollectionStatus struct { v12.ResourceStatus `json:",inline"` - // List of object references mathing the given selector - // +optional - // +listType=map - // +listMapKey=name - ObjectRefs []ObservedObjectReference `json:"objectRefs,omitempty"` - // MembershipLabel is the label set on each member of this collection // and can be used for fetching them. + // +optional MembershipLabel map[string]string `json:"membershipLabel,omitempty"` } diff --git a/apis/observedobjectcollection/v1alpha1/zz_generated.deepcopy.go b/apis/observedobjectcollection/v1alpha1/zz_generated.deepcopy.go index c58f4033..76572f75 100644 --- a/apis/observedobjectcollection/v1alpha1/zz_generated.deepcopy.go +++ b/apis/observedobjectcollection/v1alpha1/zz_generated.deepcopy.go @@ -125,11 +125,6 @@ func (in *ObservedObjectCollectionSpec) DeepCopy() *ObservedObjectCollectionSpec func (in *ObservedObjectCollectionStatus) DeepCopyInto(out *ObservedObjectCollectionStatus) { *out = *in in.ResourceStatus.DeepCopyInto(&out.ResourceStatus) - if in.ObjectRefs != nil { - in, out := &in.ObjectRefs, &out.ObjectRefs - *out = make([]ObservedObjectReference, len(*in)) - copy(*out, *in) - } if in.MembershipLabel != nil { in, out := &in.MembershipLabel, &out.MembershipLabel *out = make(map[string]string, len(*in)) diff --git a/internal/controller/observedobjectcollection/reconciler.go b/internal/controller/observedobjectcollection/reconciler.go index 88dabc24..b37676e1 100644 --- a/internal/controller/observedobjectcollection/reconciler.go +++ b/internal/controller/observedobjectcollection/reconciler.go @@ -51,7 +51,7 @@ import ( const ( errNewKubernetesClient = "cannot create new Kubernetes client" errStatusUpdate = "cannot update status" - fieldOwner = client.FieldOwner("observed-object-collection-controller") + fieldOwner = client.FieldOwner("kubernetes.crossplane.io/observed-object-collection-controller") membershipLabelKey = "kubernetes.crossplane.io/owned-by-collection" ) @@ -125,14 +125,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct log.Info("Reconciling") - mlv, err := membershipLabelValue(c) - if err != nil { - werr := errors.Wrapf(err, "error getting value for label %v", membershipLabelKey) - c.Status.SetConditions(xpv1.ReconcileError(werr)) - _ = r.client.Status().Update(ctx, c) - return ctrl.Result{}, werr - } - // Get client for the referenced provider config. clusterClient, err := r.clientForProvider(ctx, r.client, c.Spec.ProviderConfigReference.Name) if err != nil { @@ -143,9 +135,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct } // Fetch objects based on the set GVK and selector. - objects := &unstructured.UnstructuredList{} - objects.SetAPIVersion(c.Spec.ObserveObjects.APIVersion) - objects.SetKind(c.Spec.ObserveObjects.Kind) + k8sobjects := &unstructured.UnstructuredList{} + k8sobjects.SetAPIVersion(c.Spec.ObserveObjects.APIVersion) + k8sobjects.SetKind(c.Spec.ObserveObjects.Kind) selector, err := metav1.LabelSelectorAsSelector(&c.Spec.ObserveObjects.Selector) if err != nil { @@ -155,17 +147,28 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct return ctrl.Result{}, werr } - if err := clusterClient.List(ctx, objects, &client.ListOptions{LabelSelector: selector, Namespace: c.Spec.ObserveObjects.Namespace}); err != nil { - werr := errors.Wrap(err, "error listing objects") + lo := client.ListOptions{LabelSelector: selector, Namespace: c.Spec.ObserveObjects.Namespace} + if err := clusterClient.List(ctx, k8sobjects, &lo); err != nil { + werr := errors.Wrapf(err, "error fetching objects for GVK %v and options %v", k8sobjects.GetObjectKind().GroupVersionKind(), lo) + c.Status.SetConditions(xpv1.ReconcileError(werr)) + _ = r.client.Status().Update(ctx, c) + return ctrl.Result{}, werr + } + + // Fetch any existing counter-part observe only Objects by collection label. + ml := map[string]string{membershipLabelKey: c.Name} + ol := &v1alpha2.ObjectList{} + if err := r.client.List(ctx, ol, client.MatchingLabels(ml)); err != nil { + werr := errors.Wrapf(err, "cannot list members matching labels %v", ml) c.Status.SetConditions(xpv1.ReconcileError(werr)) _ = r.client.Status().Update(ctx, c) return ctrl.Result{}, werr } - // Create observed-only Objects for all found items. + // Create/update observed-only Objects for all found items. refs := sets.New[v1alpha1.ObservedObjectReference]() - for i := range objects.Items { - o := objects.Items[i] + for i := range k8sobjects.Items { + o := k8sobjects.Items[i] log.Debug("creating observed object for the matched item", "gvk", o.GroupVersionKind(), "name", o.GetName()) name, err := r.observedObjectName(c, &o) if err != nil { @@ -183,12 +186,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct _ = r.client.Status().Update(ctx, c) return ctrl.Result{}, werr } - if l := po.GetLabels(); l != nil { - l[membershipLabelKey] = mlv - po.SetLabels(l) - } else { - po.SetLabels(map[string]string{membershipLabelKey: mlv}) - } if err := r.client.Patch(ctx, po, client.Apply, fieldOwner, client.ForceOwnership); err != nil { werr := errors.Wrap(err, "cannot create observed object") c.Status.SetConditions(xpv1.ReconcileError(werr)) @@ -201,23 +198,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct } // Remove collection members that either do not exist anymore or are no match. - ol := &v1alpha2.ObjectList{} - if err := r.client.List(ctx, ol, client.MatchingLabels(map[string]string{membershipLabelKey: mlv})); err != nil { - werr := errors.Wrapf(err, "cannot list members matching label %v=%v", membershipLabelKey, mlv) - c.Status.SetConditions(xpv1.ReconcileError(werr)) - _ = r.client.Status().Update(ctx, c) - return ctrl.Result{}, werr - } - - oldRefs := sets.New[v1alpha1.ObservedObjectReference](c.Status.ObjectRefs...) for i := range ol.Items { - name := ol.Items[i].Name - if refs.Has(v1alpha1.ObservedObjectReference{Name: name}) { + o := ol.Items[i] + if refs.Has(v1alpha1.ObservedObjectReference{Name: o.Name}) { continue } - log.Debug("Removing", "name", name) + log.Debug("Removing", "name", o.Name) if err := r.client.Delete(ctx, &ol.Items[i]); err != nil { - werr := errors.Wrapf(err, "cannot delete observed object %s", name) + werr := errors.Wrapf(err, "cannot delete observed object %v", o) c.Status.SetConditions(xpv1.ReconcileError(werr)) _ = r.client.Status().Update(ctx, c) return ctrl.Result{}, werr @@ -225,11 +213,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct } c.Status.SetConditions(xpv1.ReconcileSuccess(), xpv1.Available()) - // Update references only if more or less objects got matched since the last reconciliation. - if !refs.Equal(oldRefs) { - c.Status.ObjectRefs = refs.UnsortedList() - } - c.Status.MembershipLabel = map[string]string{membershipLabelKey: mlv} + c.Status.MembershipLabel = ml return ctrl.Result{RequeueAfter: r.pollInterval()}, r.client.Status().Update(ctx, c) } @@ -252,19 +236,6 @@ func observedObjectName(collection client.Object, matchedObject client.Object) ( return id.String(), err } -func membershipLabelValue(collection *v1alpha1.ObservedObjectCollection) (string, error) { - if len(collection.GetName()) <= 63 { - return collection.GetName(), nil - } - // Otherwise, compute md5 hash of it, to reduce it length. - h := md5.New() //#nosec G401 -- used only for unique name generation - if _, err := h.Write([]byte(collection.GetName())); err != nil { - return "", err - } - id, err := uuid.FromBytes(h.Sum(nil)) - return id.String(), err -} - func observedObjectPatch(name string, matchedObject unstructured.Unstructured, collection *v1alpha1.ObservedObjectCollection) (*unstructured.Unstructured, error) { objectManifestTemplate := `{ "kind": "%s", @@ -299,14 +270,18 @@ func observedObjectPatch(name string, matchedObject unstructured.Unstructured, c }, }, } + labels := map[string]string{ + membershipLabelKey: collection.Name, + } if t := collection.Spec.Template; t != nil { - if len(t.Metadata.Labels) > 0 { - observedObject.SetLabels(t.Metadata.Labels) + for k, v := range t.Metadata.Labels { + labels[k] = v } if len(t.Metadata.Annotations) > 0 { observedObject.SetAnnotations(t.Metadata.Annotations) } } + observedObject.SetLabels(labels) v, err := runtime.DefaultUnstructuredConverter.ToUnstructured(observedObject) if err != nil { return nil, errors.Wrap(err, "cannot convert to unstructured") diff --git a/internal/controller/observedobjectcollection/reconciler_test.go b/internal/controller/observedobjectcollection/reconciler_test.go index 957ea7fd..f7faaa30 100644 --- a/internal/controller/observedobjectcollection/reconciler_test.go +++ b/internal/controller/observedobjectcollection/reconciler_test.go @@ -163,18 +163,13 @@ func TestReconciler(t *testing.T) { MockStatusUpdate: func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { c := obj.(*v1alpha1.ObservedObjectCollection) if cnd := c.Status.GetCondition(xpv1.TypeSynced); cnd.Status != corev1.ConditionTrue { - return fmt.Errorf("Object sync condition not true: %v", cnd.Message) + panic(fmt.Sprintf("Object sync condition not true: %v", cnd.Message)) } if cnd := c.Status.GetCondition(xpv1.TypeReady); cnd.Status != corev1.ConditionTrue { - return fmt.Errorf("Object ready condition not true: %v", cnd.Message) - } - - if l := len(c.Status.ObjectRefs); l != 2 { - return fmt.Errorf("Expected 2 objects refs, but got %v", l) + panic(fmt.Sprintf("Object ready condition not true: %v", cnd.Message)) } - if v := c.Status.MembershipLabel[membershipLabelKey]; v != collectionName.Name { - return fmt.Errorf("Expected membership label %v but got %v", collectionName.Name, v) + panic(fmt.Sprintf("Expected membership label %v but got %v", collectionName.Name, v)) } return nil }, @@ -270,11 +265,6 @@ func TestReconciler(t *testing.T) { if cnd := c.Status.GetCondition(xpv1.TypeReady); cnd.Status != corev1.ConditionTrue { return fmt.Errorf("Object ready condition not true: %v", cnd.Message) } - - if l := len(c.Status.ObjectRefs); l != 2 { - return fmt.Errorf("Expected 2 objects refs, but got %v", l) - } - if v := c.Status.MembershipLabel[membershipLabelKey]; v != collectionName.Name { return fmt.Errorf("Expected membership label %v but got %v", collectionName.Name, v) } @@ -402,7 +392,7 @@ func TestReconciler(t *testing.T) { } got, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: collectionName}) if !errors.Is(err, tc.want.err) { - t.Errorf("\n%s\nr.Reconcile(...): want error %v, got error %v", tc.reason, tc.want.err, err) + t.Errorf("\n%s\nr.Reconcile(...): want error: %v, got error: %v", tc.reason, tc.want.err, err) } if diff := cmp.Diff(tc.want.r, got, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nr.Reconcile(...): -want, +got:\n%s", tc.reason, diff) diff --git a/package/crds/kubernetes.crossplane.io_observedobjectcollections.yaml b/package/crds/kubernetes.crossplane.io_observedobjectcollections.yaml index 70279ef1..59384dbe 100644 --- a/package/crds/kubernetes.crossplane.io_observedobjectcollections.yaml +++ b/package/crds/kubernetes.crossplane.io_observedobjectcollections.yaml @@ -244,28 +244,13 @@ spec: MembershipLabel is the label set on each member of this collection and can be used for fetching them. type: object - objectRefs: - description: List of object references mathing the given selector - items: - description: ObservedObjectReference represents a reference to Object - with ObserveOnly management policy - properties: - name: - description: Name of the observed object - maxLength: 253 - minLength: 1 - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object required: - spec type: object + x-kubernetes-validations: + - message: metadata.name max length is 63 + rule: size(self.metadata.name) < 64 served: true storage: true subresources: