Skip to content

Commit

Permalink
strict validations for nodeoutbound lb
Browse files Browse the repository at this point in the history
  • Loading branch information
@shysank
shysank committed Mar 24, 2021
1 parent cd82b38 commit 26c06df
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 179 deletions.
63 changes: 26 additions & 37 deletions api/v1alpha4/azurecluster_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,46 +160,35 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
}

lb := c.Spec.NetworkSpec.NodeOutboundLB
if lb.Type == "" {
lb.Type = Public
}
if lb.SKU == "" {
lb.SKU = SKUStandard
}

if lb.Name == "" {
lb.Name = c.ObjectMeta.Name
}
if len(lb.FrontendIPs) == 0 {

if lb.FrontendIPsCount == nil {
lb.FrontendIPsCount = pointer.Int32Ptr(1)
lb.Type = Public
lb.SKU = SKUStandard
lb.Name = c.ObjectMeta.Name

if lb.FrontendIPsCount == nil {
lb.FrontendIPsCount = pointer.Int32Ptr(1)
}

switch *lb.FrontendIPsCount {
case 0: // do nothing
case 1:
lb.FrontendIPs = []FrontendIP{
{
Name: generateFrontendIPConfigName(lb.Name),
PublicIP: &PublicIPSpec{
Name: generateNodeOutboundIPName(c.ObjectMeta.Name),
},
},
}

switch *lb.FrontendIPsCount {
case 0: // do nothing
case 1:
lb.FrontendIPs = []FrontendIP{
{
Name: generateFrontendIPConfigName(lb.Name),
PublicIP: &PublicIPSpec{
Name: generateNodeOutboundIPName(c.ObjectMeta.Name),
},
default:
for i := 0; i < int(*lb.FrontendIPsCount); i++ {
lb.FrontendIPs = append(lb.FrontendIPs, FrontendIP{
Name: withIndex(generateFrontendIPConfigName(lb.Name), i+1),
PublicIP: &PublicIPSpec{
Name: withIndex(generateNodeOutboundIPName(c.ObjectMeta.Name), i+1),
},
}
default:
for i := 0; i < int(*lb.FrontendIPsCount); i++ {
lb.FrontendIPs = append(lb.FrontendIPs, FrontendIP{
Name: withIndex(generateFrontendIPConfigName(lb.Name), i+1),
PublicIP: &PublicIPSpec{
Name: withIndex(generateNodeOutboundIPName(c.ObjectMeta.Name), i+1),
},
})
}

})
}
} else {
lb.FrontendIPsCount = pointer.Int32Ptr(int32(len(lb.FrontendIPs)))

}
}

Expand Down
70 changes: 1 addition & 69 deletions api/v1alpha4/azurecluster_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,7 @@ func TestVnetDefaults(t *testing.T) {
Type: Public,
},
NodeOutboundLB: &LoadBalancerSpec{
Name: "my-node-outbound-lb",
SKU: SKUStandard,
FrontendIPs: []FrontendIP{
{
Name: "ip-config",
PublicIP: &PublicIPSpec{
Name: "public-ip",
DNSName: "myfqdn.azure.com",
},
},
},
Type: Public,
FrontendIPsCount: pointer.Int32Ptr(1),
},
},
},
Expand Down Expand Up @@ -832,63 +821,6 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
},
},
},
{
name: "when frontend ips are configured",
cluster: &AzureCluster{
ObjectMeta: v1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: LoadBalancerSpec{Type: Public},
NodeOutboundLB: &LoadBalancerSpec{FrontendIPs: []FrontendIP{
{
Name: "fip-1",
PublicIP: &PublicIPSpec{
Name: "pip-1",
},
},
{
Name: "fip-2",
PublicIP: &PublicIPSpec{
Name: "pip-2",
},
},
}},
},
},
},
output: &AzureCluster{
ObjectMeta: v1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: LoadBalancerSpec{Type: Public},
NodeOutboundLB: &LoadBalancerSpec{
Name: "cluster-test",
Type: Public,
SKU: SKUStandard,
FrontendIPs: []FrontendIP{
{
Name: "fip-1",
PublicIP: &PublicIPSpec{
Name: "pip-1",
},
},
{
Name: "fip-2",
PublicIP: &PublicIPSpec{
Name: "pip-2",
},
},
},
FrontendIPsCount: pointer.Int32Ptr(2),
},
},
},
},
},
}

for _, c := range cases {
Expand Down
51 changes: 31 additions & 20 deletions api/v1alpha4/azurecluster_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,38 +283,49 @@ func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserv
return allErrs
}

// SKU should be Standard and is immutable.
if lb.SKU != SKUStandard {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("sku"), lb.SKU, []string{string(SKUStandard)}))
if old == nil && lb.ID != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("id"), "Node outbound load balancer ID is not allowed to be configured."))
}
if old != nil && old.SKU != "" && old.SKU != lb.SKU {
allErrs = append(allErrs, field.Invalid(fldPath.Child("sku"), lb.SKU, "Node outbound load balancer SKU should not be modified after AzureCluster creation."))
if old != nil && old.ID != lb.ID {
allErrs = append(allErrs, field.Invalid(fldPath.Child("id"), lb.ID, "Node outbound load balancer ID should not be modified after AzureCluster creation."))
}

// Type should be Public.
if lb.Type != Public {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("type"), lb.Type, []string{string(Public)}))
if old == nil && lb.Name != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "Node outbound load balancer Name is not allowed to be configured."))
}
if old != nil && old.Type != "" && old.Type != lb.Type {
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), lb.Type, "Node outbound load balancer type should not be modified after AzureCluster creation."))
if old != nil && old.Name != lb.Name {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), lb.Name, "Node outbound load balancer Name should not be modified after AzureCluster creation."))
}

// Name should be valid.
if err := validateLoadBalancerName(lb.Name, fldPath.Child("name")); err != nil {
allErrs = append(allErrs, err)
if old == nil && lb.SKU != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("sku"), "Node outbound load balancer SKU is not allowed to be configured."))
}
if old != nil && old.Name != "" && old.Name != lb.Name {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), lb.Type, "Node outbound load balancer name should not be modified after AzureCluster creation."))
if old != nil && old.SKU != lb.SKU {
allErrs = append(allErrs, field.Invalid(fldPath.Child("sku"), lb.SKU, "Node outbound load balancer SKU should not be modified after AzureCluster creation."))
}

for i, frontEndIps := range lb.FrontendIPs {

if frontEndIps.PrivateIPAddress != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(i).Child("privateIP"),
"Public Load Balancers cannot have a Private IP"))
if old == nil && len(lb.FrontendIPs) > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPs"), "Node outbound load balancer FrontendIPs is not allowed to be configured."))
}
if old != nil && len(lb.FrontendIPs) != len(old.FrontendIPs) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPs"), "Node outbound load balancer FrontendIPs is not allowed to be modified after AzureCluster creation."))
}
if old != nil && len(lb.FrontendIPs) == len(old.FrontendIPs) {
for i, frontEndIps := range lb.FrontendIPs {
if frontEndIps != old.FrontendIPs[i] {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPs").Index(i), frontEndIps,
"Node outbound load balancer FrontendIPs is not allowed to be modified after AzureCluster creation."))
}
}
}

if old == nil && lb.Type != "" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "Node outbound load balancer Type is not allowed to be configured."))
}
if old != nil && old.Type != lb.Type {
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), lb.SKU, "Node outbound load balancer Type should not be modified after AzureCluster creation."))
}

if lb.FrontendIPsCount != nil && *lb.FrontendIPsCount > MaxLoadBalancerOutboundIPs {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPsCount"), *lb.FrontendIPsCount,
fmt.Sprintf("Max front end ips allowed is %d", MaxLoadBalancerOutboundIPs)))
Expand Down
Loading

0 comments on commit 26c06df

Please sign in to comment.