Skip to content

Commit

Permalink
skip invalid LoadBalancerSourceRanges when provisioning nsg rules and…
Browse files Browse the repository at this point in the history
… emit event
  • Loading branch information
jwtty authored and k8s-infra-cherrypick-robot committed Mar 20, 2024
1 parent cf6da5a commit 91451cf
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 117 deletions.
7 changes: 7 additions & 0 deletions pkg/provider/azure_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2856,6 +2856,13 @@ func (az *Cloud) reconcileSecurityGroup(
consts.ServiceAnnotationAllowedIPRanges, consts.ServiceAnnotationAllowedServiceTags,
))
}

if len(accessControl.InvalidRanges) > 0 {
az.Event(service, v1.EventTypeWarning, "InvalidConfiguration", fmt.Sprintf(
"Found invalid LoadBalancerSourceRanges %v, ignoring and adding a default DenyAll rule in security group.",
accessControl.InvalidRanges,
))
}
}

var (
Expand Down
21 changes: 12 additions & 9 deletions pkg/provider/loadbalancer/accesscontrol.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type AccessControl struct {
// immutable pre-compute states.
SourceRanges []netip.Prefix
AllowedIPRanges []netip.Prefix
InvalidRanges []string
AllowedServiceTags []string
securityRuleDestinationPortsByProtocol map[network.SecurityRuleProtocol][]int32
}
Expand Down Expand Up @@ -82,20 +83,17 @@ func NewAccessControl(svc *v1.Service, sg *network.SecurityGroup, opts ...Access
logger.Error(err, "Failed to initialize RuleHelper")
return nil, err
}
sourceRanges, err := SourceRanges(svc)
sourceRanges, invalidSourceRanges, err := SourceRanges(svc)
if err != nil && !options.SkipAnnotationValidation {
logger.Error(err, "Failed to parse SourceRange configuration")
return nil, err
}
allowedIPRanges, err := AllowedIPRanges(svc)
allowedIPRanges, invalidAllowedIPRanges, err := AllowedIPRanges(svc)
if err != nil && !options.SkipAnnotationValidation {
logger.Error(err, "Failed to parse AllowedIPRanges configuration")
return nil, err
}
allowedServiceTags, err := AllowedServiceTags(svc)
if err != nil && !options.SkipAnnotationValidation {
logger.Error(err, "Failed to parse AllowedServiceTags configuration")
return nil, err
}
securityRuleDestinationPortsByProtocol, err := securityRuleDestinationPortsByProtocol(svc)
if err != nil {
Expand All @@ -114,13 +112,14 @@ func NewAccessControl(svc *v1.Service, sg *network.SecurityGroup, opts ...Access
SourceRanges: sourceRanges,
AllowedIPRanges: allowedIPRanges,
AllowedServiceTags: allowedServiceTags,
InvalidRanges: append(invalidSourceRanges, invalidAllowedIPRanges...),
securityRuleDestinationPortsByProtocol: securityRuleDestinationPortsByProtocol,
}, nil
}

// IsAllowFromInternet returns true if the given service is allowed to be accessed from internet.
// To be specific,
// 1. For all types of LB, it returns false if the given service is specified with `service tags` or `not allowed all IP ranges`.
// 1. For all types of LB, it returns false if the given service is specified with `service tags` or `not allowed all IP ranges`, including invalid IP ranges.
// 2. For internal LB, it returns true iff the given service is explicitly specified with `allowed all IP ranges`. Refer: https://github.com/kubernetes-sigs/cloud-provider-azure/issues/698
func (ac *AccessControl) IsAllowFromInternet() bool {
if len(ac.AllowedServiceTags) > 0 {
Expand All @@ -132,6 +131,9 @@ func (ac *AccessControl) IsAllowFromInternet() bool {
if len(ac.AllowedIPRanges) > 0 && !iputil.IsPrefixesAllowAll(ac.AllowedIPRanges) {
return false
}
if len(ac.InvalidRanges) > 0 {
return false
}
if !IsInternal(ac.svc) {
return true
}
Expand All @@ -143,10 +145,11 @@ func (ac *AccessControl) IsAllowFromInternet() bool {
// By default, NSG allow traffic from the VNet.
func (ac *AccessControl) DenyAllExceptSourceRanges() bool {
var (
annotationEnabled = strings.EqualFold(ac.svc.Annotations[consts.ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges], "true")
sourceRangeSpecified = len(ac.SourceRanges) > 0 || len(ac.AllowedIPRanges) > 0
annotationEnabled = strings.EqualFold(ac.svc.Annotations[consts.ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges], "true")
sourceRangeSpecified = len(ac.SourceRanges) > 0 || len(ac.AllowedIPRanges) > 0
invalidRangesSpecified = len(ac.InvalidRanges) > 0
)
return annotationEnabled && sourceRangeSpecified
return (annotationEnabled && sourceRangeSpecified) || invalidRangesSpecified
}

// AllowedIPv4Ranges returns the IPv4 ranges that are allowed to access the LoadBalancer.
Expand Down
Loading

0 comments on commit 91451cf

Please sign in to comment.