fix: handle invalid combination of external_hostname and routing_path #420
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Previously, this charm accepted the following configuration:
When external_hostname is unset, the url provided for any application related by ingress uses the LoadBalancer's external address, which may be an IP. In these cases, it would provide charms urls like
model.app.1.2.3.4
, which are invalid URLs (the last segment of a URL cannot be all-numeric). This led to an uncaught pydantic validation error when callingipa.publish_url()
because that method includes validation of the URL, putting the charm in Error state. An example of this is shown in #361Fixes #361
Solution
Ideally, the fix here would be to validate the charm configuration+LoadBalancer details and halt charm execution if the configuration was invalid, putting the charm into BlockedStatus until resolved. The problem is that the current architecture of this charm makes that solution challenging. The charm is designed to atomically handle events (doing only the work a particular event needs) rather than holistically (recomputing the world on each event), meaning that skipping or losing track of events leads to undesired charm states. Also,
ipa.publish_url()
is called deep in most (all?) event handlers, making it difficult to properly handle these errors at the charm level without a major refactor of the charm.As a compromise, the following has been done:
publish_url
method has been updated to catch the pydantic validation error cited in Unable to use Load Balancer's IP address for the ingress gateway #361 and log it rather than raise it to the charm. The library then writes ingress=None to the databag instead of the invalid URL, giving a soft indication to the user that the url is invalid.The upshot of these changes is that this charm will:
Context
#361
Testing Instructions
Everything added here is tested in unit tests. Integration tests were not added since it can be fully tested at the unit level.
To manually integration test:
Upgrade Notes
no effect on upgrade