Skip to content

Commit

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

[release-1.28] 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 3826eb6 + c2e9f02 commit 80f3e46
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 @@ -402,11 +402,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 @@ -422,7 +439,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 @@ -603,22 +604,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 @@ -627,7 +631,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 @@ -642,7 +647,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 @@ -680,7 +686,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 @@ -706,7 +713,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 @@ -746,7 +754,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 @@ -773,7 +782,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 @@ -804,7 +814,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 @@ -829,7 +840,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 @@ -858,6 +870,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 @@ -868,7 +961,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 80f3e46

Please sign in to comment.