canonical / juju-lint

GNU General Public License v3.0
0 stars 4 forks source link

alert on lack of explicit default bindings #13

Closed zxhdaze closed 2 months ago

zxhdaze commented 2 months ago

If I understand correctly, whenever an endpoint is not explicitly bound to a space, the network-get primitive for a relation bound to it will return binding address(es) / ingress address(es) / egress subnets as if that endpoint had been bound to the default space for the application. However, if the default space has not been specified, the addresses returned are somewhat random. For multi-homed units, this can result in unexpected addresses being returned (see for example LP#1850129).

It would be good if juju-lint were to warn whenever no default space has been specified. We could optionally make an exception for applications that specify explicit bindings for all their endpoints, but not for specify a default one, although I think it would be more robust to ensure we always have a default to fall back onto, as newer charm releases may always introduce new endpoints.


Imported from Launchpad using lp2gh.

zxhdaze commented 2 months ago

(by aieri) Another approach could be including known peer relations to the list LP#1840814 is concerned with (unfortunately juju status doesn't distinguish between peer and non-peer relations), and simply rely on juju 2.7+ to be able to adjust bindings after deployment.

zxhdaze commented 2 months ago

(by aieri) This is particularly relevant if we consider that bug 1850129 has been closed as won't fix.

zxhdaze commented 2 months ago

(by ec0) Generically, we need to handle bindings. I think that this is a great place to start. I've triaged this high as I think binding support and checking for default bindings will be a big improvement for the capability of this tool.

zxhdaze commented 2 months ago

(by ec0) See also bug #1840814

zxhdaze commented 2 months ago

(by elmo) I think this means looking for bindings which are bound to an 'alpha' space.

This may cause false positives, we'd probably want to verify this theory on a few clouds first.

zxhdaze commented 2 months ago

(by martin-kalcok) There seems to be a new function "check_spaces" [1] that verifies if respective endpoints of the same relation are in the same space. I think that this logic could cover this specific bug as well.

The only problem is that this function can't verify the endpoints in output of the "juju status" and output of the "juju export-bundle" does not include bindings. So the only use-case where this check works is in the (not exported) bundle that includes explicit bindings.

I think that if we modified this function to be able to parse endpoint/bindings information from "juju status" output, it would resolve the problem stated in this bug.


[1] https://git.launchpad.net/juju-lint/tree/jujulint/lint.py#n681

zxhdaze commented 2 months ago

(by martin-kalcok) Thanks to bug report found by Gabriel [1], It seems that juju does not include bindings information in exported bundles only if the model has only one space. This means that it's unnecessary to implement parsing binding information from "juju status". We can rely on the information from "juju export-bundle".

Just by adding few warning log outputs we can reach state where juju-lint warns:


[1] https://bugs.launchpad.net/juju/+bug/1949883