Skip to content

Render harness envmap in the docker compose target - #875

Open
eastagiletracker wants to merge 1 commit into
MetaCell:developfrom
eastagiletracker:agile-board/compose-envmap
Open

eastagiletracker wants to merge 1 commit into
MetaCell:developfrom
eastagiletracker:agile-board/compose-envmap

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes rendering harness.envmap in the docker compose target, so map-style environment variables reach compose services the same way they reach Helm deployments (Fixes #705). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/739. You can sign in with your GitHub ID to claim ownership of the project.

What changed and why

The Helm templates (auto-deployments.yaml and _helpers.tpl) add every harness.envmap entry to the container environment, and the docs point to envmap as the preferred way to set variables, with env deprecated. deployment-configuration/compose/templates/auto-compose.yaml only iterated the list-based harness.env, so every envmap entry was silently dropped from the generated docker-compose.yaml. The most visible effect is on the bundled accounts application: all of its Keycloak settings (KC_DB, KC_DB_URL_HOST, KC_PROXY_HEADERS, KC_HTTP_ENABLED, ...) live in envmap, and none of them reached the compose accounts service.

The fix adds a range over $app_config.harness.envmap right after the existing harness.env loop, emitting NAME=value entries. Each entry is quoted (printf "%s=%v" | quote) so values containing : or # stay valid YAML, and numbers and booleans are stringified the same way the Helm template's quote does. The deprecated env list is untouched, so existing values files render exactly as before.

Reproduction on current develop (8852037): generating the compose chart for samples (whose values.yaml sets env: WORKERS=3 and envmap: ENVIRONMENT_TEST_A/ENVIRONMENT_TEST_B) with create_docker_compose_configuration([root, 'tools/deployment-cli-tools/tests/resources'], include=['samples'], ...) and then helm template deployment/compose, the samples service environment contains WORKERS=3 but neither ENVIRONMENT_TEST_* variable. Doing the same with include=['accounts'] against the repository root yields no KC_* entries at all; with this change it yields all 14.

Verification: the new test_compose_envmap_rendering in tools/deployment-cli-tools/tests/test_dockercompose.py renders the compose chart with helm template (same approach as the existing gatekeeper rendering test) and checks that envmap entries are present next to the env list, that overriding one key keeps the others (the scenario in #705), that values with special characters and booleans render correctly, and that an app without envmap is unaffected. It fails on develop (AssertionError: assert 'ENVIRONMENT_TEST_A=value' in ['CH_CURRENT_APP_NAME=samples', ...]) and passes with the change. The full tools/deployment-cli-tools suite with Helm 3.16 on the path went from 155 passed before the change to 156 passed after, with no failures in either run.

How this was managed

This work was tracked as Application env specification should be a map, not list on a board at https://eastagiletracker.com/projects/739, imported from this repository's issues and pull requests (746 stories).

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

The Helm templates add every harness.envmap entry to the container environment, but auto-compose.yaml only iterated the deprecated harness.env list, so envmap variables were silently dropped from docker-compose.yaml (e.g. all KC_* settings of the accounts application).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Application env specification should be a map, not list

1 participant