Skip to content

Commit d99a88f

Browse files
piyush-tiwaripiyush-tiwari
committed
add NSG Support for IngressClass
- Added NSG Support for IngressClassess via new annotation - Fixed LB Shape update by replacing retryToken with etag
1 parent 418cd06 commit d99a88f

File tree

11 files changed

+236
-21
lines changed

11 files changed

+236
-21
lines changed

‎pkg/controllers/backend/backend_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
295295
return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
296296
}
297297

298+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
299+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
300+
}
301+
298302
func (m MockLoadBalancerClient) GetLoadBalancer(ctx context.Context, request ociloadbalancer.GetLoadBalancerRequest) (ociloadbalancer.GetLoadBalancerResponse, error) {
299303
res := util.SampleLoadBalancerResponse()
300304
return res, nil

‎pkg/controllers/ingress/ingress_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
225225
return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
226226
}
227227

228+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
229+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
230+
}
231+
228232
func (m MockLoadBalancerClient) CreateLoadBalancer(ctx context.Context, request ociloadbalancer.CreateLoadBalancerRequest) (ociloadbalancer.CreateLoadBalancerResponse, error) {
229233
return ociloadbalancer.CreateLoadBalancerResponse{}, nil
230234
}

‎pkg/controllers/ingressclass/ingressclass.go‎

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -207,33 +207,35 @@ func (c *Controller) sync(key string) error {
207207
return nil
208208
}
209209

210-
func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) (*ociloadbalancer.LoadBalancer, error) {
210+
func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) (*ociloadbalancer.LoadBalancer, string, error) {
211211
lbID := util.GetIngressClassLoadBalancerId(ic)
212212
if lbID == "" {
213-
klog.Errorf("LB id not set for ingressClass: %s", ic.Name)
214-
return nil, nil // LoadBalancer ID not set, Trigger new LB creation
213+
klog.Infof("LB id not set for ingressClass: %s", ic.Name)
214+
return nil, "", nil // LoadBalancer ID not set, Trigger new LB creation
215215
}
216216
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
217217
if !ok {
218-
return nil, fmt.Errorf(util.OciClientNotFoundInContextError)
218+
return nil, "", fmt.Errorf(util.OciClientNotFoundInContextError)
219219
}
220-
lb, _, err := wrapperClient.GetLbClient().GetLoadBalancer(context.TODO(), lbID)
220+
221+
lb, etag, err := wrapperClient.GetLbClient().GetLoadBalancer(context.TODO(), lbID)
221222
if err != nil {
222223
klog.Errorf("Error while fetching LB %s for ingressClass: %s, err: %s", lbID, ic.Name, err.Error())
223224

224225
// Check if Service error 404, then ignore it since LB is not found.
225226
svcErr, ok := common.IsServiceError(err)
226227
if ok && svcErr.GetHTTPStatusCode() == 404 {
227-
return nil, nil // Redirect new LB creation
228+
return nil, "", nil // Redirect new LB creation
228229
}
229-
return nil, err
230+
return nil, "", err
230231
}
231-
return lb, nil
232+
233+
return lb, etag, nil
232234
}
233235

234236
func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) error {
235237

236-
lb, err := c.getLoadBalancer(ctx, ic)
238+
lb, etag, err := c.getLoadBalancer(ctx, ic)
237239
if err != nil {
238240
return err
239241
}
@@ -262,11 +264,12 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
262264
klog.V(2).InfoS("Creating load balancer for ingress class", "ingressClass", ic.Name)
263265

264266
createDetails := ociloadbalancer.CreateLoadBalancerDetails{
265-
CompartmentId: compartmentId,
266-
DisplayName: common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
267-
ShapeName: common.String("flexible"),
268-
SubnetIds: []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
269-
IsPrivate: common.Bool(icp.Spec.IsPrivate),
267+
CompartmentId: compartmentId,
268+
DisplayName: common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
269+
ShapeName: common.String("flexible"),
270+
SubnetIds: []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
271+
IsPrivate: common.Bool(icp.Spec.IsPrivate),
272+
NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic),
270273
BackendSets: map[string]ociloadbalancer.BackendSetDetails{
271274
util.DefaultBackendSetName: {
272275
Policy: common.String("LEAST_CONNECTIONS"),
@@ -300,7 +303,15 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
300303
return err
301304
}
302305
} else {
303-
c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp)
306+
err = c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp, etag)
307+
if err != nil {
308+
return err
309+
}
310+
311+
err = c.checkForNetworkSecurityGroupsUpdate(ctx, ic)
312+
if err != nil {
313+
return err
314+
}
304315
}
305316

306317
if *lb.Id != util.GetIngressClassLoadBalancerId(ic) {
@@ -342,7 +353,8 @@ func (c *Controller) setupWebApplicationFirewall(ctx context.Context, ic *networ
342353
return nil
343354
}
344355

345-
func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, lb *ociloadbalancer.LoadBalancer, ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters) error {
356+
func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, lb *ociloadbalancer.LoadBalancer,
357+
ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters, etag string) error {
346358
// check LoadBalancerName AND MinBandwidthMbps ,MaxBandwidthMbps
347359
displayName := util.GetIngressClassLoadBalancerName(ic, icp)
348360
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
@@ -377,11 +389,11 @@ func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, l
377389

378390
req := ociloadbalancer.UpdateLoadBalancerShapeRequest{
379391
LoadBalancerId: lb.Id,
392+
IfMatch: common.String(etag),
380393
UpdateLoadBalancerShapeDetails: ociloadbalancer.UpdateLoadBalancerShapeDetails{
381394
ShapeName: common.String("flexible"),
382395
ShapeDetails: shapeDetails,
383396
},
384-
OpcRetryToken: common.String(fmt.Sprintf("update-lb-shape-%s", ic.UID)),
385397
}
386398
klog.Infof("Update lb shape request: %s", util.PrettyPrint(req))
387399
_, err := wrapperClient.GetLbClient().UpdateLoadBalancerShape(context.Background(), req)
@@ -393,6 +405,41 @@ func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, l
393405
return nil
394406
}
395407

408+
func (c *Controller) checkForNetworkSecurityGroupsUpdate(ctx context.Context, ic *networkingv1.IngressClass) error {
409+
lb, etag, err := c.getLoadBalancer(ctx, ic)
410+
if err != nil {
411+
return err
412+
}
413+
414+
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
415+
if !ok {
416+
return fmt.Errorf(util.OciClientNotFoundInContextError)
417+
}
418+
419+
nsgIdsFromSpec := util.GetIngressClassNetworkSecurityGroupIds(ic)
420+
421+
/*
422+
Only check if desired and actual slices have the same elements, ignoring order and duplicates
423+
We don't check if lb.NetworkSecurityGroupIds is nil since util.StringSlicesHaveSameElements returns true if
424+
one argument is nil and the other is empty.
425+
*/
426+
if util.StringSlicesHaveSameElements(nsgIdsFromSpec, lb.NetworkSecurityGroupIds) {
427+
return nil
428+
}
429+
430+
req := ociloadbalancer.UpdateNetworkSecurityGroupsRequest{
431+
LoadBalancerId: lb.Id,
432+
IfMatch: common.String(etag),
433+
UpdateNetworkSecurityGroupsDetails: ociloadbalancer.UpdateNetworkSecurityGroupsDetails{
434+
NetworkSecurityGroupIds: nsgIdsFromSpec,
435+
},
436+
}
437+
klog.Infof("Update lb nsg ids request: %s", util.PrettyPrint(req))
438+
439+
_, err = wrapperClient.GetLbClient().UpdateNetworkSecurityGroups(context.Background(), req)
440+
return err
441+
}
442+
396443
func (c *Controller) deleteIngressClass(ctx context.Context, ic *networkingv1.IngressClass) error {
397444

398445
err := c.deleteLoadBalancer(ctx, ic)

‎pkg/controllers/ingressclass/ingressclass_test.go‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,27 @@ func TestCheckForIngressClassParameterUpdates(t *testing.T) {
182182
MaxBandwidthMbps: 400,
183183
},
184184
}
185-
err = c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), loadBalancer, &ingressClassList.Items[0], &icp)
185+
err = c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), loadBalancer, &ingressClassList.Items[0], &icp, "etag")
186186
Expect(err).Should(BeNil())
187187
}
188188

189+
func TestCheckForNetworkSecurityGroupsUpdate(t *testing.T) {
190+
RegisterTestingT(t)
191+
192+
ctx, cancel := context.WithCancel(context.Background())
193+
defer cancel()
194+
195+
ingressClassList := util.GetIngressClassResourceWithAnnotation("ingress-class-with-nsg",
196+
map[string]string{
197+
util.IngressClassNetworkSecurityGroupIdsAnnotation: "id1,id2, id3",
198+
util.IngressClassLoadBalancerIdAnnotation: "id",
199+
}, "oci.oraclecloud.com/native-ingress-controller")
200+
c := inits(ctx, ingressClassList)
201+
202+
err := c.checkForNetworkSecurityGroupsUpdate(getContextWithClient(c, ctx), &ingressClassList.Items[0])
203+
Expect(err).To(BeNil())
204+
}
205+
189206
func TestDeleteFinalizer(t *testing.T) {
190207
RegisterTestingT(t)
191208
ctx, cancel := context.WithCancel(context.Background())
@@ -332,6 +349,15 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
332349
}, nil
333350
}
334351

352+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context,
353+
request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
354+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{
355+
RawResponse: nil,
356+
OpcWorkRequestId: common.String("id"),
357+
OpcRequestId: common.String("id"),
358+
}, nil
359+
}
360+
335361
func (m MockLoadBalancerClient) CreateLoadBalancer(ctx context.Context, request ociloadbalancer.CreateLoadBalancerRequest) (ociloadbalancer.CreateLoadBalancerResponse, error) {
336362
id := "id"
337363
return ociloadbalancer.CreateLoadBalancerResponse{

‎pkg/controllers/nodeBackend/nodeBackend_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
248248
return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
249249
}
250250

251+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
252+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
253+
}
254+
251255
func (m MockLoadBalancerClient) GetLoadBalancer(ctx context.Context, request ociloadbalancer.GetLoadBalancerRequest) (ociloadbalancer.GetLoadBalancerResponse, error) {
252256
res := util.SampleLoadBalancerResponse()
253257
return res, nil

‎pkg/controllers/routingpolicy/routingpolicy_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
220220
return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
221221
}
222222

223+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
224+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
225+
}
226+
223227
func (m MockLoadBalancerClient) CreateLoadBalancer(ctx context.Context, request ociloadbalancer.CreateLoadBalancerRequest) (ociloadbalancer.CreateLoadBalancerResponse, error) {
224228
return ociloadbalancer.CreateLoadBalancerResponse{}, nil
225229
}

‎pkg/loadbalancer/loadbalancer.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,21 @@ func (lbc *LoadBalancerClient) GetBackendSetHealth(ctx context.Context, lbID str
8686
return &resp.BackendSetHealth, nil
8787
}
8888

89+
func (lbc *LoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, req loadbalancer.UpdateNetworkSecurityGroupsRequest) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
90+
resp, err := lbc.LbClient.UpdateNetworkSecurityGroups(ctx, req)
91+
if err != nil {
92+
return resp, err
93+
}
94+
95+
lbID, err := lbc.waitForWorkRequest(ctx, *resp.OpcWorkRequestId)
96+
if err != nil {
97+
return resp, err
98+
}
99+
100+
_, _, err = lbc.getLoadBalancerBustCache(ctx, lbID)
101+
return resp, err
102+
}
103+
89104
func (lbc *LoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req loadbalancer.UpdateLoadBalancerShapeRequest) (response loadbalancer.UpdateLoadBalancerShapeResponse, err error) {
90105
resp, err := lbc.LbClient.UpdateLoadBalancerShape(ctx, req)
91106
if err != nil {

‎pkg/loadbalancer/loadbalancer_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,21 @@ func TestLoadBalancerClient_UpdateListener(t *testing.T) {
205205
Expect(err).To(BeNil())
206206
}
207207

208+
func TestLoadBalancerClient_UpdateNetworkSecurityGroups(t *testing.T) {
209+
RegisterTestingT(t)
210+
loadBalancerClient := setupLBClient()
211+
req := ociloadbalancer.UpdateNetworkSecurityGroupsRequest{
212+
LoadBalancerId: common.String("id"),
213+
UpdateNetworkSecurityGroupsDetails: ociloadbalancer.UpdateNetworkSecurityGroupsDetails{
214+
NetworkSecurityGroupIds: []string{"id1", "id2"},
215+
},
216+
OpcRetryToken: common.String("token"),
217+
}
218+
219+
_, err := loadBalancerClient.UpdateNetworkSecurityGroups(context.TODO(), req)
220+
Expect(err).To(BeNil())
221+
}
222+
208223
func setupLBClient() *LoadBalancerClient {
209224
lbClient := GetLoadBalancerClient()
210225

@@ -231,6 +246,15 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
231246
return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
232247
}
233248

249+
func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context,
250+
request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
251+
return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{
252+
RawResponse: nil,
253+
OpcWorkRequestId: common.String("id"),
254+
OpcRequestId: common.String("id"),
255+
}, nil
256+
}
257+
234258
func (m MockLoadBalancerClient) GetLoadBalancer(ctx context.Context, request ociloadbalancer.GetLoadBalancerRequest) (ociloadbalancer.GetLoadBalancerResponse, error) {
235259
res := util.SampleLoadBalancerResponse()
236260
return res, nil

‎pkg/oci/client/loadbalancer.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ type LoadBalancerInterface interface {
1111
CreateLoadBalancer(ctx context.Context, request loadbalancer.CreateLoadBalancerRequest) (loadbalancer.CreateLoadBalancerResponse, error)
1212
UpdateLoadBalancer(ctx context.Context, request loadbalancer.UpdateLoadBalancerRequest) (response loadbalancer.UpdateLoadBalancerResponse, err error)
1313
UpdateLoadBalancerShape(ctx context.Context, request loadbalancer.UpdateLoadBalancerShapeRequest) (response loadbalancer.UpdateLoadBalancerShapeResponse, err error)
14+
UpdateNetworkSecurityGroups(ctx context.Context, request loadbalancer.UpdateNetworkSecurityGroupsRequest) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error)
1415
DeleteLoadBalancer(ctx context.Context, request loadbalancer.DeleteLoadBalancerRequest) (loadbalancer.DeleteLoadBalancerResponse, error)
1516

1617
GetWorkRequest(ctx context.Context, request loadbalancer.GetWorkRequestRequest) (loadbalancer.GetWorkRequestResponse, error)
@@ -58,6 +59,10 @@ func (client LBClient) UpdateLoadBalancer(ctx context.Context, request loadbalan
5859
return client.lbClient.UpdateLoadBalancer(ctx, request)
5960
}
6061

62+
func (client LBClient) UpdateNetworkSecurityGroups(ctx context.Context, request loadbalancer.UpdateNetworkSecurityGroupsRequest) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
63+
return client.lbClient.UpdateNetworkSecurityGroups(ctx, request)
64+
}
65+
6166
func (client LBClient) DeleteLoadBalancer(ctx context.Context,
6267
request loadbalancer.DeleteLoadBalancerRequest) (loadbalancer.DeleteLoadBalancerResponse, error) {
6368
return client.lbClient.DeleteLoadBalancer(ctx, request)

‎pkg/util/util.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ const (
6363
// HTTP, HTTP2, TCP - accepted.
6464
IngressProtocolAnnotation = "oci-native-ingress.oraclecloud.com/protocol"
6565

66-
IngressPolicyAnnotation = "oci-native-ingress.oraclecloud.com/policy"
67-
IngressClassWafPolicyAnnotation = "oci-native-ingress.oraclecloud.com/waf-policy-ocid"
68-
IngressClassFireWallIdAnnotation = "oci-native-ingress.oraclecloud.com/firewall-id"
66+
IngressPolicyAnnotation = "oci-native-ingress.oraclecloud.com/policy"
67+
IngressClassWafPolicyAnnotation = "oci-native-ingress.oraclecloud.com/waf-policy-ocid"
68+
IngressClassFireWallIdAnnotation = "oci-native-ingress.oraclecloud.com/firewall-id"
69+
IngressClassNetworkSecurityGroupIdsAnnotation = "oci-native-ingress.oraclecloud.com/network-security-group-ids"
6970

7071
IngressHealthCheckProtocolAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-protocol"
7172
IngressHealthCheckPortAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-port"
@@ -153,6 +154,23 @@ func GetIngressClassFireWallId(ic *networkingv1.IngressClass) string {
153154
return value
154155
}
155156

157+
func GetIngressClassNetworkSecurityGroupIds(ic *networkingv1.IngressClass) []string {
158+
networkSecurityGroupIds := make([]string, 0)
159+
value, ok := ic.Annotations[IngressClassNetworkSecurityGroupIdsAnnotation]
160+
if !ok {
161+
return networkSecurityGroupIds
162+
}
163+
164+
for _, id := range strings.Split(value, ",") {
165+
trimmedId := strings.TrimSpace(id)
166+
if trimmedId != "" {
167+
networkSecurityGroupIds = append(networkSecurityGroupIds, trimmedId)
168+
}
169+
}
170+
171+
return networkSecurityGroupIds
172+
}
173+
156174
func GetIngressProtocol(i *networkingv1.Ingress) string {
157175
protocol, ok := i.Annotations[IngressProtocolAnnotation]
158176
if !ok {
@@ -719,3 +737,9 @@ func IsBackendServiceEqual(b1 *networkingv1.IngressBackend, b2 *networkingv1.Ing
719737
func IsIngressProtocolTCP(ingress *networkingv1.Ingress) bool {
720738
return GetIngressProtocol(ingress) == ProtocolTCP
721739
}
740+
741+
// StringSlicesHaveSameElements checks if s1 and s2 have the same elements, ignoring order and duplicates.
742+
// Returns true if one slice is nil and the other is empty.
743+
func StringSlicesHaveSameElements(s1 []string, s2 []string) bool {
744+
return sets.New(s1...).Equal(sets.New(s2...))
745+
}

0 commit comments

Comments
 (0)