Skip to content

Commit 5c6909c

Browse files
nilo19nilo19
authored
Merge pull request #9597 from nilo19/fix/cherry-pick-9596-1-33
[release-1.33] fix: IPv6 Pointer Comparison Bug
2 parents 56b4cb9 + 64a9c1c commit 5c6909c

File tree

4 files changed

+163
-6
lines changed

4 files changed

+163
-6
lines changed

‎pkg/provider/azure_loadbalancer.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,7 +1703,7 @@ func (az *Cloud) findFrontendIPConfigsOfService(
17031703
var fipIsIPv6 bool
17041704
var err error
17051705
if fipIPVersion != nil {
1706-
fipIsIPv6 = fipIPVersion == to.Ptr(armnetwork.IPVersionIPv6)
1706+
fipIsIPv6 = *fipIPVersion == armnetwork.IPVersionIPv6
17071707
} else {
17081708
if fipIsIPv6, err = az.isFIPIPv6(service, config); err != nil {
17091709
return nil, err
@@ -1902,7 +1902,7 @@ func (az *Cloud) reconcileLoadBalancer(ctx context.Context, clusterName string,
19021902
var err error
19031903
_, _, fipIPVersion := az.serviceOwnsFrontendIP(ctx, ownedFIPConfig, service)
19041904
if fipIPVersion != nil {
1905-
isIPv6 = fipIPVersion == to.Ptr(armnetwork.IPVersionIPv6)
1905+
isIPv6 = *fipIPVersion == armnetwork.IPVersionIPv6
19061906
} else {
19071907
if isIPv6, err = az.isFIPIPv6(service, ownedFIPConfig); err != nil {
19081908
return nil, false, err
@@ -2608,7 +2608,7 @@ func (az *Cloud) reconcileFrontendIPConfigs(
26082608
var isIPv6 bool
26092609
var err error
26102610
if fipIPVersion != nil {
2611-
isIPv6 = fipIPVersion == to.Ptr(armnetwork.IPVersionIPv6)
2611+
isIPv6 = *fipIPVersion == armnetwork.IPVersionIPv6
26122612
} else {
26132613
if isIPv6, err = az.isFIPIPv6(service, config); err != nil {
26142614
return nil, toDeleteConfigs, false, err

‎pkg/provider/azure_loadbalancer_test.go‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2453,6 +2453,8 @@ func TestDeterminePublicIPName(t *testing.T) {
24532453
expectedPIPName string
24542454
expectedError bool
24552455
isIPv6 bool
2456+
serviceIPv6 bool
2457+
annotations map[string]string
24562458
}{
24572459
{
24582460
desc: "determinePublicIpName shall get public IP from az.getPublicIPName if no specific " +
@@ -2481,12 +2483,22 @@ func TestDeterminePublicIPName(t *testing.T) {
24812483
expectedPIPName: "pipName",
24822484
expectedError: false,
24832485
},
2486+
{
2487+
desc: "determinePublicIpName shall use IPv6 pip annotation for IPv6 single stack service",
2488+
annotations: map[string]string{
2489+
consts.ServiceAnnotationPIPNameDualStack[true]: "service-lb-public-IP3dbe-v6",
2490+
},
2491+
expectedPIPName: "service-lb-public-IP3dbe-v6",
2492+
isIPv6: true,
2493+
serviceIPv6: true,
2494+
expectedError: false,
2495+
},
24842496
}
24852497

24862498
for _, test := range testCases {
24872499
t.Run(test.desc, func(t *testing.T) {
24882500
az := GetTestCloud(ctrl)
2489-
service := getTestService("test1", v1.ProtocolTCP, nil, false, 80)
2501+
service := getTestService("test1", v1.ProtocolTCP, test.annotations, test.serviceIPv6, 80)
24902502
setServiceLoadBalancerIP(&service, test.loadBalancerIP)
24912503

24922504
mockPIPsClient := az.NetworkClientFactory.GetPublicIPAddressClient().(*mock_publicipaddressclient.MockInterface)
@@ -8517,6 +8529,93 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
85178529
}
85188530
}
85198531

8532+
func TestFindFrontendIPConfigsOfService(t *testing.T) {
8533+
ctrl := gomock.NewController(t)
8534+
defer ctrl.Finish()
8535+
8536+
testCases := []struct {
8537+
desc string
8538+
existingPIPs []*armnetwork.PublicIPAddress
8539+
fip *armnetwork.FrontendIPConfiguration
8540+
service *v1.Service
8541+
isIPv6 bool
8542+
}{
8543+
{
8544+
desc: "config works for ipv6 service",
8545+
existingPIPs: []*armnetwork.PublicIPAddress{
8546+
{
8547+
Name: ptr.To("pip1"),
8548+
ID: ptr.To("pip1"),
8549+
Properties: &armnetwork.PublicIPAddressPropertiesFormat{
8550+
IPAddress: ptr.To("fd00::eef0"),
8551+
PublicIPAddressVersion: to.Ptr(armnetwork.IPVersionIPv6),
8552+
},
8553+
},
8554+
},
8555+
fip: &armnetwork.FrontendIPConfiguration{
8556+
Name: ptr.To("auid"),
8557+
Properties: &armnetwork.FrontendIPConfigurationPropertiesFormat{
8558+
PublicIPAddress: &armnetwork.PublicIPAddress{
8559+
ID: ptr.To("pip1"),
8560+
},
8561+
},
8562+
},
8563+
service: &v1.Service{
8564+
ObjectMeta: metav1.ObjectMeta{
8565+
UID: types.UID("secondary"),
8566+
Annotations: map[string]string{consts.ServiceAnnotationPIPNameDualStack[false]: "pip1"},
8567+
},
8568+
},
8569+
isIPv6: true,
8570+
},
8571+
{
8572+
desc: "config works for ipv4 service",
8573+
existingPIPs: []*armnetwork.PublicIPAddress{
8574+
{
8575+
Name: ptr.To("pip1"),
8576+
ID: ptr.To("pip1"),
8577+
Properties: &armnetwork.PublicIPAddressPropertiesFormat{
8578+
IPAddress: ptr.To("4.3.2.1"),
8579+
PublicIPAddressVersion: to.Ptr(armnetwork.IPVersionIPv4),
8580+
},
8581+
},
8582+
},
8583+
fip: &armnetwork.FrontendIPConfiguration{
8584+
Name: ptr.To("auid"),
8585+
Properties: &armnetwork.FrontendIPConfigurationPropertiesFormat{
8586+
PublicIPAddress: &armnetwork.PublicIPAddress{
8587+
ID: ptr.To("pip1"),
8588+
},
8589+
},
8590+
},
8591+
service: &v1.Service{
8592+
ObjectMeta: metav1.ObjectMeta{
8593+
UID: types.UID("secondary"),
8594+
Annotations: map[string]string{consts.ServiceAnnotationPIPNameDualStack[false]: "pip1"},
8595+
},
8596+
},
8597+
},
8598+
}
8599+
8600+
for _, test := range testCases {
8601+
test := test
8602+
t.Run(test.desc, func(t *testing.T) {
8603+
cloud := GetTestCloud(ctrl)
8604+
if test.existingPIPs != nil {
8605+
mockPIPsClient := cloud.NetworkClientFactory.GetPublicIPAddressClient().(*mock_publicipaddressclient.MockInterface)
8606+
mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(test.existingPIPs, nil).MaxTimes(2)
8607+
}
8608+
configs, err := cloud.findFrontendIPConfigsOfService(context.TODO(), []*armnetwork.FrontendIPConfiguration{test.fip}, test.service)
8609+
if err != nil {
8610+
t.Fatalf("unexpected error: %v", err)
8611+
}
8612+
assert.Equal(t, 1, len(configs))
8613+
assert.NotNil(t, configs[test.isIPv6])
8614+
assert.Equal(t, test.fip, configs[test.isIPv6])
8615+
})
8616+
}
8617+
}
8618+
85208619
func TestReconcileMultipleStandardLoadBalancerNodes(t *testing.T) {
85218620
ctrl := gomock.NewController(t)
85228621
defer ctrl.Finish()

‎pkg/provider/azure_utils.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,12 @@ func getServicePIPName(service *v1.Service, isIPv6 bool) string {
377377
}
378378

379379
if !isServiceDualStack(service) {
380+
v4Enabled, v6Enabled := getIPFamiliesEnabled(service)
381+
if isIPv6 && v6Enabled && !v4Enabled {
382+
if name := service.Annotations[consts.ServiceAnnotationPIPNameDualStack[true]]; name != "" {
383+
return name
384+
}
385+
}
380386
return service.Annotations[consts.ServiceAnnotationPIPNameDualStack[false]]
381387
}
382388

@@ -397,6 +403,12 @@ func getServicePIPPrefixID(service *v1.Service, isIPv6 bool) string {
397403
}
398404

399405
if !isServiceDualStack(service) {
406+
v4Enabled, v6Enabled := getIPFamiliesEnabled(service)
407+
if isIPv6 && v6Enabled && !v4Enabled {
408+
if id := service.Annotations[consts.ServiceAnnotationPIPPrefixIDDualStack[true]]; id != "" {
409+
return id
410+
}
411+
}
400412
return service.Annotations[consts.ServiceAnnotationPIPPrefixIDDualStack[false]]
401413
}
402414

‎pkg/provider/azure_utils_test.go‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ func TestGetServicePIPName(t *testing.T) {
794794
&v1.Service{
795795
ObjectMeta: metav1.ObjectMeta{
796796
Annotations: map[string]string{
797-
consts.ServiceAnnotationPIPNameDualStack[false]: "pip-name-ipv6",
797+
consts.ServiceAnnotationPIPNameDualStack[true]: "pip-name-ipv6",
798798
},
799799
},
800800
Spec: v1.ServiceSpec{
@@ -836,6 +836,21 @@ func TestGetServicePIPName(t *testing.T) {
836836
true,
837837
"pip-name-ipv6",
838838
},
839+
{
840+
"From ServiceAnnotationPIPName IPv6 single stack fallback",
841+
&v1.Service{
842+
ObjectMeta: metav1.ObjectMeta{
843+
Annotations: map[string]string{
844+
consts.ServiceAnnotationPIPNameDualStack[false]: "pip-name-ipv6",
845+
},
846+
},
847+
Spec: v1.ServiceSpec{
848+
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
849+
},
850+
},
851+
true,
852+
"pip-name-ipv6",
853+
},
839854
}
840855
for _, tc := range testcases {
841856
t.Run(tc.desc, func(t *testing.T) {
@@ -872,7 +887,7 @@ func TestGetServicePIPPrefixID(t *testing.T) {
872887
&v1.Service{
873888
ObjectMeta: metav1.ObjectMeta{
874889
Annotations: map[string]string{
875-
consts.ServiceAnnotationPIPPrefixIDDualStack[false]: "pip-prefix-id-ipv6",
890+
consts.ServiceAnnotationPIPPrefixIDDualStack[true]: "pip-prefix-id-ipv6",
876891
},
877892
},
878893
Spec: v1.ServiceSpec{
@@ -914,6 +929,21 @@ func TestGetServicePIPPrefixID(t *testing.T) {
914929
true,
915930
"pip-prefix-id-ipv6",
916931
},
932+
{
933+
"From ServiceAnnotationPIPPrefixIDDualStack IPv6 single stack fallback",
934+
&v1.Service{
935+
ObjectMeta: metav1.ObjectMeta{
936+
Annotations: map[string]string{
937+
consts.ServiceAnnotationPIPPrefixIDDualStack[false]: "pip-prefix-id-ipv6",
938+
},
939+
},
940+
Spec: v1.ServiceSpec{
941+
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
942+
},
943+
},
944+
true,
945+
"pip-prefix-id-ipv6",
946+
},
917947
}
918948
for _, tc := range testcases {
919949
t.Run(tc.desc, func(t *testing.T) {
@@ -923,6 +953,22 @@ func TestGetServicePIPPrefixID(t *testing.T) {
923953
}
924954
}
925955

956+
func TestGetServicePIPNames(t *testing.T) {
957+
svc := &v1.Service{
958+
ObjectMeta: metav1.ObjectMeta{
959+
Annotations: map[string]string{
960+
consts.ServiceAnnotationPIPNameDualStack[true]: "pip-name-ipv6",
961+
},
962+
},
963+
Spec: v1.ServiceSpec{
964+
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
965+
},
966+
}
967+
968+
names := getServicePIPNames(svc)
969+
assert.Equal(t, []string{"", "pip-name-ipv6"}, names)
970+
}
971+
926972
func TestGetResourceByIPFamily(t *testing.T) {
927973
testcases := []struct {
928974
desc string

0 commit comments

Comments
 (0)