Merge pull request #3691 from nojnhuh/aso-tags

reconcile tags with ASO

azure/services/aso/aso.go

@@ -150,6 +150,12 @@ func (s *Service) CreateOrUpdateResource(ctx context.Context, spec azure.ASOReso
 	parameters.SetName(resourceName)
 	parameters.SetNamespace(resourceNamespace)
 
+	if t, ok := spec.(TagsGetterSetter); ok {
+		if err := reconcileTags(t, existing, parameters); err != nil {
+			return nil, errors.Wrap(err, "failed to reconcile tags")
+		}
+	}
+
 	labels := make(map[string]string)
 	annotations := make(map[string]string)
 

azure/services/aso/aso_test.go

@@ -33,6 +33,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/aso/mock_aso"
 	gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -675,6 +676,127 @@ func TestCreateOrUpdateResource(t *testing.T) {
 		g.Expect(err).NotTo(BeNil())
 		g.Expect(err.Error()).To(ContainSubstring("failed to update resource"))
 	})
+
+	t.Run("with tags success", func(t *testing.T) {
+		g := NewGomegaWithT(t)
+
+		sch := runtime.NewScheme()
+		g.Expect(asoresourcesv1.AddToScheme(sch)).To(Succeed())
+		c := fakeclient.NewClientBuilder().
+			WithScheme(sch).
+			Build()
+		s := New(c, clusterName)
+
+		mockCtrl := gomock.NewController(t)
+		specMock := struct {
+			*mock_azure.MockASOResourceSpecGetter
+			*mock_aso.MockTagsGetterSetter
+		}{
+			MockASOResourceSpecGetter: mock_azure.NewMockASOResourceSpecGetter(mockCtrl),
+			MockTagsGetterSetter:      mock_aso.NewMockTagsGetterSetter(mockCtrl),
+		}
+		specMock.MockASOResourceSpecGetter.EXPECT().ResourceRef().Return(&asoresourcesv1.ResourceGroup{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "name",
+				Namespace: "namespace",
+			},
+		})
+		specMock.MockASOResourceSpecGetter.EXPECT().Parameters(gomockinternal.AContext(), gomock.Any()).DoAndReturn(func(_ context.Context, object genruntime.MetaObject) (genruntime.MetaObject, error) {
+			return nil, nil
+		})
+		specMock.MockASOResourceSpecGetter.EXPECT().WasManaged(gomock.Any()).Return(false)
+
+		specMock.MockTagsGetterSetter.EXPECT().GetActualTags(gomock.Any()).Return(nil)
+		specMock.MockTagsGetterSetter.EXPECT().GetAdditionalTags().Return(nil)
+		specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(nil)
+		specMock.MockTagsGetterSetter.EXPECT().SetTags(gomock.Any(), gomock.Any())
+
+		ctx := context.Background()
+		g.Expect(c.Create(ctx, &asoresourcesv1.ResourceGroup{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "name",
+				Namespace: "namespace",
+				Labels: map[string]string{
+					infrav1.OwnedByClusterLabelKey: clusterName,
+				},
+				Annotations: map[string]string{
+					ReconcilePolicyAnnotation: ReconcilePolicyManage,
+				},
+			},
+			Status: asoresourcesv1.ResourceGroup_STATUS{
+				Conditions: []conditions.Condition{
+					{
+						Type:   conditions.ConditionTypeReady,
+						Status: metav1.ConditionTrue,
+					},
+				},
+			},
+		})).To(Succeed())
+
+		result, err := s.CreateOrUpdateResource(ctx, specMock, "service")
+		g.Expect(result).To(BeNil())
+		g.Expect(azure.IsOperationNotDoneError(err)).To(BeTrue())
+
+		updated := &asoresourcesv1.ResourceGroup{}
+		g.Expect(c.Get(ctx, types.NamespacedName{Name: "name", Namespace: "namespace"}, updated)).To(Succeed())
+		g.Expect(updated.Annotations).To(HaveKey(tagsLastAppliedAnnotation))
+	})
+
+	t.Run("with tags failure", func(t *testing.T) {
+		g := NewGomegaWithT(t)
+
+		sch := runtime.NewScheme()
+		g.Expect(asoresourcesv1.AddToScheme(sch)).To(Succeed())
+		c := fakeclient.NewClientBuilder().
+			WithScheme(sch).
+			Build()
+		s := New(c, clusterName)
+
+		mockCtrl := gomock.NewController(t)
+		specMock := struct {
+			*mock_azure.MockASOResourceSpecGetter
+			*mock_aso.MockTagsGetterSetter
+		}{
+			MockASOResourceSpecGetter: mock_azure.NewMockASOResourceSpecGetter(mockCtrl),
+			MockTagsGetterSetter:      mock_aso.NewMockTagsGetterSetter(mockCtrl),
+		}
+		specMock.MockASOResourceSpecGetter.EXPECT().ResourceRef().Return(&asoresourcesv1.ResourceGroup{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "name",
+				Namespace: "namespace",
+			},
+		})
+		specMock.MockASOResourceSpecGetter.EXPECT().Parameters(gomockinternal.AContext(), gomock.Any()).DoAndReturn(func(_ context.Context, object genruntime.MetaObject) (genruntime.MetaObject, error) {
+			return nil, nil
+		})
+
+		ctx := context.Background()
+		g.Expect(c.Create(ctx, &asoresourcesv1.ResourceGroup{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "name",
+				Namespace: "namespace",
+				Labels: map[string]string{
+					infrav1.OwnedByClusterLabelKey: clusterName,
+				},
+				Annotations: map[string]string{
+					ReconcilePolicyAnnotation: ReconcilePolicyManage,
+					tagsLastAppliedAnnotation: "{",
+				},
+			},
+			Status: asoresourcesv1.ResourceGroup_STATUS{
+				Conditions: []conditions.Condition{
+					{
+						Type:   conditions.ConditionTypeReady,
+						Status: metav1.ConditionTrue,
+					},
+				},
+			},
+		})).To(Succeed())
+
+		result, err := s.CreateOrUpdateResource(ctx, specMock, "service")
+		g.Expect(result).To(BeNil())
+		g.Expect(err.Error()).To(ContainSubstring("failed to reconcile tags"))
+	})
 }
 
 // TestDeleteResource tests the DeleteResource function.

azure/services/aso/interfaces.go

@@ -20,6 +20,7 @@ import (
 	"context"
 
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
+	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 )
 
@@ -28,3 +29,11 @@ type Reconciler interface {
 	CreateOrUpdateResource(ctx context.Context, spec azure.ASOResourceSpecGetter, serviceName string) (result genruntime.MetaObject, err error)
 	DeleteResource(ctx context.Context, spec azure.ASOResourceSpecGetter, serviceName string) (err error)
 }
+
+// TagsGetterSetter represents an object that supports tags.
+type TagsGetterSetter interface {
+	GetAdditionalTags() infrav1.Tags
+	GetDesiredTags(resource genruntime.MetaObject) infrav1.Tags
+	GetActualTags(resource genruntime.MetaObject) infrav1.Tags
+	SetTags(resource genruntime.MetaObject, tags infrav1.Tags)
+}

azure/services/aso/tags.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package aso
+
+import (
+	"encoding/json"
+
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
+	"github.com/pkg/errors"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/tags"
+	"sigs.k8s.io/cluster-api-provider-azure/util/maps"
+)
+
+// tagsLastAppliedAnnotation is the key for the annotation which tracks the AdditionalTags.
+// See https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/
+// for annotation formatting rules.
+const tagsLastAppliedAnnotation = "sigs.k8s.io/cluster-api-provider-azure-last-applied-tags"
+
+// reconcileTags modifies parameters in place to update its tags and its last-applied annotation.
+func reconcileTags(t TagsGetterSetter, existing genruntime.MetaObject, parameters genruntime.MetaObject) error {
+	lastAppliedTags := map[string]interface{}{}
+	if existing != nil {
+		lastAppliedTagsJSON := existing.GetAnnotations()[tagsLastAppliedAnnotation]
+		if lastAppliedTagsJSON != "" {
+			err := json.Unmarshal([]byte(lastAppliedTagsJSON), &lastAppliedTags)
+			if err != nil {
+				return errors.Wrapf(err, "failed to unmarshal JSON from %s annotation", tagsLastAppliedAnnotation)
+			}
+		}
+	}
+
+	existingTags := t.GetActualTags(existing)
+	existingTagsMap := converters.TagsToMap(existingTags)
+
+	_, createdOrUpdated, deleted, newAnnotation := tags.TagsChanged(lastAppliedTags, t.GetAdditionalTags(), existingTagsMap)
+	newTags := maps.Merge(maps.Merge(existingTags, t.GetDesiredTags(parameters)), createdOrUpdated)
+	for k := range deleted {
+		delete(newTags, k)
+	}
+	if len(newTags) == 0 {
+		newTags = nil
+	}
+	t.SetTags(parameters, newTags)
+
+	// We also need to update the annotation even if nothing changed to
+	// ensure it's set immediately following resource creation.
+	newAnnotationJSON, err := json.Marshal(newAnnotation)
+	if err != nil {
+		return errors.Wrapf(err, "failed to marshal JSON to %s annotation", tagsLastAppliedAnnotation)
+	}
+	parameters.SetAnnotations(maps.Merge(parameters.GetAnnotations(), map[string]string{
+		tagsLastAppliedAnnotation: string(newAnnotationJSON),
+	}))
+
+	return nil
+}