Skip to content

Commit 2c59400

Browse files
vbhargav875l-technicore
vbhargav875
authored and
l-technicore
committed
Add annotation to enable/disable NLB source destination
1 parent a99ad7c commit 2c59400

File tree

5 files changed

+458
-153
lines changed

5 files changed

+458
-153
lines changed

‎docs/load-balancer-annotations.md

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -78,19 +78,20 @@ Note:
7878

7979
## Network Load Balancer Specific Annotations
8080

81-
| Name | Description | Default |
82-
|----------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------|
83-
| `oci-network-load-balancer.oraclecloud.com/internal` | Create an [internal network load balancer][1]. Cannot be modified after load balancer creation. | `false` |
84-
| `oci-network-load-balancer.oraclecloud.com/subnet` | The OCID of the required regional or AD specific subnet to attach the network load balancer. | Value set for the cluster |
85-
| `oci-network-load-balancer.oraclecloud.com/oci-network-security-groups` | Specifies Network Security Groups' OCIDs to be associated with the network load balancer. | `""` |
86-
| `oci-network-load-balancer.oraclecloud.com/initial-freeform-tags-override` | Specifies one or multiple Freeform tags to apply to the OCI Network Load Balancer. | `""` |
87-
| `oci-network-load-balancer.oraclecloud.com/initial-defined-tags-override` | Specifies one or multiple Defined tags to apply to the OCI Network Load Balancer. | `""` |
88-
| `oci-network-load-balancer.oraclecloud.com/health-check-retries` | The number of retries to attempt before a backend server is considered "unhealthy". | `3` |
89-
| `oci-network-load-balancer.oraclecloud.com/health-check-timeout` | The maximum time, in milliseconds, to wait for a reply to a health check. A health check is successful only if a reply returns within this timeout period. | `3000 ms` |
90-
| `oci-network-load-balancer.oraclecloud.com/health-check-interval` | The interval between health checks requests, in milliseconds. | `3000 ms` |
91-
| `oci-network-load-balancer.oraclecloud.com/backend-policy` | The network load balancer policy for the backend set. Valid values: "TWO_TUPLE", "THREE_TUPLE", or "FIVE_TUPLE" | `"FIVE_TUPLE"` |
92-
| `oci-network-load-balancer.oraclecloud.com/security-list-management-mode` | Specifies the security list mode ("All", "Frontend","None") to configure how security lists are managed. | `"None"` |
93-
| `oci-network-load-balancer.oraclecloud.com/node-label-selector` | Specifies which nodes to add as a backend to the OCI Network Load Balancer. | `"None"` |
81+
| Name | Description | Default |
82+
|-----------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------|
83+
| `oci-network-load-balancer.oraclecloud.com/internal` | Create an [internal network load balancer][1]. Cannot be modified after load balancer creation. | `false` |
84+
| `oci-network-load-balancer.oraclecloud.com/subnet` | The OCID of the required regional or AD specific subnet to attach the network load balancer. | Value set for the cluster |
85+
| `oci-network-load-balancer.oraclecloud.com/oci-network-security-groups` | Specifies Network Security Groups' OCIDs to be associated with the network load balancer. | `""` |
86+
| `oci-network-load-balancer.oraclecloud.com/initial-freeform-tags-override` | Specifies one or multiple Freeform tags to apply to the OCI Network Load Balancer. | `""` |
87+
| `oci-network-load-balancer.oraclecloud.com/initial-defined-tags-override` | Specifies one or multiple Defined tags to apply to the OCI Network Load Balancer. | `""` |
88+
| `oci-network-load-balancer.oraclecloud.com/health-check-retries` | The number of retries to attempt before a backend server is considered "unhealthy". | `3` |
89+
| `oci-network-load-balancer.oraclecloud.com/health-check-timeout` | The maximum time, in milliseconds, to wait for a reply to a health check. A health check is successful only if a reply returns within this timeout period. | `3000 ms` |
90+
| `oci-network-load-balancer.oraclecloud.com/health-check-interval` | The interval between health checks requests, in milliseconds. | `3000 ms` |
91+
| `oci-network-load-balancer.oraclecloud.com/backend-policy` | The network load balancer policy for the backend set. Valid values: "TWO_TUPLE", "THREE_TUPLE", or "FIVE_TUPLE" | `"FIVE_TUPLE"` |
92+
| `oci-network-load-balancer.oraclecloud.com/security-list-management-mode` | Specifies the security list mode ("All", "Frontend","None") to configure how security lists are managed. | `"None"` |
93+
| `oci-network-load-balancer.oraclecloud.com/node-label-selector` | Specifies which nodes to add as a backend to the OCI Network Load Balancer. | `"None"` |
94+
| `oci-network-load-balancer.oraclecloud.com/is-preserve-source` | Enable or disable the network load balancer to preserve source address of incoming traffic. Can be set only when externalTrafficPolicy is set to Local. | `"true" (if externalTrafficPolicy=Local)` |
9495

9596
Note:
9697
- The only security list management mode allowed when backend protocol is UDP is "None"

‎pkg/cloudprovider/providers/oci/load_balancer.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ func (clb *CloudLoadBalancerProvider) createLoadBalancer(ctx context.Context, sp
339339
if status != nil && len(status.Ingress) > 0 {
340340
// If the LB is successfully provisioned then open lb/node subnet seclists egress/ingress.
341341
for _, ports := range spec.Ports {
342-
if err = spec.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports, *spec.IsPreserveSourceDestination); err != nil {
342+
if err = spec.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports, *spec.IsPreserveSource); err != nil {
343343
return nil, "", err
344344
}
345345
}
@@ -701,7 +701,7 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancer(ctx context.Context, lb
701701
// We try to update the seclist this way to prevent replication
702702
// of seclist reconciliation logic
703703
for _, ports := range spec.Ports {
704-
if err = spec.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports, *spec.IsPreserveSourceDestination); err != nil {
704+
if err = spec.securityListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, nil, ports, *spec.IsPreserveSource); err != nil {
705705
return err
706706
}
707707
}
@@ -756,7 +756,7 @@ func (clb *CloudLoadBalancerProvider) updateBackendSet(ctx context.Context, lbID
756756

757757
switch action.Type() {
758758
case Create:
759-
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSourceDestination)
759+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSource)
760760
if err != nil {
761761
return err
762762
}
@@ -765,12 +765,12 @@ func (clb *CloudLoadBalancerProvider) updateBackendSet(ctx context.Context, lbID
765765
case Update:
766766
// For NLB, due to source IP preservation we need to ensure ingress rules from sourceCIDRs are added to
767767
// the backends subnet's seclist as well
768-
if err = secListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, action.OldPorts, ports, *spec.IsPreserveSourceDestination); err != nil {
768+
if err = secListManager.Update(ctx, lbSubnets, nodeSubnets, spec.SourceCIDRs, action.OldPorts, ports, *spec.IsPreserveSource); err != nil {
769769
return err
770770
}
771771
workRequestID, err = clb.lbClient.UpdateBackendSet(ctx, lbID, action.Name(), &bs)
772772
case Delete:
773-
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports, sourceCIDRs, *spec.IsPreserveSourceDestination)
773+
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports, sourceCIDRs, *spec.IsPreserveSource)
774774
if err != nil {
775775
return err
776776
}
@@ -808,21 +808,21 @@ func (clb *CloudLoadBalancerProvider) updateListener(ctx context.Context, lbID s
808808

809809
switch action.Type() {
810810
case Create:
811-
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSourceDestination)
811+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSource)
812812
if err != nil {
813813
return err
814814
}
815815

816816
workRequestID, err = clb.lbClient.CreateListener(ctx, lbID, action.Name(), &listener)
817817
case Update:
818-
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSourceDestination)
818+
err = secListManager.Update(ctx, lbSubnets, nodeSubnets, sourceCIDRs, nil, ports, *spec.IsPreserveSource)
819819
if err != nil {
820820
return err
821821
}
822822

823823
workRequestID, err = clb.lbClient.UpdateListener(ctx, lbID, action.Name(), &listener)
824824
case Delete:
825-
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports, sourceCIDRs, *spec.IsPreserveSourceDestination)
825+
err = secListManager.Delete(ctx, lbSubnets, nodeSubnets, ports, sourceCIDRs, *spec.IsPreserveSource)
826826
if err != nil {
827827
return err
828828
}
@@ -989,7 +989,11 @@ func (cp *CloudProvider) cleanupSecListForLoadBalancerDelete(lb *client.GenericL
989989
securityListManager := cp.securityListManagerFactory(
990990
secListManagerMode)
991991

992-
isPreserveSource := getPreserveSourceDestination(service)
992+
isPreserveSource, err := getPreserveSource(logger, service)
993+
if err != nil {
994+
logger.With(zap.Error(err)).Error("failed to determine value for is-preserve-source")
995+
return errors.Wrap(err, "failed to determine value for is-preserve-source")
996+
}
993997

994998
for listenerName, listener := range lb.Listeners {
995999
backendSetName := *listener.DefaultBackendSetName

‎pkg/cloudprovider/providers/oci/load_balancer_spec.go

Lines changed: 79 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ const (
202202
// ServiceAnnotationNetworkLoadBalancerNodeFilter is a service annotation to select specific nodes as your backend in the NLB
203203
// based on label selector.
204204
ServiceAnnotationNetworkLoadBalancerNodeFilter = "oci-network-load-balancer.oraclecloud.com/node-label-selector"
205+
206+
// ServiceAnnotationNetworkLoadBalancerIsPreserveSource is a service annotation to enable/disable preserving source information
207+
// on the NLB traffic. Default value when no annotation is given is to enable this for NLBs with externalTrafficPolicy=Local.
208+
ServiceAnnotationNetworkLoadBalancerIsPreserveSource = "oci-network-load-balancer.oraclecloud.com/is-preserve-source"
205209
)
206210

207211
// certificateData is a structure containing the data about a K8S secret required
@@ -267,24 +271,24 @@ func NewSSLConfig(secretListenerString string, secretBackendSetString string, se
267271
// LBSpec holds the data required to build a OCI load balancer from a
268272
// kubernetes service.
269273
type LBSpec struct {
270-
Type string
271-
Name string
272-
Shape string
273-
FlexMin *int
274-
FlexMax *int
275-
Subnets []string
276-
Internal bool
277-
Listeners map[string]client.GenericListener
278-
BackendSets map[string]client.GenericBackendSetDetails
279-
LoadBalancerIP string
280-
IsPreserveSourceDestination *bool
281-
Ports map[string]portSpec
282-
SourceCIDRs []string
283-
SSLConfig *SSLConfig
284-
securityListManager securityListManager
285-
NetworkSecurityGroupIds []string
286-
FreeformTags map[string]string
287-
DefinedTags map[string]map[string]interface{}
274+
Type string
275+
Name string
276+
Shape string
277+
FlexMin *int
278+
FlexMax *int
279+
Subnets []string
280+
Internal bool
281+
Listeners map[string]client.GenericListener
282+
BackendSets map[string]client.GenericBackendSetDetails
283+
LoadBalancerIP string
284+
IsPreserveSource *bool
285+
Ports map[string]portSpec
286+
SourceCIDRs []string
287+
SSLConfig *SSLConfig
288+
securityListManager securityListManager
289+
NetworkSecurityGroupIds []string
290+
FreeformTags map[string]string
291+
DefinedTags map[string]map[string]interface{}
288292

289293
service *v1.Service
290294
nodes []*v1.Node
@@ -316,9 +320,12 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sub
316320
return nil, err
317321
}
318322

319-
isPreserveSourceDestination := getPreserveSourceDestination(svc)
323+
isPreserveSource, err := getPreserveSource(logger, svc)
324+
if err != nil {
325+
return nil, err
326+
}
320327

321-
backendSets, err := getBackendSets(logger, svc, nodes, sslConfig, isPreserveSourceDestination)
328+
backendSets, err := getBackendSets(logger, svc, nodes, sslConfig, isPreserveSource)
322329
if err != nil {
323330
return nil, err
324331
}
@@ -351,26 +358,26 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sub
351358
lbType := getLoadBalancerType(svc)
352359

353360
return &LBSpec{
354-
Type: lbType,
355-
Name: GetLoadBalancerName(svc),
356-
Shape: shape,
357-
FlexMin: flexShapeMinMbps,
358-
FlexMax: flexShapeMaxMbps,
359-
Internal: internal,
360-
Subnets: subnets,
361-
Listeners: listeners,
362-
BackendSets: backendSets,
363-
LoadBalancerIP: loadbalancerIP,
364-
IsPreserveSourceDestination: &isPreserveSourceDestination,
365-
Ports: ports,
366-
SSLConfig: sslConfig,
367-
SourceCIDRs: sourceCIDRs,
368-
NetworkSecurityGroupIds: networkSecurityGroupIds,
369-
service: svc,
370-
nodes: nodes,
371-
securityListManager: secListFactory(secListManagerMode),
372-
FreeformTags: lbTags.FreeformTags,
373-
DefinedTags: lbTags.DefinedTags,
361+
Type: lbType,
362+
Name: GetLoadBalancerName(svc),
363+
Shape: shape,
364+
FlexMin: flexShapeMinMbps,
365+
FlexMax: flexShapeMaxMbps,
366+
Internal: internal,
367+
Subnets: subnets,
368+
Listeners: listeners,
369+
BackendSets: backendSets,
370+
LoadBalancerIP: loadbalancerIP,
371+
IsPreserveSource: &isPreserveSource,
372+
Ports: ports,
373+
SSLConfig: sslConfig,
374+
SourceCIDRs: sourceCIDRs,
375+
NetworkSecurityGroupIds: networkSecurityGroupIds,
376+
service: svc,
377+
nodes: nodes,
378+
securityListManager: secListFactory(secListManagerMode),
379+
FreeformTags: lbTags.FreeformTags,
380+
DefinedTags: lbTags.DefinedTags,
374381
}, nil
375382
}
376383

@@ -459,15 +466,40 @@ func validateService(svc *v1.Service) error {
459466
return nil
460467
}
461468

462-
func getPreserveSourceDestination(svc *v1.Service) bool {
469+
func getPreserveSource(logger *zap.SugaredLogger, svc *v1.Service) (bool, error) {
463470
if svc.Annotations[ServiceAnnotationLoadBalancerType] != NLB {
464-
return false
471+
return false, nil
472+
}
473+
// fail the request if externalTrafficPolicy is set to Cluster and is-preserve-source annotation is set
474+
if svc.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeCluster {
475+
_, ok := svc.Annotations[ServiceAnnotationNetworkLoadBalancerIsPreserveSource]
476+
if ok {
477+
logger.Error("error : externalTrafficPolicy is set to Cluster and the %s annotation is set", ServiceAnnotationNetworkLoadBalancerIsPreserveSource)
478+
return false, fmt.Errorf("%s annotation cannot be set when externalTrafficPolicy is set to Cluster", ServiceAnnotationNetworkLoadBalancerIsPreserveSource)
479+
}
480+
}
481+
482+
enablePreservation, err := getPreserveSourceAnnotation(logger, svc)
483+
if err != nil {
484+
return false, err
465485
}
486+
if svc.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal && enablePreservation {
487+
return true, nil
488+
}
489+
return false, nil
490+
}
466491

467-
if svc.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal {
468-
return true
492+
func getPreserveSourceAnnotation(logger *zap.SugaredLogger, svc *v1.Service) (bool, error) {
493+
if annotationString, ok := svc.Annotations[ServiceAnnotationNetworkLoadBalancerIsPreserveSource]; ok {
494+
enable, err := strconv.ParseBool(annotationString)
495+
if err != nil {
496+
logger.Error("failed to to parse %s annotation value - %s", ServiceAnnotationNetworkLoadBalancerIsPreserveSource, annotationString)
497+
return false, fmt.Errorf("failed to to parse %s annotation value - %s", ServiceAnnotationNetworkLoadBalancerIsPreserveSource, annotationString)
498+
}
499+
return enable, nil
469500
}
470-
return false
501+
// default behavior is to enable source destination preservation
502+
return true, nil
471503
}
472504

473505
func getLoadBalancerSourceRanges(service *v1.Service) ([]string, error) {
@@ -544,7 +576,7 @@ func getBackends(logger *zap.SugaredLogger, nodes []*v1.Node, nodePort int32) []
544576
return backends
545577
}
546578

547-
func getBackendSets(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sslCfg *SSLConfig, isPreserveSourceDestination bool) (map[string]client.GenericBackendSetDetails, error) {
579+
func getBackendSets(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sslCfg *SSLConfig, isPreserveSource bool) (map[string]client.GenericBackendSetDetails, error) {
548580
backendSets := make(map[string]client.GenericBackendSetDetails)
549581
loadbalancerPolicy, err := getLoadBalancerPolicy(svc)
550582
if err != nil {
@@ -579,7 +611,7 @@ func getBackendSets(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node
579611
Policy: &loadbalancerPolicy,
580612
Backends: getBackends(logger, nodes, servicePort.NodePort),
581613
HealthChecker: healthChecker,
582-
IsPreserveSource: &isPreserveSourceDestination,
614+
IsPreserveSource: &isPreserveSource,
583615
SslConfiguration: getSSLConfiguration(sslCfg, secretName, port),
584616
}
585617
}

0 commit comments

Comments
 (0)