[nginx-ingress-controller] Add cidr whitelist support

[aledbf]

fixes #1141

[aledbf]

ping @bprashanth

[bprashanth]

does this handle a command seperated list of cidrs? if not suggest making it like a similar annotation we already have for service.type=lb: https://github.com/kubernetes/kubernetes.github.io/pull/632/files. In fact can we call the same: https://github.com/kubernetes/kubernetes/blob/dae5ac482861382e18b1e7b2943b1b7f333c6a2a/pkg/api/service/annotations.go

[aledbf]

yes https://github.com/kubernetes/kubernetes/blob/master/pkg/util/net/sets/ipnet.go#L26

EDIT: thanks for the question about the list. Fixed and test added

[bprashanth]

can you include that as a comment/in the doc somewhere and change the name?

[aledbf]

change the name?

which name?

[bprashanth]

The name of the annotation, to match what we already have in the l4 loadbalancer

[aledbf]

even with this? https://github.com/kubernetes/kubernetes/blob/3beed6e1aadf489975f21e030c443ffa1dcea67d/pkg/kubectl/cmd/util/factory.go#L782

[bprashanth]

It's deprecated because it's turning into a field with the same name kubernetes/website#632

[aledbf]

done

[sander-su]

@aledbf would you also put in aliasing via configmap like I mentioned in the issue?
So a configmap (controller wide) properties would be
whilelist.alias.corporate: 52.0.0.0/16, 190.36.45.160/23
whitelist.alias.internal: 10.0.0.0/16

And than an additional annotation on the ingres would reference these like
Annotation.whitelist.aliases: corporate, internal

These would then be added to the cidr whitelisting per ingres option.

Otherwise if our corporate outbound ip changes I have to adjust all ingress configs and with aliases only one configmap property.

If this is already on planning nothing said but did not see it in the commits and thus not completely fixes the issue.

[aledbf]

@sander-su already included https://github.com/kubernetes/contrib/pull/1144/files#diff-3a0f10ad1be4fb0972021177f2cb9e9aR236
But the behavior is to replace the global value, not append

[sander-su]

@aledbf
Ok, so just to check.
When putting something in the configmap it will be used for the whole ingresscontroller and ingres on it?

Then to validate something else, could be my misunderstanding. An ingress controller handeles ingresess from all namespaces, right? (seen this behaviour in 0.61) An ingres can only forward incomming traffic to the services of the ns it belongs to, although the controller could be in another ns.
How can I limit access to certain services and not to other? Cannot make 2 controllers because every ingress is deployed on both.

Furthermore, we need to specify which alias for which ingres.
As we use aws some devs ops use clientvpn/stepstone and thus have internal vpc ip. Site to site vpn to our corporate network is not desired. I want to whitelist some services to corporate use only. So devs would use monitoring, no kubernetes authz because only dev, so no proxy. Also project external hire.
Corporate is needed for extra security and easy access when distributing accounts is difficult.
Hence I need the differiantation between public, internal, corporate.

What are your thoughts?

[aledbf]

When putting something in the configmap it will be used for the whole ingresscontroller and ingres on it?

Yes

An ingress controller handeles ingresess from all namespaces, right?

This is the default. You can specify one namespace to restrict where the controller should look for Ingress rules.

[aledbf]

Hence I need the differiantation between public, internal, corporate.

Currently there's no way to express this with Ingress. I think you can have one controller for each environment, but even with that you need different ingress rules

[sander-su]

If I can restrict an ingress controller to a namespace for rules this would be suficient.
Devops monitoring/jenkins could be in kube-system and corporate only would get its own ns.

Which setting restrict the controller to one (or multiple?) ns for its rules. Cannot find it easily.

Think it would be good to name this in the documentation.

[sander-su]

And thanks for resolving the issue this quick ;)

[aledbf]

Which setting restrict the controller to one (or multiple?) ns for its rules. Cannot find it easily.

It's an argument
--watch-namespace="": Namespace to watch for Ingress. Default is to watch all namespaces

Just in case only is possible to watch all the namespaces or one (no multiple)

Think it would be good to name this in the documentation.

Good point. I will add a section for this in #1130.

[sander-su]

One more, regarding the discussion on the issue. (And one for the docs)
So for now only sourceip filtering is added? (For me for now sufficient but cant speak for anyone else)
No filtering by x-header?

[aledbf]

So for now only sourceip filtering is added?

Yes.

Please open an issue with your use case to use a different source

[sander-su]

Not needed.
but good to mention in the docs that if you are using aws/elb it must be combined with proxy protocol for it to work.

[sander-su]

Load-balancer-source-ranges is a bit misleading though. I would interpreted it as i need to specify the range of my load balancer here. But I whitelist the ip of the traffic comming into the loadbalancer by using proxy protocol.
Whitelist-source-range would be more clear i think.

[bprashanth]

I would interpreted it as i need to specify the range of my load balancer here

I would interpret it as I need to specify the source range of my loadbalancer here. I don't particularly have a strong preference about including/leaving out the whitelist prefix. If it makes things that much clearer to a majority of users, we can do it. It might lead to confusion because people don't get how it ties in with loadbalancer-source-ranges upstream, but we can fix that with documentation hopefully.

[bprashanth]

LGTM thanks