Skip to content

Commit 61b38fa

Browse files
pranavsriram8YashwantGohokar
authored andcommitted
Provide an option to skip private IP in LB Status for public NLBs
1 parent 140e9d9 commit 61b38fa

File tree

4 files changed

+364
-7
lines changed

4 files changed

+364
-7
lines changed

pkg/cloudprovider/providers/oci/load_balancer.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,11 @@ func (cp *CloudProvider) GetLoadBalancer(ctx context.Context, clusterName string
212212

213213
return nil, false, err
214214
}
215-
216-
lbStatus, err := loadBalancerToStatus(lb)
215+
skipPrivateIP, err := isSkipPrivateIP(service)
216+
if err != nil {
217+
return nil, false, err
218+
}
219+
lbStatus, err := loadBalancerToStatus(lb, nil, skipPrivateIP)
217220
return lbStatus, err == nil, err
218221
}
219222

@@ -464,7 +467,12 @@ func (clb *CloudLoadBalancerProvider) createLoadBalancer(ctx context.Context, sp
464467
}
465468

466469
logger.With("loadBalancerID", *lb.Id).Info("Load balancer created")
467-
status, err := loadBalancerToStatus(lb)
470+
471+
skipPrivateIP, err := isSkipPrivateIP(spec.service)
472+
if err != nil {
473+
return nil, "", err
474+
}
475+
status, err := loadBalancerToStatus(lb, spec.ingressIpMode, skipPrivateIP)
468476

469477
if status != nil && len(status.Ingress) > 0 {
470478
// If the LB is successfully provisioned then open lb/node subnet seclists egress/ingress.
@@ -835,6 +843,7 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
835843
dimensionsMap[metrics.ResourceOCIDDimension] = newLBOCID
836844
metrics.SendMetricData(cp.metricPusher, getMetric(loadBalancerType, Create), time.Since(startTime).Seconds(), dimensionsMap)
837845
}
846+
838847
return lbStatus, err
839848
}
840849

@@ -901,7 +910,12 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
901910
dimensionsMap[metrics.ComponentDimension] = lbMetricDimension
902911
dimensionsMap[metrics.BackendSetsCountDimension] = strconv.Itoa(len(lb.BackendSets))
903912
metrics.SendMetricData(cp.metricPusher, getMetric(loadBalancerType, Update), syncTime, dimensionsMap)
904-
return loadBalancerToStatus(lb)
913+
914+
skipPrivateIP, err := isSkipPrivateIP(service)
915+
if err != nil {
916+
return nil, err
917+
}
918+
return loadBalancerToStatus(lb, spec.ingressIpMode, skipPrivateIP)
905919
}
906920

907921
func getDefaultLBSubnets(subnet1, subnet2 string) []string {
@@ -1952,7 +1966,7 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancerIpVersion(ctx context.Co
19521966
}
19531967

19541968
// Given an OCI load balancer, return a LoadBalancerStatus
1955-
func loadBalancerToStatus(lb *client.GenericLoadBalancer) (*v1.LoadBalancerStatus, error) {
1969+
func loadBalancerToStatus(lb *client.GenericLoadBalancer, ipMode *v1.LoadBalancerIPMode, skipPrivateIp bool) (*v1.LoadBalancerStatus, error) {
19561970
if len(lb.IpAddresses) == 0 {
19571971
return nil, errors.Errorf("no ip addresses found for load balancer %q", *lb.DisplayName)
19581972
}
@@ -1962,8 +1976,15 @@ func loadBalancerToStatus(lb *client.GenericLoadBalancer) (*v1.LoadBalancerStatu
19621976
if ip.IpAddress == nil {
19631977
continue // should never happen but appears to when EnsureLoadBalancer is called with 0 nodes.
19641978
}
1965-
ingress = append(ingress, v1.LoadBalancerIngress{IP: *ip.IpAddress})
1979+
1980+
if skipPrivateIp {
1981+
if !pointer.BoolDeref(ip.IsPublic, false) {
1982+
continue
1983+
}
1984+
}
1985+
ingress = append(ingress, v1.LoadBalancerIngress{IP: *ip.IpAddress, IPMode: ipMode})
19661986
}
1987+
19671988
return &v1.LoadBalancerStatus{Ingress: ingress}, nil
19681989
}
19691990

pkg/cloudprovider/providers/oci/load_balancer_spec.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ const (
175175

176176
// ServiceAnnotationLoadbalancerBackendSetSSLConfig is a service annotation allows you to set the cipher suite on the backendSet
177177
ServiceAnnotationLoadbalancerBackendSetSSLConfig = "oci.oraclecloud.com/oci-load-balancer-backendset-ssl-config"
178+
179+
// ServiceAnnotationIngressIpMode is a service annotation allows you to set the ".status.loadBalancer.ingress.ipMode" for a Service
180+
// with type set to LoadBalancer.
181+
// https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-ip-mode:~:text=Specifying%20IPMode%20of%20load%20balancer%20status
182+
ServiceAnnotationIngressIpMode = "oci.oraclecloud.com/ingress-ip-mode"
178183
)
179184

180185
// NLB specific annotations
@@ -239,6 +244,9 @@ const (
239244

240245
// ServiceAnnotationNetworkLoadBalancerIsPpv2Enabled is a service annotation to enable/disable PPv2 feature for the listeners of this NLB.
241246
ServiceAnnotationNetworkLoadBalancerIsPpv2Enabled = "oci-network-load-balancer.oraclecloud.com/is-ppv2-enabled"
247+
248+
// ServiceAnnotationNetworkLoadBalancerExternalIpOnly is a service a boolean annotation to skip private ip when assigning to ingress resource for NLB service
249+
ServiceAnnotationNetworkLoadBalancerExternalIpOnly = "oci-network-load-balancer.oraclecloud.com/external-ip-only"
242250
)
243251

244252
const (
@@ -344,6 +352,7 @@ type LBSpec struct {
344352
FreeformTags map[string]string
345353
DefinedTags map[string]map[string]interface{}
346354
SystemTags map[string]map[string]interface{}
355+
ingressIpMode *v1.LoadBalancerIPMode
347356

348357
service *v1.Service
349358
nodes []*v1.Node
@@ -433,6 +442,11 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes []*v
433442
managedNsg.backendNsgId = backendNsgOcids
434443
}
435444

445+
ingressIpMode, err := getIngressIpMode(svc)
446+
if err != nil {
447+
return nil, err
448+
}
449+
436450
return &LBSpec{
437451
Type: lbType,
438452
Name: GetLoadBalancerName(svc),
@@ -457,6 +471,7 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes []*v
457471
FreeformTags: lbTags.FreeformTags,
458472
DefinedTags: lbTags.DefinedTags,
459473
SystemTags: getResourceTrackingSystemTagsFromConfig(logger, initialLBTags),
474+
ingressIpMode: ingressIpMode,
460475
}, nil
461476
}
462477

@@ -1570,3 +1585,53 @@ func isServiceDualStack(svc *v1.Service) bool {
15701585
}
15711586
return false
15721587
}
1588+
1589+
// getIngressIpMode reads ingress ipMode specified in the service annotation if exists
1590+
func getIngressIpMode(service *v1.Service) (*v1.LoadBalancerIPMode, error) {
1591+
var ipMode, exists = "", false
1592+
1593+
if ipMode, exists = service.Annotations[ServiceAnnotationIngressIpMode]; !exists {
1594+
return nil, nil
1595+
}
1596+
1597+
switch strings.ToLower(ipMode) {
1598+
case "proxy":
1599+
ipModeProxy := v1.LoadBalancerIPModeProxy
1600+
return &ipModeProxy, nil
1601+
case "vip":
1602+
ipModeProxy := v1.LoadBalancerIPModeVIP
1603+
return &ipModeProxy, nil
1604+
default:
1605+
return nil, errors.New("IpMode can only be set as Proxy or VIP")
1606+
}
1607+
}
1608+
1609+
// isSkipPrivateIP determines if skipPrivateIP annotation is set or not
1610+
func isSkipPrivateIP(svc *v1.Service) (bool, error) {
1611+
lbType := getLoadBalancerType(svc)
1612+
annotationValue := ""
1613+
annotationExists := false
1614+
annotationString := ""
1615+
annotationValue, annotationExists = svc.Annotations[ServiceAnnotationNetworkLoadBalancerExternalIpOnly]
1616+
if !annotationExists {
1617+
return false, nil
1618+
}
1619+
1620+
if lbType != NLB {
1621+
return false, nil
1622+
}
1623+
1624+
internal, err := isInternalLB(svc)
1625+
if err != nil {
1626+
return false, err
1627+
}
1628+
if internal {
1629+
return false, nil
1630+
}
1631+
1632+
skipPrivateIp, err := strconv.ParseBool(annotationValue)
1633+
if err != nil {
1634+
return false, errors.Wrap(err, fmt.Sprintf("invalid value: %s provided for annotation: %s", annotationValue, annotationString))
1635+
}
1636+
return skipPrivateIp, nil
1637+
}

pkg/cloudprovider/providers/oci/load_balancer_spec_test.go

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"context"
1919
"encoding/json"
2020
"fmt"
21-
"k8s.io/utils/pointer"
2221
"net/http"
2322
"reflect"
2423
"testing"
@@ -28,6 +27,7 @@ import (
2827
v1 "k8s.io/api/core/v1"
2928
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3029
"k8s.io/apimachinery/pkg/util/sets"
30+
"k8s.io/utils/pointer"
3131

3232
providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
3333
"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
@@ -11782,3 +11782,74 @@ func Test_getLoadBalancerSourceRanges(t *testing.T) {
1178211782
})
1178311783
}
1178411784
}
11785+
11786+
func TestIsSkipPrivateIP_NLB(t *testing.T) {
11787+
tests := []struct {
11788+
name string
11789+
svcAnnotations map[string]string
11790+
expected bool
11791+
wantErr bool
11792+
}{
11793+
{
11794+
name: "skip-private-ip-enabled",
11795+
svcAnnotations: map[string]string{
11796+
ServiceAnnotationLoadBalancerType: NLB,
11797+
ServiceAnnotationNetworkLoadBalancerExternalIpOnly: "true",
11798+
},
11799+
expected: true,
11800+
wantErr: false,
11801+
},
11802+
{
11803+
name: "skip-private-ip-disabled",
11804+
svcAnnotations: map[string]string{
11805+
ServiceAnnotationLoadBalancerType: NLB,
11806+
ServiceAnnotationNetworkLoadBalancerExternalIpOnly: "false",
11807+
},
11808+
expected: false,
11809+
wantErr: false,
11810+
},
11811+
{
11812+
name: "skip-private-ip-invalid-value",
11813+
svcAnnotations: map[string]string{
11814+
ServiceAnnotationLoadBalancerType: NLB,
11815+
ServiceAnnotationNetworkLoadBalancerExternalIpOnly: "invalid",
11816+
},
11817+
expected: false,
11818+
wantErr: true,
11819+
},
11820+
{
11821+
name: "skip-private-ip with internal loadbalancer",
11822+
svcAnnotations: map[string]string{
11823+
ServiceAnnotationLoadBalancerType: NLB,
11824+
ServiceAnnotationNetworkLoadBalancerInternal: "true",
11825+
ServiceAnnotationNetworkLoadBalancerExternalIpOnly: "true",
11826+
},
11827+
expected: false,
11828+
wantErr: false,
11829+
},
11830+
{
11831+
name: "no-skip-private-ip-annotation",
11832+
svcAnnotations: map[string]string{},
11833+
expected: false,
11834+
wantErr: false,
11835+
},
11836+
}
11837+
11838+
for _, tt := range tests {
11839+
t.Run(tt.name, func(t *testing.T) {
11840+
svc := &v1.Service{
11841+
ObjectMeta: metav1.ObjectMeta{
11842+
Annotations: tt.svcAnnotations,
11843+
},
11844+
}
11845+
got, err := isSkipPrivateIP(svc)
11846+
if (err != nil) != tt.wantErr {
11847+
t.Errorf("isSkipPrivateIP() error = %v, wantErr %v", err, tt.wantErr)
11848+
return
11849+
}
11850+
if got != tt.expected {
11851+
t.Errorf("isSkipPrivateIP() = %v, expected %v", got, tt.expected)
11852+
}
11853+
})
11854+
}
11855+
}

0 commit comments

Comments
 (0)