feat: nginx cache optimizations, TLS key permissions, and test framework migration - #142
Draft
florentianayuwono wants to merge 63 commits into
Draft
Conversation
…llocation Story 1 of ISD296: Remove hostname/path-based routing from content-cache and content-cache-backends-config charms. Each cache-config relation now gets a unique nginx listening port (starting at 8080), replacing the server_name + location-based routing model. Changes: - content-cache/src/state.py: Remove hostname, path, backends_path from LocationConfig; change NginxConfig type to dict[relation_id, LocationConfig]; update get_nginx_config to return flat per-relation dict - content-cache/src/nginx_manager.py: Replace server_name/location routing with listen <port> and single location /; update _create_virtualhost_config to accept port; remove hostname_to_cert parameter - content-cache/src/charm.py: Add StoredState port_map; add _get_port_for_relation; remove TLS certificate management (no hostnames); remove set_ports(80,443) - content-cache-backends-config/src/state.py: Remove hostname, path, backends_path from Configuration model and from_charm/to_integration_data - content-cache-backends-config/config.yaml: Remove hostname, path, backends-path config options - Tests updated throughout to reflect new behaviour Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- charm-design.md: replace hostname-based routing with per-relation port allocation; update nginx config examples; update cache dir paths from /data/nginx/cache/<hostname>/ to /data/nginx/cache/<port>/; remove TLS termination section; update healthcheck Lua snippet - tutorial: remove hostname= config option; update curl test to use port 8080 - enable-https.md: rewrite as 'connect to HTTPS backends' guide since TLS termination for incoming traffic is now handled upstream by ingress - components.md: remove stale warning about charm compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix E303/E302 extra blank lines in test_charm.py and test_state.py - Fix isort: add blank line between stdlib and local imports in test_charm.py - Fix black formatting in charm.py, nginx_manager.py, test_charm.py, test_nginx_manager.py - Fix mypy: use typed local variable for StoredState port_map access in charm.py - Fix pylint W0612: replace unused relation_id loop variable with _ in nginx_manager.py - Fix vale spell check: reword possessives in charm-design.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove Story 1 TDD comment blocks from all test files - Remove tests that assert absence of legacy fields (no_hostname, no_path, no_backends_path, no_routing_keys) — the positive behavior tests are sufficient - Remove bold formatting and em dashes from docs (charm-design.md, enable-https.md) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove hostname/path-based routing config from all integration tests. Replace hostname-based query_cache calls with port-based (default 8080). Skip TLS cert tests pending re-implementation. Update metric test to remove hostname/secrets usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Without backends-path routing, nginx now forwards the exact request path to the backend. Change http-ok and https-ok test servers to serve at '/' instead of a random path, so queries to the cache at '/' reach a handler that returns 200. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…test teardown - Add port_map.pop() in _on_cache_config_relation_broken so the freed port becomes available for the next relation. Without this, relation 2's port (8080) remained reserved, causing relation 3 to get port 8081. Tests querying the default port 8080 then get ConnectionRefused. - In the integration test fixture teardown, wait for config app units to be fully removed before the next test starts. Previously only the cache app's blocked state was awaited, leaving a dying config unit that caused race conditions when the subsequent test created a new relation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Erin Conley <erin.conley@canonical.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Call unit.set_ports() with allocated ports after nginx active, and update on relation-broken so Juju firewall opens/closes ports correctly - Raise RuntimeError with clear message when port range is exhausted instead of falling through to a bare KeyError - Fix update_and_load_config type annotation from NginxConfig to dict[int, tuple[int, LocationConfig]] to match actual callers - Remove unused NginxConfig import from nginx_manager.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The original skip reason implied TLS termination was permanently removed. Per ISD-296 spec, content-cache will still terminate TLS for HAProxy→content-cache traffic via a single cache cert (tls-certificates relation) — not hostname-based. Update skip reason to clarify. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onfig charm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…he state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… on broken Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ache-backends Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Erin Conley <erin.conley@canonical.com>
…erence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use per-peer healthcheck ports by omitting the global 'port' override in hc.spawn_checker; each server entry already carries host:port so the healthcheck library uses each peer's own port - Remove dead else-branch fallback on first_url.port (pydantic AnyHttpUrl always populates the default port) - Clear cache-backends on all relations when config fails validation or nginx does not reach ActiveStatus, preventing stale values misleading ingress components - Only write cache-backends when the value differs to avoid unnecessary relation-changed churn on every update-status hook - Add _clear_cache_backends helper; add tests for stale-clearing and no-write-when-unchanged behaviour - Remove dead CERTIFICATE_INTEGRATION_NAME constant from state.py - Add validate_backends_scheme validator to backends-config Configuration so mixed-scheme errors surface on the correct charm - Add comment explaining proxy_set_header Host is intentionally absent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mment - Remove code comment about proxy_set_header Host being intentionally absent - Add test_healthcheck_worker_uses_per_peer_ports: verifies no global 'port =' override in the generated lua script when backends use different ports - Add test_healthcheck_worker_upstream_entries_carry_ports: verifies each server entry carries its own host:port for the healthcheck library Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
florentianayuwono
requested review from
swetha1654 and
yanksyoon
and removed request for
a team
August 8, 2026 14:10
florentianayuwono
marked this pull request as draft
August 8, 2026 14:10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
florentianayuwono
force-pushed
the
feat/optimizations-and-refactorings
branch
from
August 8, 2026 14:17
6739a6b to
68912c3
Compare
…failures
Two bugs found and fixed:
1. test_healthchecks_ssl_verify (parametrised): the no_ssl_verify run
added a receive-ca-cert relation but never removed it in teardown.
When the ssl_verify run called model.integrate() again the relation
already existed and raised, causing the test to fail before
integrate_config() was ever called - leaving the cache stuck in
'Waiting for integration with config charm'. Fix: wrap the test body
in try/finally to always remove the cert relation after each
parametrised run.
2. test_certificate_transfer_full_lifecycle: the test integrated
cache-config first, then the CA cert provider. When
certificate_set_updated fired immediately (before the provider had
written its cert data), event.certificates was empty. Writing an
empty CA bundle caused nginx to reject its config and stay in
'maintenance: Nginx is not ready'. Fix:
a) Reorder the test to integrate cert first (before cache-config),
matching the working pattern in test_healthchecks_ssl_verify.
b) Guard _on_certificates_available against empty cert sets so the
charm ignores premature events and waits for the real certs.
c) Make get_ca_bundle_path() return None for an empty bundle file
so an empty CA bundle never reaches update_and_load_config().
Also add try-finally cleanup for both cert relations in test_tls_cert
so subsequent tests start with clean state even on failure.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
florentianayuwono
force-pushed
the
feat/optimizations-and-refactorings
branch
2 times, most recently
from
August 8, 2026 16:34
fc56f29 to
d8e598b
Compare
…rrive The _on_certificates_available event and cache_config_relation_changed can fire in any order. If cache_config_relation_changed arrives while the cert provider is still processing its hooks (cert not yet written to the relation databag), the charm sets WaitingStatus and never recovers — the subsequent certificate_set_updated event writes the bundle but no further event re-triggers _load_nginx_config(). Fix: add _sync_ca_certs_from_relations() which reads from all active cert-transfer relations via get_all_certificates_by_relation(). Call it inside _load_nginx_config() when HTTPS backends are present but the CA bundle is missing. This eliminates the timing dependency entirely. Also revert the test ordering in test_certificate_transfer_full_lifecycle to the natural sequence (cache-config first, then cert) which is the real-world case that exercises the race fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cache_path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rewrote test_charm.py using scenario.Context, scenario.State, and scenario.SubordinateRelation (required for subordinate charm) - Rewrote conftest.py with scenario fixtures (ctx, ctx_leader) - Added ops-scenario>=7.0.0,<9.0.0 to requirements.txt - Fixed try-except-raise pattern in state.py validators to satisfy both DCO051 (flake8) and W0706 (pylint) - All 57 unit tests pass; lint 10.00/10 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ilant - Replace async/await + python-libjuju/pytest-asyncio with sync jubilant - Rewrite conftest.py: juju fixture using jubilant.temp_model(), all fixtures sync, app fixtures return str names instead of Application - Rewrite helpers.py: CacheTester uses jubilant.Juju + app name strings, read_file/get_cache_backends/run_in_unit use juju.exec() - Rewrite test_basic.py, test_healthchecks.py, test_metric.py, test_tls_cert.py: remove async def / await / asyncio.sleep, use juju.wait() predicates, juju.integrate(), juju.remove_relation() - Update tox.ini: replace juju+pytest-asyncio with jubilant>=1.10.0,<2.0.0 - Use correct jubilant API: juju.status() (method), app_status.current, workload_status.current/.message, add_unit(app, num_units=1) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
florentianayuwono
force-pushed
the
feat/optimizations-and-refactorings
branch
from
August 8, 2026 18:23
d8e598b to
d9a9b4d
Compare
When ssl_verify=false, nginx should not verify the backend's TLS certificate — neither in healthchecks nor in proxied requests. Previously, proxy_ssl_verify was always set to 'on' whenever a CA bundle existed, regardless of the ssl_verify config value. This caused test_healthchecks_ssl_verify[no_ssl_verify] to fail with 502 because the backend's self-signed cert (not signed by cert_app's CA) could not be verified. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add juju.wait(active) after integrate_config()+setup_config() in test_tls_termination_full_lifecycle. Without this wait, the TLS cert from cache-lego can arrive before config subordinate hooks deliver valid backend data, leaving the charm blocked on 'Waiting for integration with config charm' for the entire 10-min window. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Overview
This PR implements several nginx cache optimizations, a TLS security hardening, and migrates both unit and integration tests to modern frameworks.
Stories
Story 1 & 2: Configurable
cache-inactiveandcache-max-sizecache-inactiveconfig option (default:10m) tocontent-cache-backends-configcache-max-sizeconfig option (default: empty = no limit) tocontent-cache-backends-configproxy_cache_pathdirectiveStory 3: Always-on
proxy_cache_lockproxy_cache_lock onto all nginx location blocksStory 4: TLS private key permission hardening
os.chmod(pem_file_path, 0o644)→0o640www-datauser/group onlyStory 5: Test framework migration
content-cacheandcontent-cache-backends-configfromops.testing.Harnesstoops-scenario(state-based testing)python-libjuju+pytest-asyncioto syncjubilantCommits
feat(backends-config): add cache-inactive and cache-max-size config optionsfeat(content-cache): wire cache-inactive and cache-max-size to proxy_cache_pathfeat(nginx): add proxy_cache_lock on to all location blocksfix(tls): restrict TLS private key file permissions to 0o640refactor(unit): migrate content-cache unit tests to ops-scenariorefactor(unit): migrate backends-config unit tests to ops-scenariorefactor(integration): migrate content-cache integration tests to jubilant