Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make Private IP of the internal LB of the API Server configurable #5274

Merged
merged 1 commit into from
Nov 14, 2024
+248 −79
Merged

make Private IP of the internal LB of the API Server configurable #5274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions api/v1beta1/azurecluster_default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,31 @@
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"

Check warning on line 242 in api/v1beta1/azurecluster_default.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/azurecluster_default.go#L241-L242 

Added lines #L241 - L242 were not covered by tests
}
privateIPFound = true
break

Check warning on line 245 in api/v1beta1/azurecluster_default.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/azurecluster_default.go#L244-L245 

Added lines #L244 - L245 were not covered by tests
}
}

// 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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,33 +400,58 @@
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 {
nawazkh marked this conversation as resolved.
Show resolved Hide resolved
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."))

Check warning on line 454 in api/v1beta1/azurecluster_validation.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/azurecluster_validation.go#L454 

Added line #L454 was not covered by tests
}
}
}
Expand Down
95 changes: 89 additions & 6 deletions api/v1beta1/azurecluster_validation_test.go
View file
Open in desktop
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",
nawazkh marked this conversation as resolved.
Show resolved Hide resolved
},
},
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
Loading