Skip to content

Commit

Permalink
remove subnet manager
Browse files Browse the repository at this point in the history
  • Loading branch information
@bitoku
bitoku committed Oct 24, 2024
1 parent 520b325 commit f7b208f
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 195 deletions.
13 changes: 9 additions & 4 deletions pkg/operator/controllers/previewfeature/nsgflowlogs/nsgflowlogs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
sdknetwork "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2"
"github.com/Azure/go-autorest/autorest/azure"
"k8s.io/utils/ptr"
Expand All @@ -16,7 +17,7 @@ import (
"github.com/Azure/ARO-RP/pkg/util/subnet"
)

func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.KubeManager, subnets subnet.Manager, location string) *nsgFlowLogsFeature {
func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.KubeManager, subnets armnetwork.SubnetsClient, location string) *nsgFlowLogsFeature {
return &nsgFlowLogsFeature{
kubeSubnets: kubeSubnets,
flowLogsClient: flowLogsClient,
Expand All @@ -27,7 +28,7 @@ func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.Kub

type nsgFlowLogsFeature struct {
kubeSubnets subnet.KubeManager
subnets subnet.Manager
subnets armnetwork.SubnetsClient
flowLogsClient armnetwork.FlowLogsClient
location string
}
Expand Down Expand Up @@ -135,11 +136,15 @@ func (n *nsgFlowLogsFeature) getNSGs(ctx context.Context) (map[string]struct{},

nsgs := map[string]struct{}{}
for _, kubeSubnet := range subnets {
net, err := n.subnets.Get(ctx, kubeSubnet.ResourceID)
r, err := arm.ParseResourceID(kubeSubnet.ResourceID)
if err != nil {
return nil, err
}
nsgs[*net.NetworkSecurityGroup.ID] = struct{}{}
net, err := n.subnets.Get(ctx, r.ResourceGroupName, r.Parent.Name, r.Name, nil)
if err != nil {
return nil, err
}
nsgs[*net.Properties.NetworkSecurityGroup.ID] = struct{}{}
}
return nsgs, nil
}
91 changes: 53 additions & 38 deletions pkg/operator/controllers/previewfeature/nsgflowlogs/nsgflowlogs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2"
mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
"github.com/Azure/go-autorest/autorest/to"
"go.uber.org/mock/gomock"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -85,14 +84,14 @@ func getValidFlowLogFeature() *armnetwork.FlowLog {
func TestReconcileManager(t *testing.T) {
for _, tt := range []struct {
name string
subnetMock func(*mock_subnet.MockManager, *mock_subnet.MockKubeManager)
subnetMock func(*mock_armnetwork.MockSubnetsClient, *mock_subnet.MockKubeManager)
instance func(*aropreviewv1alpha1.PreviewFeature)
flowLogClientMock func(*mock_armnetwork.MockFlowLogsClient)
wantErr string
}{
{
name: "do not enable flow log if parameters are missing/wrong",
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
{
ResourceID: resourceIdMaster,
Expand All @@ -101,17 +100,21 @@ func TestReconcileManager(t *testing.T) {
ResourceID: resourceIdWorker,
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
},
},
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID,
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID,
},
},
},
}, nil)
Expand All @@ -123,7 +126,7 @@ func TestReconcileManager(t *testing.T) {
},
{
name: "enable flow log",
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
{
ResourceID: resourceIdMaster,
Expand All @@ -135,24 +138,30 @@ func TestReconcileManager(t *testing.T) {
ResourceID: resourceIdWorker2,
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
},
},
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
},
},
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdWorker2).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID, // different NSG ID. expect another one call to create
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker2, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID, // different NSG ID. expect another one call to create
},
},
},
}, nil)
Expand All @@ -175,7 +184,7 @@ func TestReconcileManager(t *testing.T) {
},
{
name: "disable flow log",
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
{
ResourceID: resourceIdMaster,
Expand All @@ -187,24 +196,30 @@ func TestReconcileManager(t *testing.T) {
ResourceID: resourceIdWorker2,
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID,
},
},
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
},
},
},
}, nil)
mock.EXPECT().Get(gomock.Any(), resourceIdWorker2).Return(&mgmtnetwork.Subnet{
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID, // in order to test calls to disable once per NSG
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker2, nil).Return(armnetwork.SubnetsClientGetResponse{
Subnet: armnetwork.Subnet{
Properties: &armnetwork.SubnetPropertiesFormat{
NetworkSecurityGroup: &armnetwork.SecurityGroup{
ID: &subnetNameWorkerNSGID, // in order to test calls to disable once per NSG
},
},
},
}, nil)
Expand All @@ -224,7 +239,7 @@ func TestReconcileManager(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()

subnets := mock_subnet.NewMockManager(controller)
subnets := mock_armnetwork.NewMockSubnetsClient(controller)
kubeSubnets := mock_subnet.NewMockKubeManager(controller)
if tt.subnetMock != nil {
tt.subnetMock(subnets, kubeSubnets)
Expand Down
17 changes: 4 additions & 13 deletions pkg/operator/controllers/previewfeature/previewfeature_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/Azure/ARO-RP/pkg/operator/controllers/previewfeature/nsgflowlogs"
"github.com/Azure/ARO-RP/pkg/util/azureclient"
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armnetwork"
"github.com/Azure/ARO-RP/pkg/util/clusterauthorizer"
"github.com/Azure/ARO-RP/pkg/util/subnet"
)

Expand Down Expand Up @@ -73,17 +72,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, err
}

// create refreshable authorizer from token
azRefreshAuthorizer, err := clusterauthorizer.NewAzRefreshableAuthorizer(r.log, &azEnv, r.client)
if err != nil {
return reconcile.Result{}, err
}

authorizer, err := azRefreshAuthorizer.NewRefreshableAuthorizerToken(ctx)
if err != nil {
return reconcile.Result{}, err
}

credential, err := azidentity.NewDefaultAzureCredential(azEnv.DefaultAzureCredentialOptions())
if err != nil {
return reconcile.Result{}, err
Expand All @@ -97,7 +85,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
}

kubeSubnets := subnet.NewKubeManager(r.client, resource.SubscriptionID)
subnets := subnet.NewManager(&azEnv, resource.SubscriptionID, authorizer)
subnets, err := armnetwork.NewSubnetsClient(resource.SubscriptionID, credential, options)
if err != nil {
return reconcile.Result{}, err
}

features := []feature{
nsgflowlogs.NewFeature(flowLogsClient, kubeSubnets, subnets, clusterInstance.Spec.Location),
Expand Down
37 changes: 0 additions & 37 deletions pkg/util/azureclient/mgmt/network/flowlogs.go
View file

This file was deleted.

34 changes: 0 additions & 34 deletions pkg/util/azureclient/mgmt/network/flowlogs_addons.go
View file

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/util/azureclient/mgmt/network/generate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ package network
// Licensed under the Apache License 2.0.

//go:generate rm -rf ../../../../util/mocks/$GOPACKAGE
//go:generate mockgen -destination=../../../../util/mocks/azureclient/mgmt/$GOPACKAGE/$GOPACKAGE.go github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/$GOPACKAGE InterfacesClient,LoadBalancersClient,PrivateEndpointsClient,PublicIPAddressesClient,LoadBalancerBackendAddressPoolsClient,RouteTablesClient,SubnetsClient,VirtualNetworksClient,UsageClient,FlowLogsClient
//go:generate mockgen -destination=../../../../util/mocks/azureclient/mgmt/$GOPACKAGE/$GOPACKAGE.go github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/$GOPACKAGE InterfacesClient,LoadBalancersClient,PrivateEndpointsClient,PublicIPAddressesClient,LoadBalancerBackendAddressPoolsClient,RouteTablesClient,SubnetsClient,VirtualNetworksClient,UsageClient
//go:generate goimports -local=github.com/Azure/ARO-RP -e -w ../../../../util/mocks/azureclient/mgmt/$GOPACKAGE/$GOPACKAGE.go
Loading

0 comments on commit f7b208f

Please sign in to comment.