Skip to content

Commit d3d2191

Browse files
bdourallawziprydie
bdourallawzi
authored and
prydie
committed
Support configuration of security list management modes as LB service annotation (#225)
1 parent 4d5e784 commit d3d2191

File tree

5 files changed

+61
-37
lines changed

5 files changed

+61
-37
lines changed

‎pkg/oci/ccm.go‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3232
wait "k8s.io/apimachinery/pkg/util/wait"
3333
informers "k8s.io/client-go/informers"
34-
informersv1 "k8s.io/client-go/informers/core/v1"
3534
clientset "k8s.io/client-go/kubernetes"
3635
listersv1 "k8s.io/client-go/listers/core/v1"
3736
cache "k8s.io/client-go/tools/cache"
@@ -59,8 +58,8 @@ type CloudProvider struct {
5958
client client.Interface
6059
kubeclient clientset.Interface
6160

62-
securityListManager securityListManager
63-
config *Config
61+
securityListManagerFactory securityListManagerFactory
62+
config *Config
6463
}
6564

6665
// Compile time check that CloudProvider implements the cloudprovider.Interface
@@ -130,24 +129,23 @@ func (cp *CloudProvider) Initialize(clientBuilder controller.ControllerClientBui
130129

131130
nodeInformer := factory.Core().V1().Nodes()
132131
go nodeInformer.Informer().Run(wait.NeverStop)
132+
serviceInformer := factory.Core().V1().Services()
133+
go serviceInformer.Informer().Run(wait.NeverStop)
134+
133135
glog.Info("Waiting for node informer cache to sync")
134-
if !cache.WaitForCacheSync(wait.NeverStop, nodeInformer.Informer().HasSynced) {
135-
utilruntime.HandleError(fmt.Errorf("Timed out waiting for node informer to sync"))
136+
if !cache.WaitForCacheSync(wait.NeverStop, nodeInformer.Informer().HasSynced, serviceInformer.Informer().HasSynced) {
137+
utilruntime.HandleError(fmt.Errorf("Timed out waiting for informers to sync"))
136138
}
137139
cp.NodeLister = nodeInformer.Lister()
138140

139-
if !cp.config.LoadBalancer.Disabled {
140-
var serviceInformer informersv1.ServiceInformer
141-
if cp.config.LoadBalancer.SecurityListManagementMode != ManagementModeNone {
142-
serviceInformer = factory.Core().V1().Services()
143-
go serviceInformer.Informer().Run(wait.NeverStop)
144-
glog.Info("Waiting for service informer cache to sync")
145-
if !cache.WaitForCacheSync(wait.NeverStop, serviceInformer.Informer().HasSynced) {
146-
utilruntime.HandleError(fmt.Errorf("Timed out waiting for service informer to sync"))
147-
}
141+
cp.securityListManagerFactory = func(mode string) securityListManager {
142+
if cp.config.LoadBalancer.Disabled {
143+
return newSecurityListManagerNOOP()
148144
}
149-
150-
cp.securityListManager = newSecurityListManager(cp.client, serviceInformer, cp.config.LoadBalancer.SecurityLists, cp.config.LoadBalancer.SecurityListManagementMode)
145+
if len(mode) == 0 {
146+
mode = cp.config.LoadBalancer.SecurityListManagementMode
147+
}
148+
return newSecurityListManager(cp.client, serviceInformer, cp.config.LoadBalancer.SecurityLists, mode)
151149
}
152150
}
153151

‎pkg/oci/load_balancer.go‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,18 @@ const (
5858
ServiceAnnotationLoadBalancerSSLPorts = "service.beta.kubernetes.io/oci-load-balancer-ssl-ports"
5959

6060
// ServiceAnnotationLoadBalancerTLSSecret is a Service annotation for
61-
// specifying the TLS secret ti install on the load balancer listeners which
61+
// specifying the TLS secret to install on the load balancer listeners which
6262
// have SSL enabled.
6363
// See: https://kubernetes.io/docs/concepts/services-networking/ingress/#tls
6464
ServiceAnnotationLoadBalancerTLSSecret = "service.beta.kubernetes.io/oci-load-balancer-tls-secret"
6565

6666
// ServiceAnnotationLoadBalancerConnectionIdleTimeout is the annotation used
6767
// on the service to specify the idle connection timeout.
6868
ServiceAnnotationLoadBalancerConnectionIdleTimeout = "service.beta.kubernetes.io/oci-load-balancer-connection-idle-timeout"
69+
70+
// ServiceAnnotaionLoadBalancerSecurityListManagementMode is a Service annotation for
71+
// specifying the security list managment mode ("All", "Frontend", "None") that configures how security lists are managed by the CCM
72+
ServiceAnnotaionLoadBalancerSecurityListManagementMode = "service.beta.kubernetes.io/oci-load-balancer-security-list-management-mode"
6973
)
7074

7175
// DefaultLoadBalancerPolicy defines the default traffic policy for load
@@ -242,7 +246,7 @@ func (cp *CloudProvider) createLoadBalancer(ctx context.Context, spec *LBSpec) (
242246
}
243247

244248
for _, ports := range spec.Ports {
245-
if err = cp.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports); err != nil {
249+
if err = spec.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports); err != nil {
246250
return nil, err
247251
}
248252
}
@@ -305,7 +309,7 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
305309
ssl = NewSSLConfig(lbName, ports, cp)
306310
}
307311
subnets := []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}
308-
spec, err := NewLBSpec(service, nodes, subnets, ssl)
312+
spec, err := NewLBSpec(service, nodes, subnets, ssl, cp.securityListManagerFactory)
309313
if err != nil {
310314
glog.Errorf("Failed to derive LBSpec: %+v", err)
311315
return nil, err
@@ -364,7 +368,7 @@ func (cp *CloudProvider) updateLoadBalancer(ctx context.Context, lb *loadbalance
364368
for _, action := range actions {
365369
switch a := action.(type) {
366370
case *BackendSetAction:
367-
err := cp.updateBackendSet(ctx, lbID, a, lbSubnets, nodeSubnets)
371+
err := cp.updateBackendSet(ctx, lbID, a, lbSubnets, nodeSubnets, spec.securityListManager)
368372
if err != nil {
369373
return errors.Wrap(err, "updating BackendSet")
370374
}
@@ -381,7 +385,7 @@ func (cp *CloudProvider) updateLoadBalancer(ctx context.Context, lb *loadbalance
381385
ports = spec.Ports[backendSetName]
382386
}
383387

384-
err := cp.updateListener(ctx, lbID, a, ports, lbSubnets, nodeSubnets, spec.SourceCIDRs)
388+
err := cp.updateListener(ctx, lbID, a, ports, lbSubnets, nodeSubnets, spec.SourceCIDRs, spec.securityListManager)
385389
if err != nil {
386390
return errors.Wrap(err, "updating listener")
387391
}
@@ -390,7 +394,7 @@ func (cp *CloudProvider) updateLoadBalancer(ctx context.Context, lb *loadbalance
390394
return nil
391395
}
392396

393-
func (cp *CloudProvider) updateBackendSet(ctx context.Context, lbID string, action *BackendSetAction, lbSubnets, nodeSubnets []*core.Subnet) error {
397+
func (cp *CloudProvider) updateBackendSet(ctx context.Context, lbID string, action *BackendSetAction, lbSubnets, nodeSubnets []*core.Subnet, secListManager securityListManager) error {
394398
var (
395399
sourceCIDRs = []string{}
396400
workRequestID string
@@ -403,19 +407,19 @@ func (cp *CloudProvider) updateBackendSet(ctx context.Context, lbID string, acti
403407

404408
switch action.Type() {
405409
case Create:
406-
err = cp.securityListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
410+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
407411
if err != nil {
408412
return err
409413
}
410414

411415
workRequestID, err = cp.client.LoadBalancer().CreateBackendSet(ctx, lbID, action.Name(), bs)
412416
case Update:
413-
if err = cp.securityListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, action.OldPorts, ports); err != nil {
417+
if err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, action.OldPorts, ports); err != nil {
414418
return err
415419
}
416420
workRequestID, err = cp.client.LoadBalancer().UpdateBackendSet(ctx, lbID, action.Name(), bs)
417421
case Delete:
418-
err = cp.securityListManager.Delete(ctx, lbSubnets, nodeSubnets, ports)
422+
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports)
419423
if err != nil {
420424
return err
421425
}
@@ -435,7 +439,7 @@ func (cp *CloudProvider) updateBackendSet(ctx context.Context, lbID string, acti
435439
return nil
436440
}
437441

438-
func (cp *CloudProvider) updateListener(ctx context.Context, lbID string, action *ListenerAction, ports portSpec, lbSubnets, nodeSubnets []*core.Subnet, sourceCIDRs []string) error {
442+
func (cp *CloudProvider) updateListener(ctx context.Context, lbID string, action *ListenerAction, ports portSpec, lbSubnets, nodeSubnets []*core.Subnet, sourceCIDRs []string, secListManager securityListManager) error {
439443
var workRequestID string
440444
var err error
441445
listener := action.Listener
@@ -445,21 +449,21 @@ func (cp *CloudProvider) updateListener(ctx context.Context, lbID string, action
445449

446450
switch action.Type() {
447451
case Create:
448-
err = cp.securityListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
452+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
449453
if err != nil {
450454
return err
451455
}
452456

453457
workRequestID, err = cp.client.LoadBalancer().CreateListener(ctx, lbID, action.Name(), listener)
454458
case Update:
455-
err = cp.securityListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
459+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports)
456460
if err != nil {
457461
return err
458462
}
459463

460464
workRequestID, err = cp.client.LoadBalancer().UpdateListener(ctx, lbID, action.Name(), listener)
461465
case Delete:
462-
err = cp.securityListManager.Delete(ctx, lbSubnets, nodeSubnets, ports)
466+
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports)
463467
if err != nil {
464468
return err
465469
}
@@ -547,6 +551,9 @@ func (cp *CloudProvider) EnsureLoadBalancerDeleted(ctx context.Context, clusterN
547551
return errors.Wrap(err, "getting subnets for load balancers")
548552
}
549553

554+
securityListManager := cp.securityListManagerFactory(
555+
service.Annotations[ServiceAnnotaionLoadBalancerSecurityListManagementMode])
556+
550557
for listenerName, listener := range lb.Listeners {
551558
backendSetName := *listener.DefaultBackendSetName
552559
bs, ok := lb.BackendSets[backendSetName]
@@ -559,7 +566,7 @@ func (cp *CloudProvider) EnsureLoadBalancerDeleted(ctx context.Context, clusterN
559566

560567
glog.V(4).Infof("Deleting security rules for listener %q for load balancer %q ports=%+v", listenerName, id, ports)
561568

562-
if err := cp.securityListManager.Delete(ctx, lbSubnets, nodeSubnets, ports); err != nil {
569+
if err := securityListManager.Delete(ctx, lbSubnets, nodeSubnets, ports); err != nil {
563570
return errors.Wrapf(err, "delete security rules for listener %q on load balancer %q", listenerName, name)
564571
}
565572
}

‎pkg/oci/load_balancer_security_lists.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ type baseSecurityListManager struct {
8282
securityLists map[string]string
8383
}
8484

85+
type securityListManagerFactory func(mode string) securityListManager
86+
8587
func newSecurityListManager(client client.Interface, serviceInformer informersv1.ServiceInformer, securityLists map[string]string, mode string) securityListManager {
8688
if securityLists == nil {
8789
securityLists = make(map[string]string)

‎pkg/oci/load_balancer_spec.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,17 @@ type LBSpec struct {
7474
Listeners map[string]loadbalancer.ListenerDetails
7575
BackendSets map[string]loadbalancer.BackendSetDetails
7676

77-
Ports map[string]portSpec
78-
SourceCIDRs []string
79-
SSLConfig *SSLConfig
77+
Ports map[string]portSpec
78+
SourceCIDRs []string
79+
SSLConfig *SSLConfig
80+
securityListManager securityListManager
8081

8182
service *v1.Service
8283
nodes []*v1.Node
8384
}
8485

8586
// NewLBSpec creates a LB Spec from a Kubernetes service and a slice of nodes.
86-
func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslCfg *SSLConfig) (*LBSpec, error) {
87+
func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslCfg *SSLConfig, secListFactory securityListManagerFactory) (*LBSpec, error) {
8788
if len(defaultSubnets) != 2 {
8889
return nil, errors.New("default subnets incorrectly configured")
8990
}
@@ -148,6 +149,8 @@ func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslCf
148149

149150
service: svc,
150151
nodes: nodes,
152+
securityListManager: secListFactory(
153+
svc.Annotations[ServiceAnnotaionLoadBalancerSecurityListManagementMode]),
151154
}, nil
152155
}
153156

‎pkg/oci/load_balancer_spec_test.go‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ func TestNewLBSpecSuccess(t *testing.T) {
8383
HealthCheckerPort: 10256,
8484
},
8585
},
86+
securityListManager: newSecurityListManagerNOOP(),
8687
},
8788
},
8889
"internal": {
@@ -137,6 +138,7 @@ func TestNewLBSpecSuccess(t *testing.T) {
137138
HealthCheckerPort: 10256,
138139
},
139140
},
141+
securityListManager: newSecurityListManagerNOOP(),
140142
},
141143
},
142144
"subnet annotations": {
@@ -192,8 +194,10 @@ func TestNewLBSpecSuccess(t *testing.T) {
192194
HealthCheckerPort: 10256,
193195
},
194196
},
197+
securityListManager: newSecurityListManagerNOOP(),
195198
},
196199
},
200+
//"security list manager annotation":
197201
"custom shape": {
198202
defaultSubnetOne: "one",
199203
defaultSubnetTwo: "two",
@@ -246,6 +250,7 @@ func TestNewLBSpecSuccess(t *testing.T) {
246250
HealthCheckerPort: 10256,
247251
},
248252
},
253+
securityListManager: newSecurityListManagerNOOP(),
249254
},
250255
},
251256
"custom idle connection timeout": {
@@ -303,6 +308,7 @@ func TestNewLBSpecSuccess(t *testing.T) {
303308
HealthCheckerPort: 10256,
304309
},
305310
},
311+
securityListManager: newSecurityListManagerNOOP(),
306312
},
307313
},
308314
}
@@ -312,7 +318,10 @@ func TestNewLBSpecSuccess(t *testing.T) {
312318
// we expect the service to be unchanged
313319
tc.expected.service = tc.service
314320
subnets := []string{tc.defaultSubnetOne, tc.defaultSubnetTwo}
315-
result, err := NewLBSpec(tc.service, tc.nodes, subnets, nil)
321+
slManagerFactory := func(mode string) securityListManager {
322+
return newSecurityListManagerNOOP()
323+
}
324+
result, err := NewLBSpec(tc.service, tc.nodes, subnets, nil, slManagerFactory)
316325
if err != nil {
317326
t.Error(err)
318327
}
@@ -330,7 +339,8 @@ func TestNewLBSpecFailure(t *testing.T) {
330339
defaultSubnetTwo string
331340
nodes []*v1.Node
332341
service *v1.Service
333-
expectedErrMsg string
342+
//add cp or cp security list
343+
expectedErrMsg string
334344
}{
335345
"unsupported udp protocol": {
336346
service: &v1.Service{
@@ -415,6 +425,7 @@ func TestNewLBSpecFailure(t *testing.T) {
415425
Spec: v1.ServiceSpec{
416426
SessionAffinity: v1.ServiceAffinityNone,
417427
Ports: []v1.ServicePort{},
428+
//add security list mananger in spec
418429
},
419430
},
420431
expectedErrMsg: "a configuration for subnet1 must be specified for an internal load balancer",
@@ -424,7 +435,10 @@ func TestNewLBSpecFailure(t *testing.T) {
424435
for name, tc := range testCases {
425436
t.Run(name, func(t *testing.T) {
426437
subnets := []string{tc.defaultSubnetOne, tc.defaultSubnetTwo}
427-
_, err := NewLBSpec(tc.service, tc.nodes, subnets, nil)
438+
slManagerFactory := func(mode string) securityListManager {
439+
return newSecurityListManagerNOOP()
440+
}
441+
_, err := NewLBSpec(tc.service, tc.nodes, subnets, nil, slManagerFactory)
428442
if err == nil || err.Error() != tc.expectedErrMsg {
429443
t.Errorf("Expected error with message %q but got %q", tc.expectedErrMsg, err)
430444
}

0 commit comments

Comments
 (0)