Add rawsetenv message type for provider plugins#13742
Conversation
Providers can now send rawsetenv messages to inject environment variables into dependent services without the automatic service name prefix. This enables use cases where applications require exact variable names that cannot be altered. Closes docker#13727 Signed-off-by: Yohta Kimura <38206553+rajyan@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
this patch worked for my issue with mariadb requiring MYSQL_ROOT_PASSWORD with no override, not LOCKET_MYSQL_ROOT_PASSWORD as the compose provider was passing. |
glours
left a comment
glours
left a comment
There was a problem hiding this comment.
Few things to address before we can merge.
Also we need to add a sibling fixture to pkg/e2e/fixtures/providers/rawsetenv.yaml that sets environment: { CLOUD_REGION: user-value } on the test service, and assert that the resulting value matches the documented behavior (skipped write, warning, or overwrite, depending on which resolution is chosen).
| } | ||
|
|
||
| variables, err := s.executePlugin(cmd, command, service) | ||
| vars, err := s.executePlugin(cmd, command, service) |
There was a problem hiding this comment.
There's no good reason for it. Reverted to variables.
| s.Environment[prefix+key] = &val | ||
| } | ||
| for key, val := range vars.raw { | ||
| s.Environment[key] = &val |
There was a problem hiding this comment.
rawsetenv silently overwrites user-defined env vars
If a user sets environment: { CLOUD_REGION: eu-west-1 } and a provider emits rawsetenv CLOUD_REGION=us-east-1, the user value is silently clobbered. setenv was immune thanks to the prefix.
Please either skip writes when the key already exists, or we need to document this precedence explicitly.
@ndeloof what is you preference here? I'm in favor of the overwrite + warning message in logs, and you?
There was a problem hiding this comment.
Went with overwrite + warning as suggested. One clarification on the framing though: setenv normally doesn't overwrite anything, since the prefix gives each provider its own namespace and collisions are rare. But the write has no existence check either, so if a service does define the exact prefixed key (e.g. a user-set SECRETS_URL and a provider named "secret" set URL), setenv silently overwrites it the same way. I kept the warning on rawsetenv only since prefixed collisions are unlikely, but can extend it to setenv if you'd prefer consistency.
| This is useful when injecting secrets or configuration values that must match exact variable names expected by | ||
| applications or frameworks. Unlike `setenv`, which avoids collisions through automatic prefixing, `rawsetenv` keys | ||
| are the provider's responsibility to keep unique. If multiple providers emit the same `rawsetenv` key, the last one | ||
| to run will overwrite previous values. |
There was a problem hiding this comment.
executor.go runs plan nodes concurrently via errgroup. Two providers without a mutual dependency run in parallel; mux serializes the writes but not their order. The doc says "the last one to run will overwrite previous values", literally true, but readers will assume declaration order wins.
Either tighten the doc, or detect cross-provider rawsetenv collisions and fail.
There was a problem hiding this comment.
Reworded. Dropped the "last one to run will overwrite" phrasing and now state that providers without a depends_on link may run concurrently, so the resulting value is non-deterministic on collision, and that an override logs a warning.
| env := getEnv(res.Combined(), false) | ||
| assert.Check(t, slices.Contains(env, "PROVIDER1_URL=https://magic.cloud/provider1"), env) | ||
| assert.Check(t, slices.Contains(env, "PROVIDER2_URL=https://magic.cloud/provider2"), env) | ||
| assert.Check(t, slices.Contains(env, "CLOUD_REGION=us-east-1"), env) |
There was a problem hiding this comment.
both providers emit the same CLOUD_REGION value, so the conflict path is never exercised. Please add a test where a rawsetenv key collides with a user-defined env var
There was a problem hiding this comment.
Removed the CLOUD_REGION assertion from TestDependsOnMultipleProviders since, as you noted, both providers emit the same value so it never exercised a conflict, and a real cross-provider conflict can't be asserted deterministically (independent providers run concurrently, so it would be flaky). The deterministic conflict is now covered by TestProviderRawSetEnvOverridesUserEnv, which asserts the provider value wins over a user-defined one and that the override is logged as a warning rather than happening silently. The non-deterministic cross-provider case is documented in extension.md.
rawsetenv injects provider variables without the service-name prefix, so a key can collide with a value already set on the dependent service, whether declared by the user in environment or emitted by another provider. Log a warning and overwrite on collision, document the precedence and the non-deterministic ordering between concurrent providers, and cover the user-environment override with an e2e test. Signed-off-by: Yohta Kimura <38206553+rajyan@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
What I did
Added a new
rawsetenvmessage type to the provider plugin protocol. This allows providers to inject environment variables into dependent services without the automatic service name prefix.Currently,
setenvalways prefixes variables with the service name (e.g.,URLbecomesDATABASE_URL). This works well for connection strings, but some applications require exact variable names that cannot be altered. There is no way to inject these as-is today.With this change, providers can choose per-variable whether to use the prefixed (
setenv) or unprefixed (rawsetenv) behavior:{"type": "setenv", "message": "URL=https://example.com"} {"type": "rawsetenv", "message": "SECRET_KEY=xxx"}Changes
pkg/compose/plugins.go— AddRawSetEnvTypeconstant andpluginVariablesstruct to separate prefixed/raw variablesdocs/extension.md— Documentrawsetenvin the protocol specificationdocs/examples/provider.go— Addrawsetenvusage to the example providerpkg/e2e/providers_test.go— Test for single-provider rawsetenv and multi-provider rawsetenvpkg/e2e/fixtures/providers/rawsetenv.yaml— Test fixtureRelated issue
resolves #13727