Skip to content

chore(VpcPeering): remove duplicated code that load local and remote KCP Network #1478

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Nov 12, 2025
Merged

chore(VpcPeering): remove duplicated code that load local and remote KCP Network #1478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1e49782
chore(VpcPeering): remove duplicated code that load local and remote …
vandjelk Oct 13, 2025
d49c643
chore(VpcPeering): remove duplicated code that loads local and remote…
vandjelk Oct 13, 2025
b9944ac
chore(VpcPeering): remove duplicated code that loads local and remote…
vandjelk Oct 13, 2025
88a1b1c
chore(VpcPeering): remove duplicated code that loads local and remote…
vandjelk Oct 16, 2025
95aaea7
Update pkg/kcp/vpcpeering/networkLocalLoad.go
vandjelk Oct 22, 2025
fd87a51
Update pkg/kcp/vpcpeering/networkLocalLoad.go
vandjelk Oct 22, 2025
4024d0c
Update pkg/kcp/vpcpeering/networkLocalLoad.go
vandjelk Oct 22, 2025
34d09fb
Update pkg/kcp/vpcpeering/networkLocalLoad.go
vandjelk Oct 22, 2025
6e0c040
feat(VpcPeering): log missing network reference
vandjelk Oct 22, 2025
589ac88
chore(VpcPeering): remove unnecessary network nil checks
vandjelk Oct 24, 2025
98ad694
chore(VpcPeering): handle network error explicitly
vandjelk Oct 24, 2025
40a69f9
chore(VpcPeering): remove unnecessary network nil checks
vandjelk Oct 24, 2025
02c3bfa
chore(VpcPeering): test Network reference missing
vandjelk Nov 4, 2025
0c56874
chore(VpcPeering): ReasonMissingDependency on Network nof found
vandjelk Nov 5, 2025
a20bf1e
chore(VpcPeering): Azure wait remote network
vandjelk Nov 5, 2025
9208d4a
chore(VpcPeering): wait remote network
vandjelk Nov 6, 2025
0797c8e
Merge branch 'main' into peering/load-kcp-networks
vandjelk Nov 6, 2025
a5fc6df
Merge branch 'main' into peering/load-kcp-networks
vandjelk Nov 6, 2025
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
320 changes: 0 additions & 320 deletions internal/controller/cloud-control/vpcpeering_aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
)

Expand Down Expand Up @@ -182,13 +181,6 @@ var _ = Describe("Feature: KCP VpcPeering", func() {

})

By("Then KCP VpcPeering is in missing local network error state", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering, NewObjActions(),
HavingCondition(cloudcontrolv1beta1.ConditionTypeError, metav1.ConditionTrue, cloudcontrolv1beta1.ReasonMissingDependency, "Local network not found"),
).Should(Succeed())
})

var localKcpNet *cloudcontrolv1beta1.Network

By("When local KCP Network is created", func() {
Expand All @@ -202,13 +194,6 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
Should(Succeed())
})

By("Then KCP VpcPeering is in waiting local network to be ready", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering, NewObjActions(),
HavingCondition(cloudcontrolv1beta1.ConditionTypeError, metav1.ConditionTrue, cloudcontrolv1beta1.ReasonWaitingDependency, "Local network not ready"),
).Should(Succeed())
})

By("When local KCP Network is ready", func() {
kcpnetwork.Ignore.RemoveName(localKcpNetworkName)
// trigger the reconciliation
Expand All @@ -219,13 +204,6 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
Should(Succeed(), "expected local kcp network to become ready but it didn't")
})

By("Then KCP VpcPeering is in missing remote network", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering, NewObjActions(),
HavingCondition(cloudcontrolv1beta1.ConditionTypeError, metav1.ConditionTrue, cloudcontrolv1beta1.ReasonMissingDependency, "Remote network not found"),
).Should(Succeed())
})

var remoteKcpNet *cloudcontrolv1beta1.Network

By("When remote KCP Network is created", func() {
Expand All @@ -239,13 +217,6 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
Should(Succeed())
})

By("Then KCP VpcPeering is in waiting remote network to be ready", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering, NewObjActions(),
HavingCondition(cloudcontrolv1beta1.ConditionTypeError, metav1.ConditionTrue, cloudcontrolv1beta1.ReasonWaitingDependency, "Remote network not ready"),
).Should(Succeed())
})

By("When remote KCP Network is ready", func() {
kcpnetwork.Ignore.RemoveName(remoteKcpNetworkName)
// trigger the reconciliation
Expand Down Expand Up @@ -400,297 +371,6 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
})
})

// When prevent deletion of KCP Network while used by VpcPeering is implemented, this test case
// is obsolete, but keeping it just in case, but with Network reconciler ignoring the created
// networks, so they can be deleted while used by VpcPeering
It("Scenario: KCP AWS VpcPeering is deleted when local and remote networks are missing", func() {
const (
kymaName = "76f1dec7-c7d3-4129-9730-478f4cba241a"
kcpPeeringName = "f658c189-0f09-4c4b-8da6-49b3db61546d"
localVpcId = "vpc-7e9d1ce03b49ae18d"
localVpcCidr = "10.180.0.0/16"
remoteVpcId = "vpc-3a1cdc66b2778658e"
remoteVpcCidr = "10.200.0.0/16"
remoteAccountId = "777755556666"
remoteRegion = "eu-west1"
localMainRouteTable = "rtb-007a6396ac2021245"
localRouteTable = "rtb-c44da7a78dbf49bde"
remoteMainRouteTable = "rtb-c0b83bb46e6d208b9"
remoteRouteTable = "rtb-30b3c0b6d895ed2d0"
)

scope := &cloudcontrolv1beta1.Scope{}

By("Given Scope exists", func() {
// Tell Scope reconciler to ignore this kymaName
kcpscope.Ignore.AddName(kymaName)

Eventually(CreateScopeAws).
WithArguments(infra.Ctx(), infra, scope, WithName(kymaName)).
Should(Succeed())
})

vpcName := scope.Spec.Scope.Aws.VpcNetwork
remoteVpcName := "Remote Network Name"

awsMockLocal := infra.AwsMock().MockConfigs(scope.Spec.Scope.Aws.AccountId, scope.Spec.Region)
awsMockRemote := infra.AwsMock().MockConfigs(remoteAccountId, remoteRegion)

By("And Given AWS VPC exists", func() {
awsMockLocal.AddVpc(
localVpcId,
localVpcCidr,
awsutil.Ec2Tags("Name", vpcName),
awsmock.VpcSubnetsFromScope(scope),
)
})

By("And Given AWS route table exists", func() {
awsMockLocal.AddRouteTable(
ptr.To(localMainRouteTable),
ptr.To(localVpcId),
awsutil.Ec2Tags(fmt.Sprintf("kubernetes.io/cluster/%s", vpcName), "1"),
[]ec2types.RouteTableAssociation{
{
Main: ptr.To(true),
},
})

awsMockLocal.AddRouteTable(
ptr.To(localRouteTable),
ptr.To(localVpcId),
awsutil.Ec2Tags(fmt.Sprintf("kubernetes.io/cluster/%s", vpcName), "1"),
[]ec2types.RouteTableAssociation{})
})

By("And Given AWS remote VPC exists", func() {
awsMockRemote.AddVpc(
remoteVpcId,
remoteVpcCidr,
awsutil.Ec2Tags("Name", remoteVpcName, kymaName, kymaName),
nil,
)
})

By("And Given AWS remote route table exists", func() {

awsMockRemote.AddRouteTable(
ptr.To(remoteMainRouteTable),
ptr.To(remoteVpcId),
awsutil.Ec2Tags(),
[]ec2types.RouteTableAssociation{
{
Main: ptr.To(true),
},
})

awsMockRemote.AddRouteTable(
ptr.To(remoteRouteTable),
ptr.To(remoteVpcId),
awsutil.Ec2Tags(),
[]ec2types.RouteTableAssociation{})
})

localKcpNetworkName := common.KcpNetworkKymaCommonName(scope.Name)
remoteKcpNetworkName := scope.Name + "--remote"

var localKcpNet *cloudcontrolv1beta1.Network

By("And Given local KCP Network exists", func() {
// must tell reconciler to ignore it, since it would prevent deletion when used by peering
kcpnetwork.Ignore.AddName(localKcpNetworkName)
localKcpNet = cloudcontrolv1beta1.NewNetworkBuilder().
WithScope(scope.Name).
WithAwsRef(scope.Spec.Scope.Aws.AccountId, scope.Spec.Region, scope.Spec.Scope.Aws.Network.VPC.Id, localKcpNetworkName).
Build()
Expect(CreateObj(infra.Ctx(), infra.KCP().Client(), localKcpNet, WithName(localKcpNetworkName))).
To(Succeed())

localKcpNet.Status.Network = localKcpNet.Spec.Network.Reference.DeepCopy()
localKcpNet.Status.State = string(cloudcontrolv1beta1.StateReady)
meta.SetStatusCondition(&localKcpNet.Status.Conditions, metav1.Condition{
Type: cloudcontrolv1beta1.ConditionTypeReady,
Status: metav1.ConditionTrue,
Reason: cloudcontrolv1beta1.ReasonReady,
Message: cloudcontrolv1beta1.ReasonReady,
})
Expect(composed.PatchObjStatus(infra.Ctx(), localKcpNet, infra.KCP().Client())).
To(Succeed())
})

var remoteKcpNet *cloudcontrolv1beta1.Network

By("And Given remote KCP Network exists", func() {
// must tell reconciler to ignore it, since it would prevent deletion when used by peering
kcpnetwork.Ignore.AddName(remoteKcpNetworkName)
remoteKcpNet = cloudcontrolv1beta1.NewNetworkBuilder().
WithScope(scope.Name).
WithAwsRef(remoteAccountId, remoteRegion, remoteVpcId, remoteVpcName).
Build()
Expect(CreateObj(infra.Ctx(), infra.KCP().Client(), remoteKcpNet, WithName(remoteKcpNetworkName))).
Should(Succeed())

remoteKcpNet.Status.Network = remoteKcpNet.Spec.Network.Reference.DeepCopy()
remoteKcpNet.Status.State = string(cloudcontrolv1beta1.StateReady)
meta.SetStatusCondition(&remoteKcpNet.Status.Conditions, metav1.Condition{
Type: cloudcontrolv1beta1.ConditionTypeReady,
Status: metav1.ConditionTrue,
Reason: cloudcontrolv1beta1.ReasonReady,
Message: cloudcontrolv1beta1.ReasonReady,
})
Expect(composed.PatchObjStatus(infra.Ctx(), remoteKcpNet, infra.KCP().Client())).
To(Succeed())
})

var kcpPeering *cloudcontrolv1beta1.VpcPeering

By("When KCP VpcPeering is created", func() {
kcpPeering = (&cloudcontrolv1beta1.VpcPeeringBuilder{}).
WithScope(kymaName).
WithRemoteRef("skr-namespace", "skr-aws-ip-range").
WithDetails(localKcpNetworkName, infra.KCP().Namespace(), remoteKcpNetworkName, infra.KCP().Namespace(), "", false, true).
Build()

Eventually(CreateObj).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering,
WithName(kcpPeeringName),
).Should(Succeed())

})

By("Then KCP VpcPeering has status id", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering,
NewObjActions(),
HaveFinalizer(api.CommonFinalizerDeletionHook),
HavingKcpVpcPeeringStatusIdNotEmpty(),
).Should(Succeed())
})

By("When remote VpcPeeringConnection is initiated", func() {
awsMockRemote.InitiateVpcPeeringConnection(kcpPeering.Status.Id, localVpcId, remoteVpcId)
})

By("When AWS VPC Peering state is active", func() {
Expect(
awsMockLocal.SetVpcPeeringConnectionStatusCode(localVpcId, remoteVpcId, ec2types.VpcPeeringConnectionStateReasonCodeActive),
).NotTo(HaveOccurred())

Expect(
awsMockRemote.SetVpcPeeringConnectionStatusCode(localVpcId, remoteVpcId, ec2types.VpcPeeringConnectionStateReasonCodeActive),
).NotTo(HaveOccurred())
})

By("Then KCP VpcPeering is ready", func() {
Eventually(LoadAndCheck).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering,
NewObjActions(),
HavingConditionTrue(cloudcontrolv1beta1.ConditionTypeReady),
).
Should(Succeed())
})

By("And Then KCP VpcPeering status id equals to remoteId", func() {
Expect(kcpPeering.Status.Id).To(Equal(kcpPeering.Status.RemoteId))
})

By("And Then KCP VpcPeering status vpcId equals to existing AWS VPC id", func() {
Expect(kcpPeering.Status.VpcId).To(Equal(localVpcId))
})

By("And Then found local VpcPeeringConnection AccepterVpcInfo VpcId equals to remote vpc id", func() {
localPeering, _ := awsMockLocal.DescribeVpcPeeringConnection(infra.Ctx(), kcpPeering.Status.Id)
Expect(*localPeering.AccepterVpcInfo.VpcId).To(Equal(remoteVpcId))
})

By("And Then local route tables have peering route to remote VPC CIDR", func() {
Expect(awsMockLocal.GetRoute(localVpcId, localMainRouteTable, kcpPeering.Status.Id, remoteVpcCidr)).
NotTo(BeNil(), fmt.Sprintf("Local main route table should have peering route to %s", remoteVpcCidr))

Expect(awsMockLocal.GetRoute(localVpcId, localRouteTable, kcpPeering.Status.Id, remoteVpcCidr)).
ToNot(BeNil(), fmt.Sprintf("Local route table should have peering route to %s", remoteVpcCidr))
})

By("And Then remote route tables have peering route to VPC CIDR", func() {
Expect(awsMockRemote.GetRoute(remoteVpcId, remoteMainRouteTable, kcpPeering.Status.RemoteId, localVpcCidr)).
NotTo(BeNil(), fmt.Sprintf("Remote route table should have peering route to %s", remoteVpcCidr))

Expect(awsMockRemote.GetRoute(remoteVpcId, remoteRouteTable, kcpPeering.Status.RemoteId, localVpcCidr)).
ToNot(BeNil(), fmt.Sprintf("Remote route table should have peering route to %s", remoteVpcCidr))
})

// Deleting KCP remote Network before VpcPeering deletion
By("When KCP local Network is deleted", func() {
Eventually(Delete).
WithArguments(infra.Ctx(), infra.KCP().Client(), localKcpNet).
Should(Succeed(), "failed deleting local KCP Network")
})

By("Then KCP local Network does not exist", func() {
Eventually(IsDeleted).
WithArguments(infra.Ctx(), infra.KCP().Client(), localKcpNet).
Should(Succeed(), "expected KCP local Network not to exist (be deleted), but it still exists")
})

// Deleting KCP remote Network before VpcPeering deletion
By("When KCP remote Network is deleted", func() {
Eventually(Delete).
WithArguments(infra.Ctx(), infra.KCP().Client(), remoteKcpNet).
Should(Succeed(), "failed deleting remote KCP Network")
})

By("Then KCP remote Network does not exist", func() {
Eventually(IsDeleted).
WithArguments(infra.Ctx(), infra.KCP().Client(), remoteKcpNet).
Should(Succeed(), "expected KCP remote Network not to exist (be deleted), but it still exists")
})

// DELETE

By("When KCP VpcPeering is deleted", func() {
Eventually(Delete).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering).
Should(Succeed(), "failed deleting VpcPeering")
})

By("Then VpcPeering does not exist", func() {
Eventually(IsDeleted).
WithArguments(infra.Ctx(), infra.KCP().Client(), kcpPeering).
Should(Succeed(), "expected VpcPeering not to exist (be deleted), but it still exists")
})

By("And Then local VpcPeeringConnection is deleted", func() {
localPeering, err := awsMockLocal.DescribeVpcPeeringConnection(infra.Ctx(), kcpPeering.Status.Id)
Expect(err).To(HaveOccurred())
Expect(localPeering).To(BeNil())
})

By("And Then local route tables have no peering routes to remote VPC CIDRs", func() {
Expect(awsMockLocal.GetRoute(localVpcId, localMainRouteTable, kcpPeering.Status.Id, remoteVpcCidr)).
To(BeNil(), fmt.Sprintf("Local main route table should not have peering route to %s", remoteVpcCidr))

Expect(awsMockLocal.GetRoute(localVpcId, localRouteTable, kcpPeering.Status.Id, remoteVpcCidr)).
To(BeNil(), fmt.Sprintf("Local route table should not have peering route to %s", remoteVpcCidr))

})

// VpcPeeringConnection and Routes are not deleted since KCP remote Network is deleted previously
By("And Then remote VpcPeeringConnection is not deleted", func() {
remotePeering, err := awsMockRemote.DescribeVpcPeeringConnection(infra.Ctx(), kcpPeering.Status.Id)
Expect(err).NotTo(HaveOccurred())
Expect(remotePeering).NotTo(BeNil())
})

By("And Then remote route tables have peering routes to local VPC CIDR", func() {
Expect(awsMockRemote.GetRoute(remoteVpcId, remoteMainRouteTable, kcpPeering.Status.RemoteId, localVpcCidr)).
NotTo(BeNil(), fmt.Sprintf("Remote main route table should have peering route to %s", localVpcCidr))

Expect(awsMockRemote.GetRoute(remoteVpcId, remoteRouteTable, kcpPeering.Status.RemoteId, localVpcCidr)).
NotTo(BeNil(), fmt.Sprintf("Remote route table should have peering route to %s", localVpcCidr))

})
})

It("Scenario: KCP AWS VpcPeering can be deleted when remote VPC Network authorization is revoked", func() {
const (
kymaName = "50de99f8-0b35-4ac2-900e-793091f1a853"
Expand Down
Loading
Loading