sapcc / elektra

An opinionated openstack Web UI for consumer self service and operations.
Apache License 2.0
74 stars 28 forks source link

[lbaas] Add Allowed CIDRs feature to UI #1232

Closed velp closed 1 year ago

velp commented 1 year ago

We just added support for allowed_cidrs to our Octavia's driver implementation (https://github.com/sapcc/octavia-f5-provider-driver/pull/229). And we would like to have it in the dashboard.

API description:

How to set allowed_cidrs for existing listener:

curl -g -i -X PUT https://<URL>/v2.0/lbaas/listeners/<LISTENER_ID> \
    -H "Accept: application/json" \
    -H "Content-Type: application/json" \
    -H "User-Agent: openstacksdk/1.2.0 keystoneauth1/5.2.1 python-requests/2.31.0 CPython/3.11.3" \
    -H "X-Auth-Token: <TOKEN>" \
-d '{"listener": {"allowed_cidrs": ["10.46.22.1/24"]}}'

How to remove allowed_cidrs from a listener (set allow any subnets):

curl -g -i -X PUT https://<URL>/v2.0/lbaas/listeners/<LISTENER_ID> \
    -H "Accept: application/json" \
    -H "Content-Type: application/json" \
    -H "User-Agent: openstacksdk/1.2.0 keystoneauth1/5.2.1 python-requests/2.31.0 CPython/3.11.3" \
    -H "X-Auth-Token: <TOKEN>" \
-d '{"listener": {"allowed_cidrs": ["0.0.0.0/0"]}}'

To update the list of CIDRs and add one more, you have to send a full list of CIDRs. For example if listener has 10.46.22.1/24 CIDR configured and you want to add another CIDR 10.8.0.1/24 you have to send a body with both CIDRs:

{"listener": {"allowed_cidrs": ["10.46.22.1/24","10.8.0.1/24"]}}
ArtieReus commented 1 year ago

Hi @velp! I will prioritise the feature request and update you once we start with it.

ArtieReus commented 1 year ago

Hi @velp! as far as I know this feature ist not yet roll out in production. If we implement the feature in Elektra will crush as soon as it reaches production. Please let me know when it is in production deployed. Thx!

velp commented 1 year ago

@ArtieReus I will start rolling on this week. And, yes, will let you know when it will be everywhere.

velp commented 1 year ago

@ArtieReus this PR can be merged. The backend side rolled out everywhere.

ArtieReus commented 1 year ago

Looking at the existing listeners without CIDRs the value allowed_cidrs is set to null. Can we keep this when removing the cidrs of an existing listener?? This is easier then to have to add an extra value when the enduser removes the cidrs.

velp commented 1 year ago

Do you mean pass body:

{"listener": {"allowed_cidrs": [}}

instead of:

{"listener": {"allowed_cidrs": ["0.0.0.0/0"]}}

when we remove all CIDRs?

ArtieReus commented 1 year ago

I mean all listeners have default null if no CIDR set. Why should then I add a string when removing the CIDRs?? Screenshot 2023-08-01 at 14 21 26