catalog-model: remove special merge treatment of annotations

.changeset/yellow-ties-switch.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-model': minor
+---
+
+The catalog no longer attempts to merge old and new annotations, when updating an entity from a remote location. This was a behavior that was copied from kubernetes, and catered to use cases where you wanted to use HTTP POST to update an entity in-place, outside of what the refresh loop does. This has proved to be a mistake, because as a side effect, the refresh loop effectively is unable to ever delete annotations when they are removed from source YAML. This is obviously a breaking change, but we believe that this is not a behavior that is relied upon in the wild, and it has never been an actually supported use flow of the catalog. We therefore choose to break the behavior outright, and instead just store updated annotations verbatim - just like we already do for example for labels

packages/catalog-model/src/entity/util.test.ts

@@ -96,18 +96,12 @@ describe('util', () => {
       b = lodash.cloneDeep(a);
       b.metadata.labels.labelKey += 'a';
       expect(entityHasChanges(a, b)).toBe(true);
-    });
-
-    it('detects annotation changes, but not removals', () => {
-      let b: any = lodash.cloneDeep(a);
+      b = lodash.cloneDeep(a);
       b.metadata.annotations.annotationKey += 'a';
       expect(entityHasChanges(a, b)).toBe(true);
       b = lodash.cloneDeep(a);
-      b.metadata.annotations.n = 'n';
-      expect(entityHasChanges(a, b)).toBe(true);
-      b = lodash.cloneDeep(a);
       delete b.metadata.annotations.annotationKey;
-      expect(entityHasChanges(a, b)).toBe(false);
+      expect(entityHasChanges(a, b)).toBe(true);
     });
 
     it('detects spec changes', () => {

packages/catalog-model/src/entity/util.ts

@@ -54,10 +54,6 @@ export function generateEntityEtag(): string {
  * @param next The new state of the entity
  */
 export function entityHasChanges(previous: Entity, next: Entity): boolean {
-  if (entityHasAnnotationChanges(previous, next)) {
-    return true;
-  }
-
   const e1 = lodash.cloneDeep(previous);
   const e2 = lodash.cloneDeep(next);
 
@@ -67,6 +63,18 @@ export function entityHasChanges(previous: Entity, next: Entity): boolean {
   if (!e2.metadata.labels) {
     e2.metadata.labels = {};
   }
+  if (!e1.metadata.annotations) {
+    e1.metadata.annotations = {};
+  }
+  if (!e2.metadata.annotations) {
+    e2.metadata.annotations = {};
+  }
+  if (!e1.metadata.tags) {
+    e1.metadata.tags = [];
+  }
+  if (!e2.metadata.tags) {
+    e2.metadata.tags = [];
+  }
 
   // Remove generated fields
   delete e1.metadata.uid;
@@ -76,10 +84,6 @@ export function entityHasChanges(previous: Entity, next: Entity): boolean {
   delete e2.metadata.etag;
   delete e2.metadata.generation;
 
-  // Remove already compared things
-  delete e1.metadata.annotations;
-  delete e2.metadata.annotations;
-
   // Remove things that we explicitly do not compare
   delete e1.relations;
   delete e2.relations;
@@ -106,14 +110,6 @@ export function generateUpdatedEntity(previous: Entity, next: Entity): Entity {
 
   const result = lodash.cloneDeep(next);
 
-  // Annotations are merged, with the new ones taking precedence
-  if (previous.metadata.annotations) {
-    next.metadata.annotations = {
-      ...previous.metadata.annotations,
-      ...next.metadata.annotations,
-    };
-  }
-
   // Generated fields are copied and updated
   const bumpEtag = entityHasChanges(previous, result);
   const bumpGeneration = !lodash.isEqual(previous.spec, result.spec);
@@ -123,26 +119,3 @@ export function generateUpdatedEntity(previous: Entity, next: Entity): Entity {
 
   return result;
 }
-
-function entityHasAnnotationChanges(previous: Entity, next: Entity): boolean {
-  // Since the next annotations get merged into the previous, extract only
-  // the overlapping keys and check if their values match.
-  if (next.metadata.annotations) {
-    if (!previous.metadata.annotations) {
-      return true;
-    }
-    if (
-      !lodash.isEqual(
-        next.metadata.annotations,
-        lodash.pick(
-          previous.metadata.annotations,
-          Object.keys(next.metadata.annotations),
-        ),
-      )
-    ) {
-      return true;
-    }
-  }
-
-  return false;
-}