Merge pull request #8766 from caseydavenport/casey-do-annotations-too

Also handle our annotations when converting v1 <-> v3
projectcalico · Apr 26, 2024 · 1b44227 · 1b44227
2 parents 905156a + 9416788
commit 1b44227
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 1 deletion.
diff --git a/libcalico-go/lib/backend/k8s/resources/customresource_test.go b/libcalico-go/lib/backend/k8s/resources/customresource_test.go
@@ -141,4 +141,89 @@ var _ = Describe("Custom resource conversion methods (tested using BGPPeer)", fu
 		Expect(kvp.Value).To(BeAssignableToTypeOf(&apiv3.BGPPeer{}))
 		Expect(kvp.Value).To(Equal(kvp1.Value))
 	})
+
+	It("should handle converting labels and annotations from v3 -> v1", func() {
+		// Create a v3 object with labels and annotations set.
+		res1 := &apiv3.BGPPeer{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       apiv3.KindBGPPeer,
+				APIVersion: apiv3.GroupVersionCurrent,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:            name1,
+				ResourceVersion: "rv",
+				UID:             convertedUID,
+				Labels: map[string]string{
+					"foo":                    "bar",
+					"projectcalico.org/foo":  "bar",
+					"operator.tigera.io/foo": "bar",
+				},
+				Annotations: map[string]string{
+					"foo":                    "bar",
+					"projectcalico.org/foo":  "bar",
+					"operator.tigera.io/foo": "bar",
+				},
+			},
+			Spec: apiv3.BGPPeerSpec{},
+		}
+
+		// Convert resource.
+		resConverted, err := ConvertCalicoResourceToK8sResource(res1)
+		Expect(err).NotTo(HaveOccurred())
+
+		// Assert that labels we own are maintained, but others are removed.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(Equal(map[string]string{
+			"projectcalico.org/foo":  "bar",
+			"operator.tigera.io/foo": "bar",
+		}))
+
+		// Assert that annotations we own are maintained, but others are removed.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).NotTo(HaveKey("foo"))
+
+		// Assert that labels we own are maintained, but others are removed.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).NotTo(HaveKey("foo"))
+
+		// Add some labels and annotations to the v1 resource, then convert back to make sure they are handled correctly.
+		resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels["foo2"] = "bar2"
+		resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels["operator.tigera.io/foo2"] = "bar2"
+		resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations["foo2"] = "bar2"
+		resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations["projectcalico.org/foo2"] = "bar2"
+
+		// Convert back to v3.
+		ConvertK8sResourceToCalicoResource(resConverted)
+
+		// Expect the original annotations plus the new ones.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("projectcalico.org/foo2", "bar2"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("foo2", "bar2"))
+
+		// Expect the original labels, plus the new one.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("foo2", "bar2"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("operator.tigera.io/foo2", "bar2"))
+
+		// Converting this resource back into v1 should sanitize the v1 labels and annotations, removing any that aren't ours.
+		resConverted, err = ConvertCalicoResourceToK8sResource(resConverted)
+		Expect(err).NotTo(HaveOccurred())
+
+		// Assert that annotations we own are maintained, but others are removed.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).NotTo(HaveKey("foo"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Annotations).NotTo(HaveKey("foo2"))
+
+		// Assert that labels we own are maintained, but others are removed.
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("projectcalico.org/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).To(HaveKeyWithValue("operator.tigera.io/foo", "bar"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).NotTo(HaveKey("foo"))
+		Expect(resConverted.(*apiv3.BGPPeer).ObjectMeta.Labels).NotTo(HaveKey("foo2"))
+	})
 })
diff --git a/libcalico-go/lib/backend/k8s/resources/resources.go b/libcalico-go/lib/backend/k8s/resources/resources.go
@@ -182,7 +182,7 @@ func ConvertCalicoResourceToK8sResource(resIn Resource) (Resource, error) {
 	// We make an exception for projectcalico.org/ labels, which we own and may use on the v1 API.
 	var v1Labels map[string]string
 	for k, v := range rom.GetLabels() {
-		if strings.Contains(k, "projectcalico.org/") {
+		if isOurs(k) {
 			if v1Labels == nil {
 				v1Labels = map[string]string{}
 			}
@@ -191,6 +191,13 @@ func ConvertCalicoResourceToK8sResource(resIn Resource) (Resource, error) {
 	}
 	meta.Labels = v1Labels
 
+	// Also maintain any annotations that we own.
+	for k, v := range rom.GetAnnotations() {
+		if isOurs(k) {
+			annotations[k] = v
+		}
+	}
+
 	if rom.GetUID() != "" {
 		uid, err := conversion.ConvertUID(rom.GetUID())
 		if err != nil {
@@ -206,6 +213,10 @@ func ConvertCalicoResourceToK8sResource(resIn Resource) (Resource, error) {
 	return resOut, nil
 }
 
+func isOurs(k string) bool {
+	return strings.Contains(k, "projectcalico.org/") || strings.Contains(k, "operator.tigera.io/")
+}
+
 // Retrieve all of the Calico Metadata from the k8s resource annotations for CRD backed
 // resources. This should remove the relevant Calico Metadata annotation when it has finished.
 func ConvertK8sResourceToCalicoResource(res Resource) error {
@@ -273,6 +284,12 @@ func ConvertK8sResourceToCalicoResource(res Resource) error {
 		}
 		labels[k] = v
 	}
+	for k, v := range meta.GetAnnotations() {
+		if annotations == nil {
+			annotations = make(map[string]string)
+		}
+		annotations[k] = v
+	}
 
 	// Manually write in the data not stored in the annotations: Name, Namespace, ResourceVersion,
 	// so that they do not get overwritten.
@@ -281,6 +298,7 @@ func ConvertK8sResourceToCalicoResource(res Resource) error {
 	meta.ResourceVersion = rom.GetResourceVersion()
 	meta.UID = rom.GetUID()
 	meta.Labels = labels
+	meta.Annotations = annotations
 
 	// If no creation timestamp was stored in the metadata annotation, use the one from the CR.
 	// The timestamp is normally set in the clientv3 code. However, for objects that bypass