From dc0c92693798734642fd8f9b3b301b2a1bae7b19 Mon Sep 17 00:00:00 2001 From: Vladimir Andjelkoski Date: Mon, 30 Sep 2024 19:14:48 +0200 Subject: [PATCH 1/3] feat(AzureVpcPeering): delete remote peering --- .../v1beta1/vpcpeering_builder.go | 3 +- api/cloud-control/v1beta1/vpcpeering_types.go | 2 + ...d-control.kyma-project.io_vpcpeerings.yaml | 2 + ...d-control.kyma-project.io_vpcpeerings.yaml | 2 + internal/api-tests/vpcpeering_test.go | 8 ++-- .../cloud-control/vpcpeering_aws_test.go | 2 +- .../cloud-control/vpcpeering_azure_test.go | 9 +++- pkg/kcp/provider/azure/vpcpeering/new.go | 5 ++- .../azure/vpcpeering/peeringRemoteDelete.go | 42 +++++++++++++++++++ 9 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go diff --git a/api/cloud-control/v1beta1/vpcpeering_builder.go b/api/cloud-control/v1beta1/vpcpeering_builder.go index 5c648bd2d..b64ac5e2c 100644 --- a/api/cloud-control/v1beta1/vpcpeering_builder.go +++ b/api/cloud-control/v1beta1/vpcpeering_builder.go @@ -87,7 +87,7 @@ func (b *VpcPeeringBuilder) WithAwsPeering(remoteVpcId, remoteRegion, remoteAcco return b } -func (b *VpcPeeringBuilder) WithDetails(localName, localNamespace, remoteName, remoteNamespace, peeringName string, importCustomRoutes bool) *VpcPeeringBuilder { +func (b *VpcPeeringBuilder) WithDetails(localName, localNamespace, remoteName, remoteNamespace, peeringName string, importCustomRoutes, deleteRemotePeering bool) *VpcPeeringBuilder { if localName == "" { if b.Obj.Spec.Details == nil { return b @@ -104,6 +104,7 @@ func (b *VpcPeeringBuilder) WithDetails(localName, localNamespace, remoteName, r b.Obj.Spec.Details.RemoteNetwork.Namespace = remoteNamespace b.Obj.Spec.Details.PeeringName = peeringName b.Obj.Spec.Details.ImportCustomRoutes = importCustomRoutes + b.Obj.Spec.Details.DeleteRemotePeering = deleteRemotePeering return b } diff --git a/api/cloud-control/v1beta1/vpcpeering_types.go b/api/cloud-control/v1beta1/vpcpeering_types.go index 9cd0fd307..bbfb44ca6 100644 --- a/api/cloud-control/v1beta1/vpcpeering_types.go +++ b/api/cloud-control/v1beta1/vpcpeering_types.go @@ -67,6 +67,8 @@ type VpcPeeringDetails struct { LocalPeeringName string `json:"localPeeringName,omitempty"` ImportCustomRoutes bool `json:"importCustomRoutes,omitempty"` + + DeleteRemotePeering bool `json:"deleteRemotePeering,omitempty"` } // +kubebuilder:validation:MinProperties=1 diff --git a/config/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml b/config/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml index 3bcc39e95..06d33d336 100644 --- a/config/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml +++ b/config/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml @@ -48,6 +48,8 @@ spec: properties: details: properties: + deleteRemotePeering: + type: boolean importCustomRoutes: type: boolean localNetwork: diff --git a/config/dist/kcp/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml b/config/dist/kcp/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml index 3bcc39e95..06d33d336 100644 --- a/config/dist/kcp/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml +++ b/config/dist/kcp/crd/bases/cloud-control.kyma-project.io_vpcpeerings.yaml @@ -48,6 +48,8 @@ spec: properties: details: properties: + deleteRemotePeering: + type: boolean importCustomRoutes: type: boolean localNetwork: diff --git a/internal/api-tests/vpcpeering_test.go b/internal/api-tests/vpcpeering_test.go index bd6cfd9e8..66fe9cd2c 100644 --- a/internal/api-tests/vpcpeering_test.go +++ b/internal/api-tests/vpcpeering_test.go @@ -25,14 +25,14 @@ var _ = Describe("Feature: KCP VpcPeering", func() { canCreate( "VpcPeering with network details", nb().WithScope("s").WithRemoteRef("ns", "n"). - WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true), + WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true, false), ) canNotCreate( "VpcPeering with both network details and GPC info can not be created", nb().WithScope("s").WithRemoteRef("ns", "n"). WithGcpPeering("peering", "project", "vpc", true). - WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true), + WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true, false), "Only one of details or vpcPeering can be specified", ) @@ -48,10 +48,10 @@ var _ = Describe("Feature: KCP VpcPeering", func() { canNotChange( "VpcPeering network reference can not change", nb().WithScope("s").WithRemoteRef("ns", "n"). - WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true), + WithDetails("loc", "loc-ns", "rem", "rem-ns", "name", true, false), func(b Builder[*cloudcontrolv1beta1.VpcPeering]) { bb(b). - WithDetails("loc2", "loc-ns2", "rem2", "rem-ns2", "name2", false) + WithDetails("loc2", "loc-ns2", "rem2", "rem-ns2", "name2", false, false) }, "Peering details are immutable", ) diff --git a/internal/controller/cloud-control/vpcpeering_aws_test.go b/internal/controller/cloud-control/vpcpeering_aws_test.go index 39a6764bf..041ccb7fa 100644 --- a/internal/controller/cloud-control/vpcpeering_aws_test.go +++ b/internal/controller/cloud-control/vpcpeering_aws_test.go @@ -136,7 +136,7 @@ var _ = Describe("Feature: KCP VpcPeering", func() { kcpPeering = (&cloudcontrolv1beta1.VpcPeeringBuilder{}). WithScope(kymaName). WithRemoteRef("skr-namespace", "skr-aws-ip-range"). - WithDetails(localKcpNetworkName, infra.KCP().Namespace(), remoteKcpNetworkName, infra.KCP().Namespace(), "", false). + WithDetails(localKcpNetworkName, infra.KCP().Namespace(), remoteKcpNetworkName, infra.KCP().Namespace(), "", false, false). Build() Eventually(CreateObj). diff --git a/internal/controller/cloud-control/vpcpeering_azure_test.go b/internal/controller/cloud-control/vpcpeering_azure_test.go index 32e74081e..1ecc7ec71 100644 --- a/internal/controller/cloud-control/vpcpeering_azure_test.go +++ b/internal/controller/cloud-control/vpcpeering_azure_test.go @@ -66,7 +66,7 @@ var _ = Describe("Feature: KCP VpcPeering", func() { kcpPeering = (&cloudcontrolv1beta1.VpcPeeringBuilder{}). WithScope(kymaName). WithRemoteRef("skr-namespace", "skr-azure-vpcpeering"). - WithDetails(localKcpNetworkName, infra.KCP().Namespace(), remoteKcpNetworkName, infra.KCP().Namespace(), remotePeeringName, true). + WithDetails(localKcpNetworkName, infra.KCP().Namespace(), remoteKcpNetworkName, infra.KCP().Namespace(), remotePeeringName, true, true). Build() Eventually(CreateObj). @@ -270,6 +270,13 @@ var _ = Describe("Feature: KCP VpcPeering", func() { Expect(peering).To(BeNil()) }) + By("And Then remote Azure peering exists", func() { + peering, err := azureMockRemote.GetPeering(infra.Ctx(), remoteResourceGroup, remoteVnetName, remotePeeringName) + Expect(err).To(HaveOccurred()) + Expect(azuremeta.IsNotFound(err)).To(BeTrue()) + Expect(peering).To(BeNil()) + }) + By("// cleanup: Local KCP Network", func() { Eventually(Delete). WithArguments(infra.Ctx(), infra.KCP().Client(), localKcpNet). diff --git a/pkg/kcp/provider/azure/vpcpeering/new.go b/pkg/kcp/provider/azure/vpcpeering/new.go index 08b449db7..53e2e9c00 100644 --- a/pkg/kcp/provider/azure/vpcpeering/new.go +++ b/pkg/kcp/provider/azure/vpcpeering/new.go @@ -25,19 +25,20 @@ func New(stateFactory StateFactory) composed.Action { kcpNetworkRemoteLoad, statusInitiated, peeringLocalLoad, + remoteClientCreate, + peeringRemoteLoad, composed.IfElse( composed.MarkedForDeletionPredicate, composed.ComposeActions( "azureVpcPeering-delete", deleteVpcPeering, + peeringRemoteDelete, actions.PatchRemoveFinalizer, ), composed.ComposeActions( "azureVpcPeering-non-delete", actions.PatchAddFinalizer, - remoteClientCreate, peeringRemoteRequireSpecifiedName, - peeringRemoteLoad, composed.If( predicateRequireVNetShootTag, vpcRemoteLoad, diff --git a/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go b/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go new file mode 100644 index 000000000..b51751c37 --- /dev/null +++ b/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go @@ -0,0 +1,42 @@ +package vpcpeering + +import ( + "context" + "github.com/kyma-project/cloud-manager/pkg/composed" + azuremeta "github.com/kyma-project/cloud-manager/pkg/kcp/provider/azure/meta" +) + +func peeringRemoteDelete(ctx context.Context, st composed.State) (error, context.Context) { + state := st.(*State) + logger := composed.LoggerFromCtx(ctx) + + lll := logger.WithValues("vpcPeeringName", state.ObjAsVpcPeering().Spec.Details.PeeringName) + + if !state.ObjAsVpcPeering().Spec.Details.DeleteRemotePeering { + return nil, nil + } + + if len(state.ObjAsVpcPeering().Status.RemoteId) == 0 { + lll.Info("Remote VpcPeering deleted before Azure peering is created") + return nil, nil + } + + // params must be the same as in peeringRemoteCreate() + err := state.remoteClient.DeletePeering( + ctx, + state.remoteNetworkId.ResourceGroup, + state.remoteNetworkId.NetworkName(), + state.ObjAsVpcPeering().Spec.Details.PeeringName, + ) + + lll = lll.WithValues("vpcPeeringId", state.ObjAsVpcPeering().Status.RemoteId) + lll.Info("Deleting VpcPeering") + + if err != nil { + return azuremeta.LogErrorAndReturn(err, "Error deleting vpc peering", composed.LoggerIntoCtx(ctx, lll)) + } + + lll.Info("Remote VpcPeering deleted") + + return nil, nil +} From 4030a1f07d5e990d0d78ca83e6e1088724bfb180 Mon Sep 17 00:00:00 2001 From: Vladimir Andjelkoski Date: Tue, 1 Oct 2024 09:17:42 +0200 Subject: [PATCH 2/3] feat(AzureVpcPeering): delete remote peering --- api/cloud-resources/v1beta1/azurevpcpeering_types.go | 2 ++ .../cloud-resources.kyma-project.io_azurevpcpeerings.yaml | 2 ++ .../cloud-resources.kyma-project.io_azurevpcpeerings.yaml | 2 ++ pkg/skr/azurevpcpeering/createKpcVpcPeering.go | 3 ++- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/cloud-resources/v1beta1/azurevpcpeering_types.go b/api/cloud-resources/v1beta1/azurevpcpeering_types.go index 31967b4d8..b94c7a78f 100644 --- a/api/cloud-resources/v1beta1/azurevpcpeering_types.go +++ b/api/cloud-resources/v1beta1/azurevpcpeering_types.go @@ -33,6 +33,8 @@ type AzureVpcPeeringSpec struct { // +kubebuilder:validation:Required // +kubebuilder:validation:XValidation:rule=(self == oldSelf), message="RemoteVnet is immutable." RemoteVnet string `json:"remoteVnet,omitempty"` + + DeleteRemotePeering bool `json:"deleteRemotePeering,omitempty"` } // AzureVpcPeeringStatus defines the observed state of AzureVpcPeering diff --git a/config/crd/bases/cloud-resources.kyma-project.io_azurevpcpeerings.yaml b/config/crd/bases/cloud-resources.kyma-project.io_azurevpcpeerings.yaml index 2af7e4b94..0e9975a4d 100644 --- a/config/crd/bases/cloud-resources.kyma-project.io_azurevpcpeerings.yaml +++ b/config/crd/bases/cloud-resources.kyma-project.io_azurevpcpeerings.yaml @@ -44,6 +44,8 @@ spec: spec: description: AzureVpcPeeringSpec defines the desired state of AzureVpcPeering properties: + deleteRemotePeering: + type: boolean remotePeeringName: type: string x-kubernetes-validations: diff --git a/config/dist/skr/crd/bases/providers/azure/cloud-resources.kyma-project.io_azurevpcpeerings.yaml b/config/dist/skr/crd/bases/providers/azure/cloud-resources.kyma-project.io_azurevpcpeerings.yaml index 2af7e4b94..0e9975a4d 100644 --- a/config/dist/skr/crd/bases/providers/azure/cloud-resources.kyma-project.io_azurevpcpeerings.yaml +++ b/config/dist/skr/crd/bases/providers/azure/cloud-resources.kyma-project.io_azurevpcpeerings.yaml @@ -44,6 +44,8 @@ spec: spec: description: AzureVpcPeeringSpec defines the desired state of AzureVpcPeering properties: + deleteRemotePeering: + type: boolean remotePeeringName: type: string x-kubernetes-validations: diff --git a/pkg/skr/azurevpcpeering/createKpcVpcPeering.go b/pkg/skr/azurevpcpeering/createKpcVpcPeering.go index cddaf1294..ef1e1e402 100644 --- a/pkg/skr/azurevpcpeering/createKpcVpcPeering.go +++ b/pkg/skr/azurevpcpeering/createKpcVpcPeering.go @@ -42,7 +42,8 @@ func createKcpVpcPeering(ctx context.Context, st composed.State) (error, context Name: state.KymaRef.Name, }, Details: &cloudcontrolv1beta1.VpcPeeringDetails{ - PeeringName: obj.Spec.RemotePeeringName, + DeleteRemotePeering: obj.Spec.DeleteRemotePeering, + PeeringName: obj.Spec.RemotePeeringName, RemoteNetwork: klog.ObjectRef{ Name: state.RemoteNetwork.Name, Namespace: state.RemoteNetwork.Namespace, From be9ff1791b9efb665b6fc4d587098735f50bc507 Mon Sep 17 00:00:00 2001 From: Vladimir Andjelkoski Date: Tue, 1 Oct 2024 12:02:38 +0200 Subject: [PATCH 3/3] feat(AzureVpcPeering): delete remote peering --- .../controller/cloud-control/vpcpeering_azure_test.go | 2 +- pkg/kcp/provider/azure/vpcpeering/deleteVpcPeering.go | 11 +++++------ .../provider/azure/vpcpeering/peeringRemoteDelete.go | 11 ++++------- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/internal/controller/cloud-control/vpcpeering_azure_test.go b/internal/controller/cloud-control/vpcpeering_azure_test.go index 1ecc7ec71..19ddb6a9c 100644 --- a/internal/controller/cloud-control/vpcpeering_azure_test.go +++ b/internal/controller/cloud-control/vpcpeering_azure_test.go @@ -27,7 +27,7 @@ var _ = Describe("Feature: KCP VpcPeering", func() { remoteSubscription = "afdbc79f-de19-4df4-94cd-6be2739dc0e0" remoteResourceGroup = "MyResourceGroup" remoteVnetName = "MyVnet" - remotePeeringName = "MyPeering" + remotePeeringName = "my-peering" ) scope := &cloudcontrolv1beta1.Scope{} diff --git a/pkg/kcp/provider/azure/vpcpeering/deleteVpcPeering.go b/pkg/kcp/provider/azure/vpcpeering/deleteVpcPeering.go index d0d0fa567..67133a873 100644 --- a/pkg/kcp/provider/azure/vpcpeering/deleteVpcPeering.go +++ b/pkg/kcp/provider/azure/vpcpeering/deleteVpcPeering.go @@ -11,17 +11,14 @@ func deleteVpcPeering(ctx context.Context, st composed.State) (error, context.Co logger := composed.LoggerFromCtx(ctx) obj := state.ObjAsVpcPeering() - lll := logger.WithValues("vpcPeeringName", obj.Name) - if len(obj.Status.Id) == 0 { - lll.Info("VpcPeering deleted before Azure peering is created") + logger.Info("VpcPeering deleted before Azure peering is created") return nil, nil } resourceGroupName := state.Scope().Spec.Scope.Azure.VpcNetwork - lll = lll.WithValues("vpcPeeringId", obj.Status.Id) - lll.Info("Deleting VpcPeering") + logger.Info("Deleting VpcPeering") err := state.localClient.DeletePeering( ctx, @@ -31,8 +28,10 @@ func deleteVpcPeering(ctx context.Context, st composed.State) (error, context.Co ) if err != nil { - return azuremeta.LogErrorAndReturn(err, "Error deleting vpc peering", composed.LoggerIntoCtx(ctx, lll)) + return azuremeta.LogErrorAndReturn(err, "Error deleting vpc peering", ctx) } + logger.Info("VpcPeering deleted") + return nil, nil } diff --git a/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go b/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go index b51751c37..3ac86c32b 100644 --- a/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go +++ b/pkg/kcp/provider/azure/vpcpeering/peeringRemoteDelete.go @@ -10,14 +10,12 @@ func peeringRemoteDelete(ctx context.Context, st composed.State) (error, context state := st.(*State) logger := composed.LoggerFromCtx(ctx) - lll := logger.WithValues("vpcPeeringName", state.ObjAsVpcPeering().Spec.Details.PeeringName) - if !state.ObjAsVpcPeering().Spec.Details.DeleteRemotePeering { return nil, nil } if len(state.ObjAsVpcPeering().Status.RemoteId) == 0 { - lll.Info("Remote VpcPeering deleted before Azure peering is created") + logger.Info("Remote VpcPeering deleted before Azure peering is created") return nil, nil } @@ -29,14 +27,13 @@ func peeringRemoteDelete(ctx context.Context, st composed.State) (error, context state.ObjAsVpcPeering().Spec.Details.PeeringName, ) - lll = lll.WithValues("vpcPeeringId", state.ObjAsVpcPeering().Status.RemoteId) - lll.Info("Deleting VpcPeering") + logger.Info("Deleting remote VpcPeering") if err != nil { - return azuremeta.LogErrorAndReturn(err, "Error deleting vpc peering", composed.LoggerIntoCtx(ctx, lll)) + return azuremeta.LogErrorAndReturn(err, "Error deleting vpc peering", ctx) } - lll.Info("Remote VpcPeering deleted") + logger.Info("Remote VpcPeering deleted") return nil, nil }