Skip to content

Commit f511938

Browse files
bitokubitoku
committed
remove subnet manager
1 parent 583fc59 commit f511938

File tree

7 files changed

+69
-195
lines changed

7 files changed

+69
-195
lines changed

‎pkg/operator/controllers/previewfeature/nsgflowlogs/nsgflowlogs.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"time"
99

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

19-
func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.KubeManager, subnets subnet.Manager, location string) *nsgFlowLogsFeature {
20+
func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.KubeManager, subnets armnetwork.SubnetsClient, location string) *nsgFlowLogsFeature {
2021
return &nsgFlowLogsFeature{
2122
kubeSubnets: kubeSubnets,
2223
flowLogsClient: flowLogsClient,
@@ -27,7 +28,7 @@ func NewFeature(flowLogsClient armnetwork.FlowLogsClient, kubeSubnets subnet.Kub
2728

2829
type nsgFlowLogsFeature struct {
2930
kubeSubnets subnet.KubeManager
30-
subnets subnet.Manager
31+
subnets armnetwork.SubnetsClient
3132
flowLogsClient armnetwork.FlowLogsClient
3233
location string
3334
}
@@ -135,11 +136,15 @@ func (n *nsgFlowLogsFeature) getNSGs(ctx context.Context) (map[string]struct{},
135136

136137
nsgs := map[string]struct{}{}
137138
for _, kubeSubnet := range subnets {
138-
net, err := n.subnets.Get(ctx, kubeSubnet.ResourceID)
139+
r, err := arm.ParseResourceID(kubeSubnet.ResourceID)
139140
if err != nil {
140141
return nil, err
141142
}
142-
nsgs[*net.NetworkSecurityGroup.ID] = struct{}{}
143+
net, err := n.subnets.Get(ctx, r.ResourceGroupName, r.Parent.Name, r.Name, nil)
144+
if err != nil {
145+
return nil, err
146+
}
147+
nsgs[*net.Properties.NetworkSecurityGroup.ID] = struct{}{}
143148
}
144149
return nsgs, nil
145150
}

‎pkg/operator/controllers/previewfeature/nsgflowlogs/nsgflowlogs_test.go

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"testing"
99

1010
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2"
11-
mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
1211
"github.com/Azure/go-autorest/autorest/to"
1312
"go.uber.org/mock/gomock"
1413
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -85,14 +84,14 @@ func getValidFlowLogFeature() *armnetwork.FlowLog {
8584
func TestReconcileManager(t *testing.T) {
8685
for _, tt := range []struct {
8786
name string
88-
subnetMock func(*mock_subnet.MockManager, *mock_subnet.MockKubeManager)
87+
subnetMock func(*mock_armnetwork.MockSubnetsClient, *mock_subnet.MockKubeManager)
8988
instance func(*aropreviewv1alpha1.PreviewFeature)
9089
flowLogClientMock func(*mock_armnetwork.MockFlowLogsClient)
9190
wantErr string
9291
}{
9392
{
9493
name: "do not enable flow log if parameters are missing/wrong",
95-
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
94+
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
9695
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
9796
{
9897
ResourceID: resourceIdMaster,
@@ -101,17 +100,21 @@ func TestReconcileManager(t *testing.T) {
101100
ResourceID: resourceIdWorker,
102101
},
103102
}, nil)
104-
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
105-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
106-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
107-
ID: &subnetNameMasterNSGID,
103+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
104+
Subnet: armnetwork.Subnet{
105+
Properties: &armnetwork.SubnetPropertiesFormat{
106+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
107+
ID: &subnetNameMasterNSGID,
108+
},
108109
},
109110
},
110111
}, nil)
111-
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
112-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
113-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
114-
ID: &subnetNameWorkerNSGID,
112+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
113+
Subnet: armnetwork.Subnet{
114+
Properties: &armnetwork.SubnetPropertiesFormat{
115+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
116+
ID: &subnetNameWorkerNSGID,
117+
},
115118
},
116119
},
117120
}, nil)
@@ -123,7 +126,7 @@ func TestReconcileManager(t *testing.T) {
123126
},
124127
{
125128
name: "enable flow log",
126-
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
129+
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
127130
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
128131
{
129132
ResourceID: resourceIdMaster,
@@ -135,24 +138,30 @@ func TestReconcileManager(t *testing.T) {
135138
ResourceID: resourceIdWorker2,
136139
},
137140
}, nil)
138-
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
139-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
140-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
141-
ID: &subnetNameMasterNSGID,
141+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
142+
Subnet: armnetwork.Subnet{
143+
Properties: &armnetwork.SubnetPropertiesFormat{
144+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
145+
ID: &subnetNameMasterNSGID,
146+
},
142147
},
143148
},
144149
}, nil)
145-
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
146-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
147-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
148-
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
150+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
151+
Subnet: armnetwork.Subnet{
152+
Properties: &armnetwork.SubnetPropertiesFormat{
153+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
154+
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
155+
},
149156
},
150157
},
151158
}, nil)
152-
mock.EXPECT().Get(gomock.Any(), resourceIdWorker2).Return(&mgmtnetwork.Subnet{
153-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
154-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
155-
ID: &subnetNameWorkerNSGID, // different NSG ID. expect another one call to create
159+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker2, nil).Return(armnetwork.SubnetsClientGetResponse{
160+
Subnet: armnetwork.Subnet{
161+
Properties: &armnetwork.SubnetPropertiesFormat{
162+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
163+
ID: &subnetNameWorkerNSGID, // different NSG ID. expect another one call to create
164+
},
156165
},
157166
},
158167
}, nil)
@@ -175,7 +184,7 @@ func TestReconcileManager(t *testing.T) {
175184
},
176185
{
177186
name: "disable flow log",
178-
subnetMock: func(mock *mock_subnet.MockManager, kmock *mock_subnet.MockKubeManager) {
187+
subnetMock: func(mock *mock_armnetwork.MockSubnetsClient, kmock *mock_subnet.MockKubeManager) {
179188
kmock.EXPECT().List(gomock.Any()).Return([]subnet.Subnet{
180189
{
181190
ResourceID: resourceIdMaster,
@@ -187,24 +196,30 @@ func TestReconcileManager(t *testing.T) {
187196
ResourceID: resourceIdWorker2,
188197
},
189198
}, nil)
190-
mock.EXPECT().Get(gomock.Any(), resourceIdMaster).Return(&mgmtnetwork.Subnet{
191-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
192-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
193-
ID: &subnetNameMasterNSGID,
199+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameMaster, nil).Return(armnetwork.SubnetsClientGetResponse{
200+
Subnet: armnetwork.Subnet{
201+
Properties: &armnetwork.SubnetPropertiesFormat{
202+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
203+
ID: &subnetNameMasterNSGID,
204+
},
194205
},
195206
},
196207
}, nil)
197-
mock.EXPECT().Get(gomock.Any(), resourceIdWorker).Return(&mgmtnetwork.Subnet{
198-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
199-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
200-
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
208+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker, nil).Return(armnetwork.SubnetsClientGetResponse{
209+
Subnet: armnetwork.Subnet{
210+
Properties: &armnetwork.SubnetPropertiesFormat{
211+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
212+
ID: &subnetNameMasterNSGID, // same NSG as the master subnet
213+
},
201214
},
202215
},
203216
}, nil)
204-
mock.EXPECT().Get(gomock.Any(), resourceIdWorker2).Return(&mgmtnetwork.Subnet{
205-
SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
206-
NetworkSecurityGroup: &mgmtnetwork.SecurityGroup{
207-
ID: &subnetNameWorkerNSGID, // in order to test calls to disable once per NSG
217+
mock.EXPECT().Get(gomock.Any(), vnetResourceGroup, vnetName, subnetNameWorker2, nil).Return(armnetwork.SubnetsClientGetResponse{
218+
Subnet: armnetwork.Subnet{
219+
Properties: &armnetwork.SubnetPropertiesFormat{
220+
NetworkSecurityGroup: &armnetwork.SecurityGroup{
221+
ID: &subnetNameWorkerNSGID, // in order to test calls to disable once per NSG
222+
},
208223
},
209224
},
210225
}, nil)
@@ -224,7 +239,7 @@ func TestReconcileManager(t *testing.T) {
224239
controller := gomock.NewController(t)
225240
defer controller.Finish()
226241

227-
subnets := mock_subnet.NewMockManager(controller)
242+
subnets := mock_armnetwork.NewMockSubnetsClient(controller)
228243
kubeSubnets := mock_subnet.NewMockKubeManager(controller)
229244
if tt.subnetMock != nil {
230245
tt.subnetMock(subnets, kubeSubnets)

‎pkg/operator/controllers/previewfeature/previewfeature_controller.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/Azure/ARO-RP/pkg/operator/controllers/previewfeature/nsgflowlogs"
2222
"github.com/Azure/ARO-RP/pkg/util/azureclient"
2323
"github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armnetwork"
24-
"github.com/Azure/ARO-RP/pkg/util/clusterauthorizer"
2524
"github.com/Azure/ARO-RP/pkg/util/subnet"
2625
)
2726

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

76-
// create refreshable authorizer from token
77-
azRefreshAuthorizer, err := clusterauthorizer.NewAzRefreshableAuthorizer(r.log, &azEnv, r.client)
78-
if err != nil {
79-
return reconcile.Result{}, err
80-
}
81-
82-
authorizer, err := azRefreshAuthorizer.NewRefreshableAuthorizerToken(ctx)
83-
if err != nil {
84-
return reconcile.Result{}, err
85-
}
86-
8775
credential, err := azidentity.NewDefaultAzureCredential(azEnv.DefaultAzureCredentialOptions())
8876
if err != nil {
8977
return reconcile.Result{}, err
@@ -93,7 +81,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
9381

9482
flowLogsClient, err := armnetwork.NewFlowLogsClient(resource.SubscriptionID, credential, options)
9583
kubeSubnets := subnet.NewKubeManager(r.client, resource.SubscriptionID)
96-
subnets := subnet.NewManager(&azEnv, resource.SubscriptionID, authorizer)
84+
subnets, err := armnetwork.NewSubnetsClient(resource.SubscriptionID, credential, options)
85+
if err != nil {
86+
return reconcile.Result{}, err
87+
}
9788

9889
features := []feature{
9990
nsgflowlogs.NewFeature(flowLogsClient, kubeSubnets, subnets, clusterInstance.Spec.Location),

‎pkg/util/azureclient/mgmt/network/flowlogs.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎pkg/util/azureclient/mgmt/network/flowlogs_addons.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎pkg/util/azureclient/mgmt/network/generate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ package network
44
// Licensed under the Apache License 2.0.
55

66
//go:generate rm -rf ../../../../util/mocks/$GOPACKAGE
7-
//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
7+
//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
88
//go:generate goimports -local=github.com/Azure/ARO-RP -e -w ../../../../util/mocks/azureclient/mgmt/$GOPACKAGE/$GOPACKAGE.go

0 commit comments

Comments
 (0)