Repository navigation
Render harness envmap in the docker compose target - #875
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
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).
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
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.
This PR proposes rendering
harness.envmapin 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.yamland_helpers.tpl) add everyharness.envmapentry to the container environment, and the docs point toenvmapas the preferred way to set variables, withenvdeprecated.deployment-configuration/compose/templates/auto-compose.yamlonly iterated the list-basedharness.env, so everyenvmapentry was silently dropped from the generateddocker-compose.yaml. The most visible effect is on the bundledaccountsapplication: all of its Keycloak settings (KC_DB,KC_DB_URL_HOST,KC_PROXY_HEADERS,KC_HTTP_ENABLED, ...) live inenvmap, and none of them reached the composeaccountsservice.The fix adds a
rangeover$app_config.harness.envmapright after the existingharness.envloop, emittingNAME=valueentries. 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'squotedoes. The deprecatedenvlist is untouched, so existing values files render exactly as before.Reproduction on current
develop(8852037): generating the compose chart forsamples(whosevalues.yamlsetsenv: WORKERS=3andenvmap: ENVIRONMENT_TEST_A/ENVIRONMENT_TEST_B) withcreate_docker_compose_configuration([root, 'tools/deployment-cli-tools/tests/resources'], include=['samples'], ...)and thenhelm template deployment/compose, thesamplesservice environment containsWORKERS=3but neitherENVIRONMENT_TEST_*variable. Doing the same withinclude=['accounts']against the repository root yields noKC_*entries at all; with this change it yields all 14.Verification: the new
test_compose_envmap_renderingintools/deployment-cli-tools/tests/test_dockercompose.pyrenders the compose chart withhelm template(same approach as the existing gatekeeper rendering test) and checks that envmap entries are present next to theenvlist, that overriding one key keeps the others (the scenario in #705), that values with special characters and booleans render correctly, and that an app withoutenvmapis unaffected. It fails ondevelop(AssertionError: assert 'ENVIRONMENT_TEST_A=value' in ['CH_CURRENT_APP_NAME=samples', ...]) and passes with the change. The fulltools/deployment-cli-toolssuite 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).
If you'd rather not receive contributions like this, reply
no-more-prson 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