canonical / paas-app-charmer

2 stars 2 forks source link

Add new env variable {framework}_BASE_URL with the ingress url #27

Closed javierdelapuente closed 4 months ago

javierdelapuente commented 4 months ago

Applicable spec:

Overview

Add new environment for the ingress url. The env variable name will be "FLASK_BASE_URL" or "DJANGO_BASE_URL".

If there is no ingress, the url returned will be the k8s service. The port hast to be opened for the service to work correctly.

Besides, pebble will be replanned on the relevant ingress events (so the workload can get the new env variable if it has been updated).

Documentation will be updated to include the new env variable when the package gets released to pipy (documentation is currently under review).

Rationale

Allow the workload to know the ingress url.

Juju Events Changes

Module Changes

Library Changes

Checklist

javierdelapuente commented 4 months ago

How about adding/updating an example in the examples dir?

The ultimate reason for this PR, is that with ingress in path routing, Gunicorn/Django/Flask look like they do not work correctly in all configurations. With this environment variable the user could configure the Flask/Django app to suit the specific use case (without installing libraries that depend on non standard headers like X-Forwarded-Prefix).

For example, with traefik, the proxy by default sends the header X-Forwarded-Prefix, but it is not read by gunicorn/django (https://code.djangoproject.com/ticket/34290, https://stackoverflow.com/questions/56556514/add-global-prefix-for-django-urls, ) and for Flask it looks like it requires a "fix" https://flask.palletsprojects.com/en/3.0.x/deploying/proxy_fix/ (haven't tested it). I think gunicorn reads SCRIPT_NAME header instead (see https://docs.gunicorn.org/en/latest/faq.html#how-do-i-set-script-name).

In Netbox, I added several changes to make it work. However I am not sure if there is a "cleaner/better" way, so I am not really sure if/how to change the examples.

javierdelapuente commented 4 months ago

LGTM. Nitpick: Please include an explanation in the PR description as to why you did not check the charmhub docs box.

Thanks for the comment! PR description updated.

github-actions[bot] commented 4 months ago

Test coverage for ce8aaa053b9458c73a3fc970862452e3b15f0e02

Name                                            Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------------------------------------
paas_app_charmer/__init__.py                       29     14      0      0    52%   13-14, 19-20, 26-27, 33-34, 40-41, 47-48, 54-55
paas_app_charmer/_gunicorn/__init__.py              0      0      0      0   100%
paas_app_charmer/_gunicorn/charm.py               203     25     48      4    88%   32-33, 40-41, 162-163, 165-166, 187->exit, 199-203, 256-258, 338-339, 344, 349, 354, 364, 369, 374, 379, 384, 409
paas_app_charmer/_gunicorn/charm_state.py         107      2     20      2    97%   182, 266
paas_app_charmer/_gunicorn/charm_utils.py          23      0      0      0   100%
paas_app_charmer/_gunicorn/observability.py        13      0      2      0   100%
paas_app_charmer/_gunicorn/secret_storage.py       13      0      0      0   100%
paas_app_charmer/_gunicorn/webserver.py            75      4     14      1    94%   156, 168-174
paas_app_charmer/_gunicorn/workload_config.py      19      0      0      0   100%
paas_app_charmer/_gunicorn/wsgi_app.py            104      0     44      2    99%   86->88, 116->exit
paas_app_charmer/database_migration.py             35      0      2      0   100%
paas_app_charmer/databases.py                      25      2     11      1    92%   89-90
paas_app_charmer/django/__init__.py                 1      0      0      0   100%
paas_app_charmer/django/charm.py                   45      6      6      1    86%   73-77, 99, 114-115
paas_app_charmer/exceptions.py                      5      0      0      0   100%
paas_app_charmer/flask/__init__.py                  1      0      0      0   100%
paas_app_charmer/flask/charm.py                    37      0      2      0   100%
paas_app_charmer/secret_storage.py                 39      3     16      5    85%   50, 54->53, 55->57, 85, 104
paas_app_charmer/utils.py                          11      0     12      0   100%
-------------------------------------------------------------------------------------------
TOTAL                                             785     56    177     16    92%

Static code analysis report

Run started:2024-07-10 07:42:07.068089

Test results:
    No issues identified.

Code scanned:
    Total lines of code: 1681
    Total lines skipped (#nosec): 0
    Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
    Total issues (by severity):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
    Total issues (by confidence):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
Files skipped (0):