Skip to content

Commit

Permalink
Merge pull request #4539 from k8s-infra-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history 
…4537-to-release-1.25

[release-1.25] fix: truncate pls frontendIPConfig name if it's too long
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 1, 2023
2 parents be52e60 + 1b83c4e commit b3c2687
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 13 deletions.
25 changes: 23 additions & 2 deletions pkg/provider/azure_privatelinkservice.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,28 @@ func (az *Cloud) reconcilePLSIpConfigs(
}

if changed {
getFrontendIPConfigName := func(suffix string) (string, error) {
// frontend ipConfig name length cannot exceed 80
maxPrefixLen := consts.FrontendIPConfigNameMaxLength - len(suffix)
if maxPrefixLen <= 0 {
return "", fmt.Errorf("reconcilePLSIpConfigs: frontend ipConfig suffix %s is too long (not likely to happen)", suffix)
}
prefix := fmt.Sprintf("%s-%s", pointer.StringDeref(subnet.Name, ""), pointer.StringDeref(existingPLS.Name, ""))
if len(prefix) > maxPrefixLen {
prefix = prefix[:maxPrefixLen]
}
return prefix + suffix, nil
}

ipConfigs := []network.PrivateLinkServiceIPConfiguration{}
for k := range staticIps {
ip := k
isPrimary := strings.EqualFold(ip, primaryIP)
configName := fmt.Sprintf("%s-%s-static-%s", pointer.StringDeref(subnet.Name, ""), pointer.StringDeref(existingPLS.Name, ""), ip)
suffix := fmt.Sprintf("-static-%s", ip)
configName, err := getFrontendIPConfigName(suffix)
if err != nil {
return false, err
}
ipConfigs = append(ipConfigs, network.PrivateLinkServiceIPConfiguration{
Name: &configName,
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
Expand All @@ -400,7 +417,11 @@ func (az *Cloud) reconcilePLSIpConfigs(
}
for i := 0; i < int(ipConfigCount)-len(staticIps); i++ {
isPrimary := primaryIP == "" && i == 0
configName := fmt.Sprintf("%s-%s-dynamic-%d", pointer.StringDeref(subnet.Name, ""), pointer.StringDeref(existingPLS.Name, ""), i)
suffix := fmt.Sprintf("-dynamic-%d", i)
configName, err := getFrontendIPConfigName(suffix)
if err != nil {
return false, err
}
ipConfigs = append(ipConfigs, network.PrivateLinkServiceIPConfiguration{
Name: &configName,
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
Expand Down
115 changes: 104 additions & 11 deletions pkg/provider/azure_privatelinkservice_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package provider

import (
"net/http"
"strings"
"testing"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
Expand Down Expand Up @@ -592,22 +593,25 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
for i, test := range []struct {
desc string
annotations map[string]string
plsName string
existingIPConfigs *[]network.PrivateLinkServiceIPConfiguration
expectedIPConfigs *[]network.PrivateLinkServiceIPConfiguration
getSubnetError *retry.Error
expectedChanged bool
expectedErr bool
}{
{
desc: "reconcilePLSIpConfigs should report error when subnet specified by service does not exist",
desc: "reconcilePLSIpConfigs should report error when subnet specified by service does not exist",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationSubnet: "subnet",
},
getSubnetError: &retry.Error{HTTPStatusCode: http.StatusNotFound},
expectedErr: true,
},
{
desc: "reconcilePLSIpConfigs should report error when ip count specified is fewer than number of static IPs",
desc: "reconcilePLSIpConfigs should report error when ip count specified is fewer than number of static IPs",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationSubnet: "subnet",
consts.ServiceAnnotationPLSIpConfigurationIPAddressCount: "1",
Expand All @@ -616,7 +620,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedErr: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if its ipConfig is nil",
desc: "reconcilePLSIpConfigs should change existingPLS if its ipConfig is nil",
plsName: "testpls",
expectedIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-testpls-dynamic-0"),
Expand All @@ -631,7 +636,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if its ipConfig count is different",
desc: "reconcilePLSIpConfigs should change existingPLS if its ipConfig count is different",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddressCount: "2",
},
Expand Down Expand Up @@ -669,7 +675,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if its subnetID is different",
desc: "reconcilePLSIpConfigs should change existingPLS if its subnetID is different",
plsName: "testpls",
existingIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-testpls-dynamic-0"),
Expand All @@ -695,7 +702,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if ip allocation type is changed from dynamic to static",
desc: "reconcilePLSIpConfigs should change existingPLS if ip allocation type is changed from dynamic to static",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddressCount: "2",
consts.ServiceAnnotationPLSIpConfigurationIPAddress: "10.2.0.4",
Expand Down Expand Up @@ -735,7 +743,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if ip allocation type is changed from static to dynamic",
desc: "reconcilePLSIpConfigs should change existingPLS if ip allocation type is changed from static to dynamic",
plsName: "testpls",
existingIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-testpls-static-10.2.0.4"),
Expand All @@ -762,7 +771,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should change existingPLS if static ip is changed",
desc: "reconcilePLSIpConfigs should change existingPLS if static ip is changed",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddress: "10.2.0.5",
},
Expand Down Expand Up @@ -793,7 +803,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should not change existingPLS if ip allocation type is dynamic only",
desc: "reconcilePLSIpConfigs should not change existingPLS if ip allocation type is dynamic only",
plsName: "testpls",
existingIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-testpls-dynamic-0"),
Expand All @@ -818,7 +829,8 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
},
},
{
desc: "reconcilePLSIpConfigs should not change existingPLS if static ip is exactly same",
desc: "reconcilePLSIpConfigs should not change existingPLS if static ip is exactly same",
plsName: "testpls",
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddress: "10.2.0.5",
},
Expand Down Expand Up @@ -847,6 +859,87 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
},
},
},
{
desc: "reconcilePLSIpConfigs should truncate frontendIPConfig name if it's too long",
plsName: strings.Repeat("12345678", 10),
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddress: "10.2.0.5",
consts.ServiceAnnotationPLSIpConfigurationIPAddressCount: "2",
},
existingIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{},
expectedIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1" + "-static-10.2.0.5"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Static,
PrivateIPAddress: pointer.String("10.2.0.5"),
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(true),
PrivateIPAddressVersion: network.IPv4,
},
},
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1234567" + "-dynamic-0"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Dynamic,
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(false),
PrivateIPAddressVersion: network.IPv4,
},
},
},
expectedChanged: true,
},
{
desc: "reconcilePLSIpConfigs should not modify existingPLS in name truncation case",
plsName: strings.Repeat("12345678", 10),
annotations: map[string]string{
consts.ServiceAnnotationPLSIpConfigurationIPAddress: "10.2.0.5",
consts.ServiceAnnotationPLSIpConfigurationIPAddressCount: "2",
},
existingIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1" + "-static-10.2.0.5"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Static,
PrivateIPAddress: pointer.String("10.2.0.5"),
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(true),
PrivateIPAddressVersion: network.IPv4,
},
},
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1234567" + "-dynamic-0"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Dynamic,
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(false),
PrivateIPAddressVersion: network.IPv4,
},
},
},
expectedIPConfigs: &[]network.PrivateLinkServiceIPConfiguration{
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1" + "-static-10.2.0.5"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Static,
PrivateIPAddress: pointer.String("10.2.0.5"),
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(true),
PrivateIPAddressVersion: network.IPv4,
},
},
{
Name: pointer.String("subnet-" + strings.Repeat("12345678", 7) + "1234567" + "-dynamic-0"),
PrivateLinkServiceIPConfigurationProperties: &network.PrivateLinkServiceIPConfigurationProperties{
PrivateIPAllocationMethod: network.Dynamic,
Subnet: &network.Subnet{ID: pointer.String("subnetID")},
Primary: pointer.Bool(false),
PrivateIPAddressVersion: network.IPv4,
},
},
},
},
} {
cloud := GetTestCloud(ctrl)
service := &v1.Service{
Expand All @@ -857,7 +950,7 @@ func TestReconcilePLSIpConfigs(t *testing.T) {
},
}
pls := &network.PrivateLinkService{
Name: pointer.String("testpls"),
Name: pointer.String(test.plsName),
PrivateLinkServiceProperties: &network.PrivateLinkServiceProperties{
IPConfigurations: test.existingIPConfigs,
},
Expand Down

0 comments on commit b3c2687

Please sign in to comment.