publish: return api.ErrCanceled when user declines interactive prompts#13674
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes compose publish to return a detectable error when the user declines interactive safety prompts, instead of returning nil (success), so automation/callers can distinguish an abort from a successful publish.
Changes:
- Introduce a new sentinel error (
ErrPublishAborted) and return it when pre-check prompts are declined. - Add unit tests intended to cover “decline” paths for
preChecksandpublish.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/compose/publish.go | Returns a sentinel error when publish is aborted via interactive decline. |
| pkg/compose/publish_test.go | Adds tests for decline behavior in pre-checks and publish. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dc3c9cb to
579bbe7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Updated PR description to reflect the use of the existing api.ErrCanceled instead of creating a new custom error. |
glours
left a comment
There was a problem hiding this comment.
Thanks for the contribution 🙏
I have small requests before we can merge it
| }, cmp.Ignore())) | ||
| } | ||
|
|
||
| func Test_preChecks_decline_returns_ErrPublishAborted(t *testing.T) { |
There was a problem hiding this comment.
There isn’t any ErrPublishedArboted error in the codebase, so the test name is misleading.
Second, this test is effectively a subset of Test_publish_decline_returns_ErrCanceled.
We can keep it, but I’d suggest repurposing it to cover the sensitive data detection path instead. That would make it exercise an uncovered code path and give it a clearer purpose.
|
Thanks for the feedback! I've repurposed the test to cover the checkForSensitiveData + decline path using a temp .env file with a dummy AWS secret key. The test name is updated to reflect what it actually tests. |
|
@ishwar170695 can you sing-off your commits please |
|
@ishwar170695 CI detected tests which need to be changed to check properly the new 130 exit code |
Signed-off-by: Ishwar <ishwarcm@iitbhilai.ac.in>
Renames test to Test_preChecks_sensitive_data_detected_decline. Uses a temporary .env file with an AWS token to reliably trigger the DefangLabs secret detector, and confirms that preChecks correctly aborts early on user decline. Signed-off-by: Ishwar <ishwarcm@iitbhilai.ac.in>
Signed-off-by: Ishwar <ishwarcm@iitbhilai.ac.in>
Head branch was pushed to by a user without write access
c8d190e to
df5d68b
Compare
|
Sorry for disabling auto-merge, the force-push after rebase triggered that. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | patch | `v5.1.1` → `v5.1.2` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v5.1.2`](https://github.com/docker/compose/releases/tag/v5.1.2) [Compare Source](docker/compose@v5.1.1...v5.1.2) #### What's Changed ##### 🐛 Fixes - Fix TTY timer rendering when duration length changes by [@​MaybeSam05](https://github.com/MaybeSam05) in [#​13634](docker/compose#13634) - Fix up attach filtering by [@​false200](https://github.com/false200) in [#​13664](docker/compose#13664) - Preserve ssh:// URL scheme when resolving Dockerfile path by [@​ssam18](https://github.com/ssam18) in [#​13669](docker/compose#13669) - Initialize and pass envFiles map in processExtends by [@​Mohamed-Moumni](https://github.com/Mohamed-Moumni) in [#​13678](docker/compose#13678) - Fix TestRunHook\_ConsoleSize on macOS by [@​thaJeztah](https://github.com/thaJeztah) in [#​13686](docker/compose#13686) - Restore post-connect fallback for multi-network stacks on API < 1.44 by [@​jotka](https://github.com/jotka) in [#​13629](docker/compose#13629) - Publish: return api.ErrCanceled when user declines interactive prompts by [@​ishwar170695](https://github.com/ishwar170695) in [#​13674](docker/compose#13674) - Return error on non-ErrNotExist stat failures in Tar.Sync() by [@​Lidang-Jiang](https://github.com/Lidang-Jiang) in [#​13684](docker/compose#13684) ##### 🔧 Internal - Refactor: thread context through publish sensitive data check by [@​ishwar170695](https://github.com/ishwar170695) in [#​13653](docker/compose#13653) - Add AI-powered MR review workflow via `docker/cagent-action` by [@​glours](https://github.com/glours) in [#​13659](docker/compose#13659) - Update `cagent-action` to latest (with better permissions) by [@​derekmisler](https://github.com/derekmisler) in [#​13665](docker/compose#13665) - Pin GitHub Actions to commit SHA, remove pr-review workflow by [@​glours](https://github.com/glours) in [#​13662](docker/compose#13662) - Exclude hook\_test.go from Windows builds and propagate ExecStart error in runWaitExec by [@​pawannn](https://github.com/pawannn) in [#​13683](docker/compose#13683) - Skip MR review workflow for Dependabot MRs by [@​glours](https://github.com/glours) in [#​13679](docker/compose#13679) - Use negotiated API version for network setup by [@​glours](https://github.com/glours) in [#​13690](docker/compose#13690) - Fix mixed assertion libraries in tests by [@​thaJeztah](https://github.com/thaJeztah) in [#​13689](docker/compose#13689) - Test: use random host port for dind TLS build test by [@​ricardobranco777](https://github.com/ricardobranco777) in [#​13630](docker/compose#13630) - Remove direct dependency on `docker/docker` by [@​glours](https://github.com/glours) in [#​13706](docker/compose#13706) ##### ⚙️ Dependencies - Bump github.com/containerd/platforms from `1.0.0-rc.2` to `1.0.0-rc.3` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13657](docker/compose#13657) - Bump golangci-lint to `v2.11.3` and configure CLAUDE to use it on change by [@​ndeloof](https://github.com/ndeloof) in [#​13656](docker/compose#13656) - Bump google.golang.org/grpc from `1.78.0` to `1.79.3` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13642](docker/compose#13642) - Bump github.com/moby/patternmatcher from `0.6.0` to `0.6.1` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13667](docker/compose#13667) - Bump go.opentelemetry.io/otel/sdk from `1.39.0` to `1.42.0` by [@​glours](https://github.com/glours) in [#​13663](docker/compose#13663) - Bump github.com/docker/cli from `29.2.1+incompatible` to `29.3.1+incompatible` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13670](docker/compose#13670) - Bump github.com/hashicorp/go-version from `1.8.0` to `1.9.0` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13692](docker/compose#13692) - Bump github.com/docker/buildx `v0.33.0`, buildkit `v0.29.0` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13693](docker/compose#13693) - Bump google.golang.org/grpc from `1.79.3` to `1.80.0` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13697](docker/compose#13697) - Bump github.com/containerd/platforms from `1.0.0-rc.3` to `1.0.0-rc.4` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13696](docker/compose#13696) - Bump github.com/moby/moby/client `v0.4.0`, moby/api `v1.54.1` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13708](docker/compose#13708) - Bump github.com/docker/cli `v29.4.0` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13707](docker/compose#13707) - Bump compose-go to version `v2.10.2` by [@​glours](https://github.com/glours) in [#​13705](docker/compose#13705) - Bump to Go `1.25.9` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13720](docker/compose#13720) #### New Contributors - [@​MaybeSam05](https://github.com/MaybeSam05) made their first contribution in [#​13634](docker/compose#13634) - [@​ishwar170695](https://github.com/ishwar170695) made their first contribution in [#​13653](docker/compose#13653) - [@​derekmisler](https://github.com/derekmisler) made their first contribution in [#​13665](docker/compose#13665) - [@​false200](https://github.com/false200) made their first contribution in [#​13664](docker/compose#13664) - [@​ssam18](https://github.com/ssam18) made their first contribution in [#​13669](docker/compose#13669) - [@​Mohamed-Moumni](https://github.com/Mohamed-Moumni) made their first contribution in [#​13678](docker/compose#13678) - [@​pawannn](https://github.com/pawannn) made their first contribution in [#​13683](docker/compose#13683) - [@​jotka](https://github.com/jotka) made their first contribution in [#​13629](docker/compose#13629) - [@​Lidang-Jiang](https://github.com/Lidang-Jiang) made their first contribution in [#​13684](docker/compose#13684) **Full Changelog**: <docker/compose@v5.1.1...v5.1.2> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMTAuOCIsInVwZGF0ZWRJblZlciI6IjQzLjExMC44IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6cGF0Y2giXX0=-->
What I did
Previously, declining the bind mount or sensitive data warning prompt caused publish() to return
nil, which is indistinguishable from success for callers and automation scripts (exit code 0).Change publish() to return the existing
api.ErrCanceledsentinel so callers can detect user-initiated cancellation viaerrors.Is(). Add unit tests covering both thepreChecksdecline path and the fullpublish()path.Signed-off-by: Ishwar chand meena ishwarcm@iitbhilai.ac.in
(not mandatory) A picture of a cute animal, if possible in relation to what you did