Aws-load-balancer-controller: Feature: Annotation for Default Security Group Rules

[iAnomaly]

Currently bring your own security group(s) is supported with the annotation alb.ingress.kubernetes.io/security-groups and absent that, you get a default SG on the ALB AND on the target instance(s) for the configured protocols and ports.

While this is a great start, it would also be convenient to have a middle ground where the default SGs could be configured with a custom whitelist not unlike the Kubernetes Service resource LoadBalancer type's loadBalancerSourceRanges annotation. Perhaps something like alb.ingress.kubernetes.io/default-security-group-source-ranges: that accepts a list of IP CIDR or SG IDS? This avoids having to maintain the security group(s) externally but still enables custom rules. This list should override the current default 0.0.0.0/0 rule; not merge with it such that 0.0.0.0/0 could be optionally omitted.

[M00nF1sh]

Hi, what's your use case for custom rules for the default created securityGroup for supporting ingresses?
Since it's only purpose is for the alb to reach it.

[iAnomaly]

Hi @M00nF1sh.

So obviously this is a proposal to expand the original purpose; I understand the current intent. For our use case we use the LoadBalancer Kubernetes Service type which supports whitelisting rules directly in the manifest without having to create and manage additional security groups somewhere else like CloudFormation, APIs, etc.

[iAnomaly]

Ideally I'm just looking for the ALB Ingress Controller analog of this existing feature: https://kubernetes.io/docs/tasks/access-application-cluster/configure-cloud-provider-firewall/#restrict-access-for-loadbalancer-service

[M00nF1sh]

/kind feature
Sorry for the late reply. This looks like a legit use case. We can add support for this(limit access to the internet facing LB)

BTW, not sure whether you are aware of this, if you only want your loadBalancer be accessible within your VPC, you can use alb.ingress.kubernetes.io/scheme:internal.

[iAnomaly]

Thank you @M00nF1sh! Yes, I am aware of alb.ingress.kubernetes.io/scheme:internal and we're using that for internal ingresses.

[M00nF1sh]

@iAnomaly Hi, I just noticed that this feature is already supported. You can add alb.ingress.kubernetes.io/security-group-inbound-cidrs: 10.0.0.0/24 annotation for it.

I'll update the docs for this in next release

[iAnomaly]

Great finding this @M00nF1sh. I just tried it:

alb.ingress.kubernetes.io/security-group-inbound-cidrs:
  - 10.0.0.0/24
  - 10.0.1.0/24

But it fails:
error: error validating "deploy/ingress.sbxdev.yaml": error validating data: ValidationError(Ingress.metadata.annotations.alb.ingress.kubernetes.io/security-group-inbound-cidrs): invalid type for io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta.annotations: got "array", expected "string"; if you choose to ignore these errors, turn validation off with --validate=false

Given that it only supports type "string" the workaround for now is to use YAML's _folded style_ block scalar with the _strip_ block chomping to exclude the final newline:

alb.ingress.kubernetes.io/security-group-inbound-cidrs: >-
  10.0.0.0/24,
  10.0.1.0/24

We can make this work for now @M00nF1sh but I would love to see type "array" support added to allow additional YAML syntactic sugar such as inline comments and dropping trailing commas:

alb.ingress.kubernetes.io/security-group-inbound-cidrs:
  # Office CIDR
  - 10.0.0.0/24
  # Some other CIDR
  - 10.0.1.0/24

Thanks again for finding this.

[M00nF1sh]

@iAnomaly hi, that annotation is actually comma-separated string, you should use 10.0.0.0/24,10.0.1.0/24 instead. 😄

[iAnomaly]

Yes, thanks @M00nF1sh. As you can see I figured that out. 😄 Like I said, we can work with this for now but we would still love to see array support for the reasons I enumerated above. I appreciate your help!

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.