feat: thread --set/--set-value-file through deploy/dev/run/exec/verify and inject deploy params into action + verify containers - #10065
Conversation
There was a problem hiding this comment.
Code Review
This pull request extends the --set and --set-value-file flags to several additional commands, including deploy, dev, run, and exec. For the exec command specifically, these parameters are now injected as environment variables into Custom Action containers, mirroring Google Cloud Deploy's custom-target behavior. The changes include comprehensive documentation, a new example, and extensive unit and integration tests. Feedback identifies a misleading comment in the new test file, an inaccuracy in the documentation regarding flag availability for the filter command, and a potential performance optimization in the parameter merging logic.
22437a2 to
a45a2e5
Compare
The --set and --set-value-file flags were previously only available on render, filter, and delete. They are now also available on deploy, dev, run, exec, and verify so users can supply deploy parameters everywhere manifests, custom-action containers, or verify containers consume them. Values are already plumbed through the renderer for deploy/dev/run via runCtx.Opts.ManifestsOverrides / ManifestsValueFile; this change removes the CLI gate and updates the usage text. Follow-up commits inject them as environment variables into custom action and verify containers for Cloud Deploy parity, where deploy parameters are surfaced to those containers.
GetActionsRunner now merges --set-value-file and --set from the run context options into the env map threaded to the underlying exec environments (Docker and Kubernetes job). Every container invoked by a custom action therefore receives the supplied key/value pairs as environment variables, matching Cloud Deploy custom-target semantics where deploy parameters are surfaced to render and deploy containers. Precedence (lowest to highest): - --env-file (pre-existing base) - --set-value-file entries - --set entries (CLI wins, matches renderer merge order) The mergeDeployParams helper is unit-tested for the full precedence chain, malformed-entry skipping, and value-file error surfacing, plus a runner-level test asserting the merged map reaches the docker exec env.
GetVerifier now merges --set-value-file and --set into the env map that is threaded to both verify execution environments (local Docker and the Kubernetes Job), reusing the same mergeDeployParams helper as custom actions. Every verify container therefore receives deploy parameters as environment variables, matching Cloud Deploy, which automatically passes values such as the deployed Cloud Run service URL into verify containers. Precedence is identical to custom actions: --env-file base, then --set-value-file, then --set (highest priority).
Extends TestExec_LocalActions with an action-deploy-params case that invokes a custom action with --set DP_STRING / DP_REGION and asserts the values are echoed by the container, and adds TestLocalVerifyWithDeployParams which runs `skaffold verify --set FOO=from-set` against the verify-succeed fixture (whose alpine container echoes $FOO) and asserts the value reaches the verify container.
Updates the custom-actions "Availability across commands" section to list verify, adds a "Passing deploy parameters" section to the verify page covering --set / --set-value-file, precedence, and the Cloud Run service-URL parity use case, and regenerates the CLI reference so the flags appear on verify with the clarified help text.
A busybox show-params action that prints the deploy parameters forwarded by `skaffold exec --set` / `--set-value-file`. Mirrored under integration/examples/ per the repo's sample conventions.
a45a2e5 to
43c337d
Compare
|
Hi @Darien-Lin @menahyouyeah could you take a look please when you get a chance? Thank you |
Fixes #10128
Threads
--set/--set-value-filethroughdeploy,dev,run,exec, andverify, and injects those key/value pairs as environment variables into every custom-action container (exec) and verify container (verify). This matches Google Cloud Deploy, which surfaces deploy parameters — e.g. a Cloud SQL URL or the deployed Cloud Run service URL — to those containers.Changes
cmd/skaffold/app/cmd/flags.go— expose--set/--set-value-fileondeploy/dev/run/exec/verify.pkg/skaffold/runner/actions_runner.go—mergeDeployParamsmerges the params into the custom-action exec-env.pkg/skaffold/runner/verifier.go—GetVerifierreuses the same helper so verify containers receive them too.custom-actions-deploy-paramsexample.Precedence
--env-filebase <--set-value-file<--set(highest). On key collision with a container's ownenv:, the deploy parameter wins — matching Cloud Deploy.Tests
mergeDeployParamsprecedence/malformed/missing-file, the actions exec-env wiring, and flag presence/binding across all commands.skaffold exec --setandskaffold verify --setreach the container env.