Add support for controlling LoadBalancer source subnet #941
Conversation
| @@ -12,7 +12,7 @@ metadata: | |||
| service.beta.openshift.io/serving-cert-secret-name: noobaa-mgmt-serving-cert | |||
| service.alpha.openshift.io/serving-cert-secret-name: noobaa-mgmt-serving-cert | |||
| spec: | |||
| type: LoadBalancer | |||
| type: ClusterIP | |||
There was a problem hiding this comment.
Why do we change the mgmt to be ClusterIP ?
There was a problem hiding this comment.
@liranmauda this was one the feedback that I received during our 4.12 epic meeting. Unfortunately, I no longer remember the rationale behind the feedback but I would guess that it was regarding the fact that the mgmt service isn't being reached from outside the cluster (not even by the metrics aggregator).
There was a problem hiding this comment.
prometheus or anything also that reach the metrics will reach the mgmt URL.
I don't see any restriction that the metric should be reached only from within the cluster, and thats why I think it should not be ClusterIP by default.
And regarding to remote endpoints, they will need the mgmt to be LoadBalancer
@jackyalbo @dannyzaken keep me honest
| } | ||
|
|
||
| // LoadBalancerSourceSubnetSpec defines the subnets that will be allowed to access the NooBaa services | ||
| type LoadBalancerSourceSubnetSpec struct { |
There was a problem hiding this comment.
Why aren't we doing the same for the mgmt?
There was a problem hiding this comment.
@liranmauda, because the service is changed to type ClusterIP, setting the source subnets will not have any effects.
| break | ||
| fi | ||
| done | ||
| } |
|
Few General questions:
|
Actually this was done in response to feedbacks as mentioned here 😅.
We have a total of 3 service:
|
|
@liranmauda @jackyalbo please see if there is anything left open here per Utkarsh's last comment |
tangledbytes
force-pushed
the
utkarsh-pro/feature/loadbalancer-cidr-control
branch
3 times, most recently
from
9a77eab
to
79f800e
Compare
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com> add support for loadbalancer subnet in CLI Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com> add tests Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com> remove validation from reconcile loop and move to admission controller Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com> fix failing unit test Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com> allow per service subnet configuration Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
tangledbytes
force-pushed
the
utkarsh-pro/feature/loadbalancer-cidr-control
branch
from
79f800e
to
a4d632e
Compare
Explain the changes
This PR adds support for controlling NooBaa services' (s3, sts, mgmt) LoadBalancer's access subnet.
Consequently, it also helps control AWS security groups.
Example:
When no
--<svc>-load-balancer-source-subnetsis provided, AWS SG are wide open (as mentioned in the given BZ) however when these are provided, i.e:noobaa install -n <ns> --s3-load-balancer-source-subnets 10.0.0.0/16 --s3-load-balancer-source-subnets 159.89.162.248/32 --sts-load-balancer-source-subnets 0.0.0.0/0Will result in following:
Issues: Fixed #xxx / Gap #xxx
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2024681
Testing Instructions:
nb install -n <ns> --operator-image <image> --load-balancer-source-subnets <subnet>