Skip to content

Enable setting peering properties #16

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
nprokopic merged 4 commits into from
Apr 3, 2023
Merged

Enable setting peering properties #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b9e63a8
Enable setting peering properties
nprokopic Mar 10, 2023
b95ad46
Set missing peering parameters in Azure API call
nprokopic Mar 13, 2023
cea87be
Add unit tests for VNet peering with additional properties
nprokopic Mar 21, 2023
6c1ebe0
Refactor VNet peering unit tests
nprokopic Apr 3, 2023
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
20 changes: 20 additions & 0 deletions api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,26 @@ type VnetPeeringClassSpec struct {

// RemoteVnetName defines name of the remote virtual network.
RemoteVnetName string `json:"remoteVnetName"`

// +optional
ForwardPeeringProperties VnetPeeringProperties `json:"forwardPeeringProperties,omitempty"`

// +optional
ReversePeeringProperties VnetPeeringProperties `json:"reversePeeringProperties,omitempty"`
}

type VnetPeeringProperties struct {
// +optional
AllowForwardedTraffic *bool `json:"allowForwardedTraffic,omitempty"`

// +optional
AllowGatewayTransit *bool `json:"allowGatewayTransit,omitempty"`

// +optional
AllowVirtualNetworkAccess *bool `json:"allowVirtualNetworkAccess,omitempty"`

// +optional
UseRemoteGateways *bool `json:"useRemoteGateways,omitempty"`
}

// VnetPeerings is a slice of VnetPeering.
Expand Down
55 changes: 50 additions & 5 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 25 additions & 16 deletions azure/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import (
"github.com/pkg/errors"
"k8s.io/utils/net"
"k8s.io/utils/pointer"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"

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/services/bastionhosts"
Expand All @@ -45,10 +50,6 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// ClusterScopeParams defines the input parameters used to create a new Scope.
Expand Down Expand Up @@ -425,20 +426,28 @@ func (s *ClusterScope) VnetPeeringSpecs() []azure.ResourceSpecGetter {
peeringSpecs := make([]azure.ResourceSpecGetter, 2*len(s.Vnet().Peerings))
for i, peering := range s.Vnet().Peerings {
forwardPeering := &vnetpeerings.VnetPeeringSpec{
PeeringName: azure.GenerateVnetPeeringName(s.Vnet().Name, peering.RemoteVnetName),
SourceVnetName: s.Vnet().Name,
SourceResourceGroup: s.Vnet().ResourceGroup,
RemoteVnetName: peering.RemoteVnetName,
RemoteResourceGroup: peering.ResourceGroup,
SubscriptionID: s.SubscriptionID(),
PeeringName: azure.GenerateVnetPeeringName(s.Vnet().Name, peering.RemoteVnetName),
SourceVnetName: s.Vnet().Name,
SourceResourceGroup: s.Vnet().ResourceGroup,
RemoteVnetName: peering.RemoteVnetName,
RemoteResourceGroup: peering.ResourceGroup,
SubscriptionID: s.SubscriptionID(),
AllowForwardedTraffic: peering.ForwardPeeringProperties.AllowForwardedTraffic,
AllowGatewayTransit: peering.ForwardPeeringProperties.AllowGatewayTransit,
AllowVirtualNetworkAccess: peering.ForwardPeeringProperties.AllowVirtualNetworkAccess,
UseRemoteGateways: peering.ForwardPeeringProperties.UseRemoteGateways,
}
reversePeering := &vnetpeerings.VnetPeeringSpec{
PeeringName: azure.GenerateVnetPeeringName(peering.RemoteVnetName, s.Vnet().Name),
SourceVnetName: peering.RemoteVnetName,
SourceResourceGroup: peering.ResourceGroup,
RemoteVnetName: s.Vnet().Name,
RemoteResourceGroup: s.Vnet().ResourceGroup,
SubscriptionID: s.SubscriptionID(),
PeeringName: azure.GenerateVnetPeeringName(peering.RemoteVnetName, s.Vnet().Name),
SourceVnetName: peering.RemoteVnetName,
SourceResourceGroup: peering.ResourceGroup,
RemoteVnetName: s.Vnet().Name,
RemoteResourceGroup: s.Vnet().ResourceGroup,
SubscriptionID: s.SubscriptionID(),
AllowForwardedTraffic: peering.ReversePeeringProperties.AllowForwardedTraffic,
AllowGatewayTransit: peering.ReversePeeringProperties.AllowGatewayTransit,
AllowVirtualNetworkAccess: peering.ReversePeeringProperties.AllowVirtualNetworkAccess,
UseRemoteGateways: peering.ReversePeeringProperties.UseRemoteGateways,
}
peeringSpecs[i*2] = forwardPeering
peeringSpecs[i*2+1] = reversePeering
Expand Down
21 changes: 15 additions & 6 deletions azure/services/vnetpeerings/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,22 @@ import (
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network"
"github.com/pkg/errors"
"k8s.io/utils/pointer"

"sigs.k8s.io/cluster-api-provider-azure/azure"
)

// VnetPeeringSpec defines the specification for a virtual network peering.
type VnetPeeringSpec struct {
SourceResourceGroup string
SourceVnetName string
RemoteResourceGroup string
RemoteVnetName string
PeeringName string
SubscriptionID string
SourceResourceGroup string
SourceVnetName string
RemoteResourceGroup string
RemoteVnetName string
PeeringName string
SubscriptionID string
AllowForwardedTraffic *bool
AllowGatewayTransit *bool
AllowVirtualNetworkAccess *bool
UseRemoteGateways *bool
}

// ResourceName returns the name of the virtual network peering.
Expand Down Expand Up @@ -64,6 +69,10 @@ func (s *VnetPeeringSpec) Parameters(ctx context.Context, existing interface{})
RemoteVirtualNetwork: &network.SubResource{
ID: pointer.String(vnetID),
},
AllowForwardedTraffic: s.AllowForwardedTraffic,
AllowGatewayTransit: s.AllowGatewayTransit,
AllowVirtualNetworkAccess: s.AllowVirtualNetworkAccess,
UseRemoteGateways: s.UseRemoteGateways,
}
return network.VirtualNetworkPeering{
Name: pointer.String(s.PeeringName),
Expand Down
60 changes: 47 additions & 13 deletions azure/services/vnetpeerings/vnetpeerings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"
"k8s.io/utils/pointer"

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/services/async/mock_async"
Expand Down Expand Up @@ -65,6 +67,30 @@ var (
RemoteResourceGroup: "group1",
SubscriptionID: "sub1",
}
fakePeeringHubToSpoke = VnetPeeringSpec{
PeeringName: "hub-to-spoke",
SourceVnetName: "hub-vnet",
SourceResourceGroup: "hub-group",
RemoteVnetName: "spoke-vnet",
RemoteResourceGroup: "spoke-group",
SubscriptionID: "sub1",
AllowForwardedTraffic: pointer.Bool(true),
AllowGatewayTransit: pointer.Bool(true),
AllowVirtualNetworkAccess: pointer.Bool(true),
UseRemoteGateways: pointer.Bool(false),
}
fakePeeringSpokeToHub = VnetPeeringSpec{
PeeringName: "spoke-to-hub",
SourceVnetName: "spoke-vnet",
SourceResourceGroup: "spoke-group",
RemoteVnetName: "hub-vnet",
RemoteResourceGroup: "hub-group",
SubscriptionID: "sub1",
AllowForwardedTraffic: pointer.Bool(true),
AllowGatewayTransit: pointer.Bool(false),
AllowVirtualNetworkAccess: pointer.Bool(true),
UseRemoteGateways: pointer.Bool(true),
}
fakePeeringExtra = VnetPeeringSpec{
PeeringName: "extra-peering",
SourceVnetName: "vnet3",
Expand All @@ -73,7 +99,7 @@ var (
RemoteResourceGroup: "group4",
SubscriptionID: "sub1",
}
fakePeeringSpecs = []azure.ResourceSpecGetter{&fakePeering1To2, &fakePeering2To1, &fakePeering1To3, &fakePeering3To1}
fakePeeringSpecs = []azure.ResourceSpecGetter{&fakePeering1To2, &fakePeering2To1, &fakePeering1To3, &fakePeering3To1, &fakePeeringHubToSpoke, &fakePeeringSpokeToHub}
fakePeeringExtraSpecs = []azure.ResourceSpecGetter{&fakePeering1To2, &fakePeering2To1, &fakePeeringExtra}
internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: http.StatusInternalServerError}, "Internal Server Error")
notDoneError = azure.NewOperationNotDoneError(&infrav1.Future{})
Expand Down Expand Up @@ -131,6 +157,8 @@ func TestReconcileVnetPeerings(t *testing.T) {
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(&fakePeering1To3, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(&fakePeeringHubToSpoke, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(&fakePeeringSpokeToHub, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil)
},
},
Expand All @@ -143,18 +171,8 @@ func TestReconcileVnetPeerings(t *testing.T) {
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
{
name: "error in creating peering",
expectedError: "#: Internal Server Error: StatusCode=500",
expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
p.VnetPeeringSpecs().Return(fakePeeringSpecs)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(&fakePeeringHubToSpoke, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(&fakePeeringSpokeToHub, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -167,6 +185,8 @@ func TestReconcileVnetPeerings(t *testing.T) {
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(&fakePeeringHubToSpoke, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(&fakePeeringSpokeToHub, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -179,6 +199,8 @@ func TestReconcileVnetPeerings(t *testing.T) {
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(&fakePeeringHubToSpoke, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(&fakePeeringSpokeToHub, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -191,6 +213,8 @@ func TestReconcileVnetPeerings(t *testing.T) {
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(&fakePeeringHubToSpoke, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(&fakePeeringSpokeToHub, nil)
p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, notDoneError)
},
},
Expand Down Expand Up @@ -277,6 +301,8 @@ func TestDeleteVnetPeerings(t *testing.T) {
r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(nil)
p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil)
},
},
Expand All @@ -289,6 +315,8 @@ func TestDeleteVnetPeerings(t *testing.T) {
r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(internalError)
r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(nil)
p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -301,6 +329,8 @@ func TestDeleteVnetPeerings(t *testing.T) {
r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(internalError)
r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError)
r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(nil)
p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -313,6 +343,8 @@ func TestDeleteVnetPeerings(t *testing.T) {
r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError)
r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(internalError)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(nil)
p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError)
},
},
Expand All @@ -325,6 +357,8 @@ func TestDeleteVnetPeerings(t *testing.T) {
r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError)
r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringHubToSpoke, ServiceName).Return(nil)
r.DeleteResource(gomockinternal.AContext(), &fakePeeringSpokeToHub, ServiceName).Return(nil)
p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, notDoneError)
},
},
Expand Down
Loading