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.

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

Add referencing existing security groups for inbound traffic #3829

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions docs/guide/ingress/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,24 @@ Access control for LoadBalancer can be controlled with following annotations:
```
alb.ingress.kubernetes.io/inbound-cidrs: 10.0.0.0/24
```
- <a name="inbound-security-groups">`alb.ingress.kubernetes.io/inbound-security-groups`</a> specifies the SecurtityGroups that are allowed to access LoadBalancer.

!!!note "Merge Behavior"
`inbound-security-groups` is merged across all Ingresses in IngressGroup, but is exclusive per listen-port.

- the `inbound-security-groups` will only impact the ports defined for that Ingress.
- if same listen-port is defined by multiple Ingress within IngressGroup, `inbound-security-groups` should only be defined on one of the Ingress.

!!!warning ""
this annotation will be ignored if `alb.ingress.kubernetes.io/security-groups` is specified.

!!!tip ""
Both name or ID of securityGroups are supported. Name matches a `Name` tag, not the `groupName` attribute.

!!!example
```
alb.ingress.kubernetes.io/inbound-security-groups: sg-xxxx, nameOfSg1, nameOfSg2
```

- <a name="security-group-prefix-lists">`alb.ingress.kubernetes.io/security-group-prefix-lists`</a> specifies the managed prefix lists that are allowed to access LoadBalancer.

Expand Down
1 change: 1 addition & 0 deletions pkg/annotations/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const (
IngressSuffixlsAttsAnnotationPrefix = "listener-attributes"
IngressLBSuffixMultiClusterTargetGroup = "multi-cluster-target-group"
IngressSuffixLoadBalancerCapacityReservation = "minimum-load-balancer-capacity"
IngressSuffixInboundSecurityGroups = "inbound-security-groups"

// NLB annotation suffixes
// prefixes service.beta.kubernetes.io, service.kubernetes.io
Expand Down
31 changes: 27 additions & 4 deletions pkg/ingress/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,25 @@ type listenPortConfig struct {
sslPolicy *string
tlsCerts []string
mutualAuthentication *elbv2model.MutualAuthenticationAttributes
securityGroupIDs []string
}

func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context.Context, ing *ClassifiedIngress) (map[int32]listenPortConfig, error) {
explicitTLSCertARNs := t.computeIngressExplicitTLSCertARNs(ctx, ing)
explicitSSLPolicy := t.computeIngressExplicitSSLPolicy(ctx, ing)
var prefixListIDs []string
t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixSecurityGroupPrefixLists, &prefixListIDs, ing.Ing.Annotations)

securityGroupIDs, err := t.computeIngressExplicitSecurityGroupIDs(ctx, ing)
if err != nil {
return nil, err
}

inboundCIDRv4s, inboundCIDRV6s, err := t.computeIngressExplicitInboundCIDRs(ctx, ing)
if err != nil {
return nil, err
}

mutualAuthenticationAttributes, err := t.computeIngressMutualAuthentication(ctx, ing)
if err != nil {
return nil, err
Expand Down Expand Up @@ -161,10 +169,11 @@ func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context
listenPortConfigByPort := make(map[int32]listenPortConfig, len(listenPorts))
for port, protocol := range listenPorts {
cfg := listenPortConfig{
protocol: protocol,
inboundCIDRv4s: inboundCIDRv4s,
inboundCIDRv6s: inboundCIDRV6s,
prefixLists: prefixListIDs,
protocol: protocol,
inboundCIDRv4s: inboundCIDRv4s,
inboundCIDRv6s: inboundCIDRV6s,
prefixLists: prefixListIDs,
securityGroupIDs: securityGroupIDs,
}
if protocol == elbv2model.ProtocolHTTPS {
if len(explicitTLSCertARNs) == 0 {
Expand Down Expand Up @@ -240,6 +249,20 @@ func (t *defaultModelBuildTask) computeIngressListenPorts(_ context.Context, ing
return portAndProtocols, nil
}

func (t *defaultModelBuildTask) computeIngressExplicitSecurityGroupIDs(ctx context.Context, ing *ClassifiedIngress) ([]string, error) {
var rawSecurityGroups []string
if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixInboundSecurityGroups, &rawSecurityGroups, ing.Ing.Annotations); !exists {
return nil, nil
}

securityGroupIDs, err := t.sgResolver.ResolveViaNameOrID(ctx, rawSecurityGroups)
if err != nil {
return nil, fmt.Errorf("invalid %v settings on Ingress: %v: %w", annotations.IngressSuffixInboundSecurityGroups, k8s.NamespacedName(ing.Ing), err)
}

return securityGroupIDs, nil
}

func (t *defaultModelBuildTask) computeIngressExplicitInboundCIDRs(_ context.Context, ing *ClassifiedIngress) ([]string, []string, error) {
var rawInboundCIDRs []string
fromIngressClassParams := false
Expand Down
12 changes: 12 additions & 0 deletions pkg/ingress/model_build_managed_sg.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,18 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupIngressPermissions(_ co
},
})
}
for _, sgID := range cfg.securityGroupIDs {
permissions = append(permissions, ec2model.IPPermission{
IPProtocol: "tcp",
FromPort: awssdk.Int64(port),
ToPort: awssdk.Int64(port),
UserIDGroupPairs: []ec2model.UserIDGroupPair{
{
GroupID: sgID,
},
},
})
}
}
return permissions
}
15 changes: 15 additions & 0 deletions pkg/ingress/model_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,9 @@ func (t *defaultModelBuildTask) mergeListenPortConfigs(_ context.Context, listen
var mergedMtlsAttributesProvider *types.NamespacedName
var mergedMtlsAttributes *elbv2model.MutualAuthenticationAttributes

var mergedSecurityGroupProvider *types.NamespacedName
mergedSecurityGroups := sets.NewString()

for _, cfg := range listenPortConfigs {
if mergedProtocolProvider == nil {
mergedProtocolProvider = &cfg.ingKey
Expand Down Expand Up @@ -345,6 +348,17 @@ func (t *defaultModelBuildTask) mergeListenPortConfigs(_ context.Context, listen
}
}

if len(cfg.listenPortConfig.securityGroupIDs) != 0 {
cfgSecurityGroups := sets.NewString(cfg.listenPortConfig.securityGroupIDs...)
if mergedSecurityGroupProvider == nil {
mergedSecurityGroupProvider = &cfg.ingKey
mergedSecurityGroups = cfgSecurityGroups
} else if !mergedSecurityGroups.Equal(cfgSecurityGroups) {
return listenPortConfig{}, errors.Errorf("conflicting security groups, %v: %v | %v: %v",
*mergedSecurityGroupProvider, mergedSecurityGroups.List(), cfg.ingKey, cfgSecurityGroups.List())
}
}

if cfg.listenPortConfig.sslPolicy != nil {
if mergedSSLPolicyProvider == nil {
mergedSSLPolicyProvider = &cfg.ingKey
Expand Down Expand Up @@ -391,6 +405,7 @@ func (t *defaultModelBuildTask) mergeListenPortConfigs(_ context.Context, listen
sslPolicy: mergedSSLPolicy,
tlsCerts: mergedTLSCerts,
mutualAuthentication: mergedMtlsAttributes,
securityGroupIDs: mergedSecurityGroups.List(),
}, nil
}

Expand Down