Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(AzureVpcPeering): handles CIDR overlap error #871

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions api/cloud-control/v1beta1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ const (

ReasonScopeNotFound = "ScopeNoFound"

ReasonUnknown = "Unknown"
ReasonReady = "Ready"
ReasonGcpError = "GCPError"
ReasonValidationFailed = "ValidationFailed"
ReasonMissingDependency = "MissingDependency"
ReasonWaitingDependency = "WaitingDependency"
ReasonDeleteWhileUsed = "DeleteWhileUsed"
ReasonUnknown = "Unknown"
ReasonReady = "Ready"
ReasonGcpError = "GCPError"
ReasonValidationFailed = "ValidationFailed"
ReasonMissingDependency = "MissingDependency"
ReasonWaitingDependency = "WaitingDependency"
ReasonDeleteWhileUsed = "DeleteWhileUsed"
ReasonRemoteClientFailed = "RemoteClientFailed"
)
14 changes: 0 additions & 14 deletions internal/controller/cloud-control/vpcpeering_azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,25 +500,11 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
Should(Succeed(), "expected VpcPeering not to exist (be deleted), but it still exists")
})

By("And Then local Azure peering does not exist", func() {
peering, err := azureMockLocal.GetPeering(infra.Ctx(), localResourceGroupName, localVirtualNetworkName, kcpPeeringName)
Expect(err).To(HaveOccurred())
Expect(azuremeta.IsNotFound(err)).To(BeTrue())
Expect(peering).To(BeNil())
})

By("And Then remote Azure peering exists", func() {
peering, err := azureMockRemote.GetPeering(infra.Ctx(), remoteResourceGroup, remoteVnetName, remotePeeringName)
Expect(peering).NotTo(BeNil())
Expect(err).NotTo(HaveOccurred())
})

By("// cleanup: Scope", func() {
Eventually(Delete).
WithArguments(infra.Ctx(), infra.KCP().Client(), scope).
Should(Succeed())
})

})

It("Scenario: KCP Azure VpcPeering can be deleted when unauthorized", func() {
Expand Down
6 changes: 6 additions & 0 deletions pkg/kcp/provider/azure/meta/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/kyma-project/cloud-manager/pkg/composed"
"github.com/kyma-project/cloud-manager/pkg/util"
"net/http"
"regexp"
)

const (
Expand All @@ -21,6 +22,8 @@ const (
VnetAddressSpaceOverlapsWithAlreadyPeeredVnetMessage = "Cannot create or update peering. Virtual networks cannot be peered because address space of the first virtual network overlaps with address space of third virtual network already peered with the second virtual network."
InvalidResourceName = "InvalidResourceName"
InvalidResourceNameMessage = "Resource name is invalid. The name can be up to 80 characters long. It must begin with a word character, and it must end with a word character. The name may contain word characters or '.', '-'."
VnetAddressSpacesOverlap = "VnetAddressSpacesOverlap"
VnetAddressSpacesOverlapMessage = "Cannot create or update peering. Virtual networks cannot be peered because their address spaces overlap. "
)

func IsTooManyRequests(err error) bool {
Expand Down Expand Up @@ -100,6 +103,9 @@ func GetErrorMessage(err error) (string, bool) {
return VnetAddressSpaceOverlapsWithAlreadyPeeredVnetMessage, true
case InvalidResourceName:
return InvalidResourceNameMessage, true
case VnetAddressSpacesOverlap:
r := regexp.MustCompile(`Overlapping address prefixes:[^\"]*`)
dushanpantic marked this conversation as resolved.
Show resolved Hide resolved
return VnetAddressSpacesOverlapMessage + r.FindString(respErr.Error()), true
}
}

Expand Down
22 changes: 16 additions & 6 deletions pkg/kcp/provider/azure/vpcpeering/peeringLocalLoad.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import (
"context"
"github.com/kyma-project/cloud-manager/pkg/composed"
azuremeta "github.com/kyma-project/cloud-manager/pkg/kcp/provider/azure/meta"
"github.com/kyma-project/cloud-manager/pkg/kcp/provider/azure/util"
azureutil "github.com/kyma-project/cloud-manager/pkg/kcp/provider/azure/util"
"github.com/kyma-project/cloud-manager/pkg/util"
"k8s.io/utils/ptr"
)

Expand All @@ -16,7 +17,7 @@ func peeringLocalLoad(ctx context.Context, st composed.State) (error, context.Co
ok := false

if len(state.ObjAsVpcPeering().Status.Id) > 0 {
resourceID, err := util.ParseResourceID(state.ObjAsVpcPeering().Status.Id)
resourceID, err := azureutil.ParseResourceID(state.ObjAsVpcPeering().Status.Id)
if err == nil {
resourceGroup = resourceID.ResourceGroup
networkName = resourceID.ResourceName
Expand Down Expand Up @@ -44,11 +45,20 @@ func peeringLocalLoad(ctx context.Context, st composed.State) (error, context.Co
state.ObjAsVpcPeering().GetLocalPeeringName(),
)

if azuremeta.IsNotFound(err) {
logger.Info("Local Azure Peering not found for KCP VpcPeering")
return nil, nil
}
if err != nil {
if azuremeta.IsTooManyRequests(err) {
return composed.LogErrorAndReturn(err,
"Azure vpc peering too many requests on peering local load",
composed.StopWithRequeueDelay(util.Timing.T10000ms()),
ctx,
)
}

if azuremeta.IsNotFound(err) {
logger.Info("Local Azure Peering not found for KCP VpcPeering")
return nil, nil
}

return azuremeta.LogErrorAndReturn(err, "Error loading VPC Peering", ctx)
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/kcp/provider/azure/vpcpeering/peeringRemoteLoad.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ func peeringRemoteLoad(ctx context.Context, st composed.State) (error, context.C

// remote client not created
if state.remoteClient == nil {
logger.Info("Azure remote client not initialized. Skipping loading of remote peering")
return nil, nil
}

var resourceGroup, networkName string
ok := false

if len(state.ObjAsVpcPeering().Status.Id) > 0 {
if len(state.ObjAsVpcPeering().Status.RemoteId) > 0 {
resourceID, err := azureutil.ParseResourceID(state.ObjAsVpcPeering().Status.RemoteId)
if err == nil {
resourceGroup = resourceID.ResourceGroup
Expand All @@ -33,7 +34,7 @@ func peeringRemoteLoad(ctx context.Context, st composed.State) (error, context.C

}

if !ok && state.localNetworkId != nil {
if !ok && state.remoteNetworkId != nil {
resourceGroup = state.remoteNetworkId.ResourceGroup
networkName = state.remoteNetworkId.NetworkName()
ok = true
Expand Down
2 changes: 1 addition & 1 deletion pkg/kcp/provider/azure/vpcpeering/remoteClientCreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func remoteClientCreate(ctx context.Context, st composed.State) (error, context.
SetExclusiveConditions(metav1.Condition{
Type: cloudcontrolv1beta1.ConditionTypeError,
Status: metav1.ConditionTrue,
Reason: cloudcontrolv1beta1.ConditionTypeError,
Reason: cloudcontrolv1beta1.ReasonRemoteClientFailed,
Message: fmt.Sprintf("Faile creating Azure client for tenant %s subscription %s", state.remoteNetworkId.Subscription, tenantId),
}).
ErrorLogMessage("Error patching KCP VpcPeering with error state after remote client creation failed").
Expand Down
Loading