Skip to content

Commit

Permalink
Merge pull request #5274 from nawazkh/make_private_lb_ip_configurable
Browse files Browse the repository at this point in the history 
make Private IP of the internal LB of the API Server configurable
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Nov 14, 2024
2 parents 630e39c + 2a44ea8 commit 4ff53b9
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 79 deletions.
30 changes: 22 additions & 8 deletions api/v1beta1/azurecluster_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,31 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
if lb.Name == "" {
lb.Name = generateInternalLBName(c.ObjectMeta.Name)
}
if len(lb.FrontendIPs) == 0 {
lb.FrontendIPs = []FrontendIP{
{
Name: generateFrontendIPConfigName(lb.Name),
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
}

// create default private IP if not set
privateIPFound := false
for i := range lb.FrontendIPs {
if lb.FrontendIPs[i].FrontendIPClass.PrivateIPAddress != "" {
if lb.FrontendIPs[i].Name == "" {
lb.FrontendIPs[i].Name = generateFrontendIPConfigName(lb.Name) + "-internal-ip"
}
privateIPFound = true
break
}
}

// if no private IP found, create a default one
if !privateIPFound {
privateIP := FrontendIP{
Name: generateFrontendIPConfigName(lb.Name) + "-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
}
lb.FrontendIPs = append(lb.FrontendIPs, privateIP)
}

c.SetAPIServerLBBackendPoolNameDefault()
}

Expand Down
21 changes: 17 additions & 4 deletions api/v1beta1/azurecluster_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ func TestVnetDefaults(t *testing.T) {
Subnets: Subnets{
{
SubnetClassSpec: SubnetClassSpec{
Role: SubnetControlPlane,
Name: "control-plane-subnet",
Role: SubnetControlPlane,
Name: "control-plane-subnet",
CIDRBlocks: []string{DefaultControlPlaneSubnetCIDR},
},

SecurityGroup: SecurityGroup{},
Expand All @@ -132,6 +133,12 @@ func TestVnetDefaults(t *testing.T) {
DNSName: "myfqdn.azure.com",
},
},
{
Name: "ip-config-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Expand Down Expand Up @@ -1237,6 +1244,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
DNSName: "",
},
},
{
Name: "cluster-test-public-lb-frontEnd-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
BackendPool: BackendPool{
Name: "cluster-test-public-lb-backendPool",
Expand Down Expand Up @@ -1276,7 +1289,7 @@ func TestAPIServerLBDefaults(t *testing.T) {
APIServerLB: LoadBalancerSpec{
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-internal-lb-frontEnd",
Name: "cluster-test-internal-lb-frontEnd-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
Expand Down Expand Up @@ -1324,7 +1337,7 @@ func TestAPIServerLBDefaults(t *testing.T) {
APIServerLB: LoadBalancerSpec{
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-internal-lb-frontEnd",
Name: "cluster-test-internal-lb-frontEnd-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
Expand Down
63 changes: 44 additions & 19 deletions api/v1beta1/azurecluster_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,33 +400,58 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation."))
}

// There should only be one IP config.
if len(lb.FrontendIPs) != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
publicIPCount := 0
privateIPCount := 0
newPrivateIP := ""
for i := range lb.FrontendIPs {
if lb.FrontendIPs[i].PublicIP != nil {
publicIPCount++
}
if lb.FrontendIPs[i].PrivateIPAddress != "" {
privateIPCount++
newPrivateIP = lb.FrontendIPs[i].PrivateIPAddress
}
}

if lb.Type == Public {
// public IP count should be 1 for public LB.
if publicIPCount != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
"API Server Load balancer should have 1 Frontend IP"))
}
}

// if Internal, IP config should not have a public IP.
if lb.Type == Internal {
if publicIPCount != 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
"API Server's associated internal load balancer cannot have a Public IP"))
}
}

// private IP count should be 1 for public LB.
if privateIPCount != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
"API Server Load balancer should have 1 Frontend IP"))
"API Server Load balancer should have 1 private IP"))
} else {
// if Internal, IP config should not have a public IP.
if lb.Type == Internal {
if lb.FrontendIPs[0].PublicIP != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
"Internal Load Balancers cannot have a Public IP"))
}
if lb.FrontendIPs[0].PrivateIPAddress != "" {
if err := validateInternalLBIPAddress(lb.FrontendIPs[0].PrivateIPAddress, cidrs,
for i := range lb.FrontendIPs {
if lb.FrontendIPs[i].PrivateIPAddress != "" {
if err := validateInternalLBIPAddress(lb.FrontendIPs[i].PrivateIPAddress, cidrs,
fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
allErrs = append(allErrs, err)
}
if len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
}
}
}

// if Public, IP config should not have a private IP.
if lb.Type == Public {
if lb.FrontendIPs[0].PrivateIPAddress != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
"Public Load Balancers cannot have a Private IP"))
if len(old.FrontendIPs) != 0 {
oldPrivateIP := ""
for i := range old.FrontendIPs {
if old.FrontendIPs[i].PrivateIPAddress != "" {
oldPrivateIP = old.FrontendIPs[i].PrivateIPAddress
}
}
if newPrivateIP != oldPrivateIP {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
}
}
}
Expand Down
95 changes: 89 additions & 6 deletions api/v1beta1/azurecluster_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ func TestValidateAPIServerLB(t *testing.T) {
{
name: "too many IP configs",
lb: LoadBalancerSpec{
Name: "my-valid-lb",
FrontendIPs: []FrontendIP{
{
Name: "ip-1",
Expand All @@ -899,6 +900,10 @@ func TestValidateAPIServerLB(t *testing.T) {
Name: "ip-2",
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Public,
SKU: SKUStandard,
},
},
wantErr: true,
expectedErr: field.Error{
Expand All @@ -916,26 +921,80 @@ func TestValidateAPIServerLB(t *testing.T) {
},
},
{
name: "public LB with private IP",
name: "too many private IP configs",
lb: LoadBalancerSpec{
Name: "my-valid-lb",
FrontendIPs: []FrontendIP{
{
Name: "ip-1",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.4",
PrivateIPAddress: "10.0.0.100",
},
},
{
Name: "ip-2",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.200",
},
},
{
Name: "ip-3",
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Public,
SKU: SKUStandard,
},
},
wantErr: true,
expectedErr: field.Error{
Type: "FieldValueForbidden",
Field: "apiServerLB.frontendIPConfigs[0].privateIP",
Detail: "Public Load Balancers cannot have a Private IP",
Type: "FieldValueInvalid",
Field: "apiServerLB.frontendIPConfigs",
BadValue: []FrontendIP{
{
Name: "ip-1",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.100",
},
},
{
Name: "ip-2",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.200",
},
},
{
Name: "ip-3",
},
},
Detail: "API Server Load balancer should have 1 private IP",
},
},
{
name: "public LB with private IP",
cpCIDRS: []string{"10.0.0.0/24"},
lb: LoadBalancerSpec{
Name: "my-valid-lb",
FrontendIPs: []FrontendIP{
{
Name: "ip-1",
PublicIP: &PublicIPSpec{
Name: "my-valid-ip-name",
},
},
{
Name: "ip-1",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.4",
},
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Public,
SKU: SKUStandard,
},
},
wantErr: false,
},
{
name: "internal LB with public IP",
Expand All @@ -956,7 +1015,7 @@ func TestValidateAPIServerLB(t *testing.T) {
expectedErr: field.Error{
Type: "FieldValueForbidden",
Field: "apiServerLB.frontendIPConfigs[0].publicIP",
Detail: "Internal Load Balancers cannot have a Public IP",
Detail: "API Server's associated internal load balancer cannot have a Public IP",
},
},
{
Expand Down Expand Up @@ -1483,12 +1542,18 @@ func createClusterNetworkSpec() NetworkSpec {
Vnet: VnetSpec{
ResourceGroup: "custom-vnet",
Name: "my-vnet",
VnetClassSpec: VnetClassSpec{
CIDRBlocks: []string{DefaultVnetCIDR},
},
},
Subnets: Subnets{
{
SubnetClassSpec: SubnetClassSpec{
Role: "cluster",
Name: "cluster-subnet",
CIDRBlocks: []string{
DefaultClusterSubnetCIDR,
},
},
},
},
Expand All @@ -1502,12 +1567,18 @@ func createValidNetworkSpecWithClusterSubnet() NetworkSpec {
Vnet: VnetSpec{
ResourceGroup: "custom-vnet",
Name: "my-vnet",
VnetClassSpec: VnetClassSpec{
CIDRBlocks: []string{DefaultVnetCIDR},
},
},
Subnets: Subnets{
{
SubnetClassSpec: SubnetClassSpec{
Role: "cluster",
Name: "cluster-subnet",
CIDRBlocks: []string{
DefaultClusterSubnetCIDR,
},
},
},
},
Expand All @@ -1521,6 +1592,9 @@ func createValidNetworkSpec() NetworkSpec {
Vnet: VnetSpec{
ResourceGroup: "custom-vnet",
Name: "my-vnet",
VnetClassSpec: VnetClassSpec{
CIDRBlocks: []string{DefaultVnetCIDR},
},
},
Subnets: createValidSubnets(),
APIServerLB: createValidAPIServerLB(),
Expand All @@ -1534,6 +1608,9 @@ func createValidSubnets() Subnets {
SubnetClassSpec: SubnetClassSpec{
Role: "control-plane",
Name: "control-plane-subnet",
CIDRBlocks: []string{
DefaultControlPlaneSubnetCIDR,
},
},
},
{
Expand Down Expand Up @@ -1566,6 +1643,12 @@ func createValidAPIServerLB() LoadBalancerSpec {
DNSName: "myfqdn.azure.com",
},
},
{
Name: "ip-config-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Expand Down
Loading

0 comments on commit 4ff53b9

Please sign in to comment.