fix: support exceptions in diff suppression (#1828)

Co-authored-by: Jack Luo <[email protected]>
Co-authored-by: Matt Trachier <[email protected]>

Author: 3 people

rancher2/schema_common.go

@@ -11,35 +11,49 @@ const (
 	commonAnnotationLabelRancher = "rancher.io/"
 )
 
+// exceptions is a list of annotation/label keys that should not be suppressed.
+var exceptions = []string{
+	"rancher.io/imported-cluster-version-management",
+}
+
+// suppressFunc is a DiffSuppressFunc that prevents Terraform from removing Rancher-managed annotations or labels.
+// It also ignores changes to a predefined set of Rancher-managed annotation/label keys.
+//
+// Thought it is not recommended, users can still add annotations or labels whose keys contain Rancher-managed keys,
+// but they won't be able to remove them once added.
+// Note: Terraform prefixes the key `k` with either "annotations." or "labels."
+var suppressFunc = func(k, old, new string, d *schema.ResourceData) bool {
+	for _, exception := range exceptions {
+		// Explicitly check if the key ends with the exception
+		if strings.HasSuffix(k, exception) {
+			return false
+		}
+	}
+
+	if (strings.Contains(k, commonAnnotationLabelCattle) || strings.Contains(k, commonAnnotationLabelRancher)) && new == "" {
+		return true
+	}
+
+	return false
+}
+
 // Schemas
 
 func commonAnnotationLabelFields() map[string]*schema.Schema {
 	s := map[string]*schema.Schema{
 		"annotations": {
-			Type:        schema.TypeMap,
-			Optional:    true,
-			Computed:    true,
-			Description: "Annotations of the resource",
-			DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-				// Suppressing diff for annotations containing cattle.io/
-				if (strings.Contains(k, commonAnnotationLabelCattle) || strings.Contains(k, commonAnnotationLabelRancher)) && new == "" {
-					return true
-				}
-				return false
-			},
+			Type:             schema.TypeMap,
+			Optional:         true,
+			Computed:         true,
+			Description:      "Annotations of the resource",
+			DiffSuppressFunc: suppressFunc,
 		},
 		"labels": {
-			Type:        schema.TypeMap,
-			Optional:    true,
-			Computed:    true,
-			Description: "Labels of the resource",
-			DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-				// Suppressing diff for labels containing cattle.io/
-				if (strings.Contains(k, commonAnnotationLabelCattle) || strings.Contains(k, commonAnnotationLabelRancher)) && new == "" {
-					return true
-				}
-				return false
-			},
+			Type:             schema.TypeMap,
+			Optional:         true,
+			Computed:         true,
+			Description:      "Labels of the resource",
+			DiffSuppressFunc: suppressFunc,
 		},
 	}
 	return s

rancher2/schema_common_test.go

@@ -0,0 +1,94 @@
+package rancher2
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSupressFunc(t *testing.T) {
+	cases := []struct {
+		Name           string
+		K              string
+		Old            string
+		New            string
+		ExpectedResult bool
+	}{
+		{
+			Name:           "Exception key change should not be suppressed",
+			K:              "annotations.rancher.io/imported-cluster-version-management",
+			Old:            "true",
+			New:            "false",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "Exception key removal should not be suppressed",
+			K:              "labels.rancher.io/imported-cluster-version-management",
+			Old:            "false",
+			New:            "",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "Exception key but not as suffix should not be suppressed",
+			K:              "labels.rancher.io/imported-cluster-version-management-new",
+			Old:            "old_val",
+			New:            "new_val",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "Rancher.io annotation/label removal should be suppressed",
+			K:              "annotations.rancher.io/creator",
+			Old:            "user-123",
+			New:            "",
+			ExpectedResult: true,
+		},
+		{
+			Name:           "Cattle.io annotation/label removal should be suppressed",
+			K:              "annotations.cattle.io/some-state",
+			Old:            "some-val",
+			New:            "",
+			ExpectedResult: true,
+		},
+		{
+			Name:           "Cattle.io annotation/label addition should not be suppressed",
+			K:              "annotations.cattle.io/some-state",
+			Old:            "",
+			New:            "some_val",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "User-managed change to a rancher.io annotation should not be suppressed",
+			K:              "annotations.rancher.io/creator",
+			Old:            "user-123",
+			New:            "user-456",
+			ExpectedResult: false,
+		},
+		{Name: "User-managed change to a cattle.io annotation should not be suppressed",
+			K:              "annotations.cattle.io/creator",
+			Old:            "user-123",
+			New:            "user-456",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "Non-Rancher annotation should not be suppressed",
+			K:              "annotations.my-custom-annotation",
+			Old:            "old-val",
+			New:            "new-val",
+			ExpectedResult: false,
+		},
+		{
+			Name:           "Non-Rancher annotation removal should not be suppressed",
+			K:              "annotations.my-custom-annotation",
+			Old:            "old-val",
+			New:            "",
+			ExpectedResult: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			result := suppressFunc(tc.K, tc.Old, tc.New, nil)
+			assert.Equal(t, tc.ExpectedResult, result)
+		})
+	}
+}