From 8deb3c7eba4465185e35c6eca9a824c9c6f96d05 Mon Sep 17 00:00:00 2001 From: Ashutosh Kumar Date: Mon, 25 Oct 2021 17:37:47 +0530 Subject: [PATCH] chore(private_dns): tag dns zone and delete only if owned This commit tags private DNS zone created by CAPZ and delete only if it has been created by CAPZ Signed-off-by: Ashutosh Kumar --- azure/services/privatedns/client.go | 13 +- .../privatedns/mock_privatedns/client_mock.go | 15 ++ azure/services/privatedns/privatedns.go | 35 +++- azure/services/privatedns/privatedns_test.go | 158 +++++++++++++++++- 4 files changed, 209 insertions(+), 12 deletions(-) diff --git a/azure/services/privatedns/client.go b/azure/services/privatedns/client.go index e3c0fdc80a80..8df5ea8b7104 100644 --- a/azure/services/privatedns/client.go +++ b/azure/services/privatedns/client.go @@ -28,6 +28,7 @@ import ( // Client wraps go-sdk. type client interface { + GetZone(context.Context, string, string) (privatedns.PrivateZone, error) CreateOrUpdateZone(context.Context, string, string, privatedns.PrivateZone) error DeleteZone(context.Context, string, string) error CreateOrUpdateLink(context.Context, string, string, string, privatedns.VirtualNetworkLink) error @@ -74,11 +75,21 @@ func newRecordSetsClient(subscriptionID string, baseURI string, authorizer autor return recordsClient } +// GetZone returns a private zone. +func (ac *azureClient) GetZone(ctx context.Context, resourceGroupName, zoneName string) (privatedns.PrivateZone, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "privatedns.AzureClient.GetZone") + defer done() + gotZone, err := ac.privatezones.Get(ctx, resourceGroupName, zoneName) + if err != nil { + return privatedns.PrivateZone{}, err + } + return gotZone, nil +} + // CreateOrUpdateZone creates or updates a private zone. func (ac *azureClient) CreateOrUpdateZone(ctx context.Context, resourceGroupName string, zoneName string, zone privatedns.PrivateZone) error { ctx, _, done := tele.StartSpanWithLogger(ctx, "privatedns.AzureClient.CreateOrUpdateZone") defer done() - future, err := ac.privatezones.CreateOrUpdate(ctx, resourceGroupName, zoneName, zone, "", "") if err != nil { return err diff --git a/azure/services/privatedns/mock_privatedns/client_mock.go b/azure/services/privatedns/mock_privatedns/client_mock.go index 7bf87e084b61..fbf8f62b2c73 100644 --- a/azure/services/privatedns/mock_privatedns/client_mock.go +++ b/azure/services/privatedns/mock_privatedns/client_mock.go @@ -134,3 +134,18 @@ func (mr *MockclientMockRecorder) DeleteZone(arg0, arg1, arg2 interface{}) *gomo mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteZone", reflect.TypeOf((*Mockclient)(nil).DeleteZone), arg0, arg1, arg2) } + +// GetZone mocks base method. +func (m *Mockclient) GetZone(arg0 context.Context, arg1, arg2 string) (privatedns.PrivateZone, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetZone", arg0, arg1, arg2) + ret0, _ := ret[0].(privatedns.PrivateZone) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetZone indicates an expected call of GetZone. +func (mr *MockclientMockRecorder) GetZone(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetZone", reflect.TypeOf((*Mockclient)(nil).GetZone), arg0, arg1, arg2) +} diff --git a/azure/services/privatedns/privatedns.go b/azure/services/privatedns/privatedns.go index f4aced098711..c8dfc4e9ee56 100644 --- a/azure/services/privatedns/privatedns.go +++ b/azure/services/privatedns/privatedns.go @@ -19,6 +19,8 @@ package privatedns import ( "context" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns" "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" @@ -59,7 +61,15 @@ func (s *Service) Reconcile(ctx context.Context) error { if zoneSpec != nil { // Create the private DNS zone. s.Scope.V(2).Info("creating private DNS zone", "private dns zone", zoneSpec.ZoneName) - err := s.client.CreateOrUpdateZone(ctx, s.Scope.ResourceGroup(), zoneSpec.ZoneName, privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + pDNS := privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.ClusterName(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Additional: s.Scope.AdditionalTags(), + })), + } + err := s.client.CreateOrUpdateZone(ctx, s.Scope.ResourceGroup(), zoneSpec.ZoneName, pDNS) if err != nil { return errors.Wrapf(err, "failed to create private DNS zone %s", zoneSpec.ZoneName) } @@ -119,6 +129,16 @@ func (s *Service) Delete(ctx context.Context) error { zoneSpec := s.Scope.PrivateDNSSpec() if zoneSpec != nil { + isManaged, err := s.isPrivateDNSManaged(ctx, s.Scope.ResourceGroup(), zoneSpec.ZoneName) + if err != nil && !azure.ResourceNotFound(err) { + return errors.Wrapf(err, "could not get private DNS zone state of %s in resource group %s", zoneSpec.ZoneName, s.Scope.ResourceGroup()) + } + + if !isManaged { + s.Scope.V(2).Info("Skipping private DNS zone deletion for unmanaged private DNS zone", "private DNS", zoneSpec.ZoneName) + return nil + } + for _, linkSpec := range zoneSpec.Links { // Remove each virtual network link. s.Scope.V(2).Info("removing virtual network link", "virtual network", linkSpec.VNetName, "private dns zone", zoneSpec.ZoneName) @@ -130,7 +150,7 @@ func (s *Service) Delete(ctx context.Context) error { // Delete the private DNS zone, which also deletes all records. s.Scope.V(2).Info("deleting private dns zone", "private dns zone", zoneSpec.ZoneName) - err := s.client.DeleteZone(ctx, s.Scope.ResourceGroup(), zoneSpec.ZoneName) + err = s.client.DeleteZone(ctx, s.Scope.ResourceGroup(), zoneSpec.ZoneName) if err != nil && azure.ResourceNotFound(err) { // already deleted return nil @@ -142,3 +162,14 @@ func (s *Service) Delete(ctx context.Context) error { } return nil } + +// isPrivateDNSManaged returns true if the private DNS has an owned tag with the cluster name as value, +// meaning that the DNS lifecycle is managed. +func (s *Service) isPrivateDNSManaged(ctx context.Context, resourceGroup, zoneName string) (bool, error) { + gotZone, err := s.client.GetZone(ctx, resourceGroup, zoneName) + if err != nil { + return false, err + } + tags := converters.MapToTags(gotZone.Tags) + return tags.HasOwned(s.Scope.ClusterName()), nil +} diff --git a/azure/services/privatedns/privatedns_test.go b/azure/services/privatedns/privatedns_test.go index c318bb9497d1..81131f22f99e 100644 --- a/azure/services/privatedns/privatedns_test.go +++ b/azure/services/privatedns/privatedns_test.go @@ -70,8 +70,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.SubscriptionID().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -120,8 +127,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -174,8 +188,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") - s.SubscriptionID().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) + s.SubscriptionID().AnyTimes().Return("123") + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -224,8 +245,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -278,8 +306,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") - s.SubscriptionID().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) + s.SubscriptionID().AnyTimes().Return("123") + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -323,8 +358,15 @@ func TestReconcilePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{Location: to.StringPtr(azure.Global)}) + m.CreateOrUpdateZone(gomockinternal.AContext(), "my-rg", "my-dns-zone", privatedns.PrivateZone{ + Location: to.StringPtr(azure.Global), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + }, + }) m.CreateOrUpdateLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1", privatedns.VirtualNetworkLink{ VirtualNetworkLinkProperties: &privatedns.VirtualNetworkLinkProperties{ VirtualNetwork: &privatedns.SubResource{ @@ -391,7 +433,7 @@ func TestDeletePrivateDNS(t *testing.T) { }, }, { - name: "delete the dns zone", + name: "delete the dns zone managed by capz", expectedError: "", expect: func(s *mock_privatedns.MockScopeMockRecorder, m *mock_privatedns.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) @@ -412,10 +454,44 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link") m.DeleteZone(gomockinternal.AContext(), "my-rg", "my-dns-zone") }, }, + { + name: "skip unmanaged private dns zone deletion", + expectedError: "", + expect: func(s *mock_privatedns.MockScopeMockRecorder, m *mock_privatedns.MockclientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.PrivateDNSSpec().Return(&azure.PrivateDNSSpec{ + ZoneName: "my-dns-zone", + Links: []azure.PrivateDNSLinkSpec{ + { + VNetName: "my-vnet", + VNetResourceGroup: "vnet-rg", + LinkName: "my-link", + }, + }, + Records: []infrav1.AddressRecord{ + { + Hostname: "hostname-1", + IP: "10.0.0.8", + }, + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone") + }, + }, { name: "delete the dns zone with multiple links", expectedError: "", @@ -448,6 +524,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-2") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-3") @@ -476,6 +560,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.DeleteZone(gomockinternal.AContext(), "my-rg", "my-dns-zone") @@ -513,6 +605,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-2"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -542,6 +642,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.DeleteZone(gomockinternal.AContext(), "my-rg", "my-dns-zone"). @@ -570,6 +678,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) }, @@ -606,6 +722,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-2"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) @@ -633,6 +757,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link") m.DeleteZone(gomockinternal.AContext(), "my-rg", "my-dns-zone"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) @@ -670,6 +802,14 @@ func TestDeletePrivateDNS(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") + s.ClusterName().AnyTimes().Return("my-cluster") + m.GetZone(gomockinternal.AContext(), "my-rg", "my-dns-zone").Return(privatedns.PrivateZone{ + Name: to.StringPtr("my-dns-zone"), + Tags: map[string]*string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), + "foo": to.StringPtr("bar"), + }, + }, nil) m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-1") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-2") m.DeleteLink(gomockinternal.AContext(), "my-rg", "my-dns-zone", "my-link-3")