Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[27.1 backport] fix: ctx cancellation on login prompt #5260

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

thaJeztah
Copy link
Member

- What I did

Implemented a new Prompt function to capture user input called PromptForInput. This function works virtually similar to PromptForConfirmation and handles context cancellation.

- How I did it

Replicate PromptForConfirmation.

- How to verify it
Run docker login and cancel it midway with a SIGINT or SIGTERM or ctrl+c.

docker login

Username:^C

PromptForInput also allows for hiding user input - for example when the user enters their password.

The original behavior of docker login should still be in-tact with trimming spaces and not showing user input on the password prompt.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Alano Terblanche <[email protected]>
(cherry picked from commit c15ade0)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah added this to the 27.0.4 milestone Jul 18, 2024
@thaJeztah thaJeztah self-assigned this Jul 18, 2024
@thaJeztah thaJeztah closed this Jul 19, 2024
@thaJeztah thaJeztah reopened this Jul 19, 2024
@vvoland vvoland closed this Jul 19, 2024
@vvoland vvoland reopened this Jul 19, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 43.75000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 61.44%. Comparing base (69a2c9f) to head (4d67ef0).
Report is 12 commits behind head on 27.0.

Additional details and impacted files
@@            Coverage Diff             @@
##             27.0    #5260      +/-   ##
==========================================
+ Coverage   61.37%   61.44%   +0.06%     
==========================================
  Files         295      298       +3     
  Lines       20811    20826      +15     
==========================================
+ Hits        12773    12796      +23     
+ Misses       7119     7115       -4     
+ Partials      919      915       -4     

@thaJeztah thaJeztah merged commit 7e972d2 into docker:27.0 Jul 19, 2024
101 checks passed
@thaJeztah thaJeztah deleted the 27.1_backport_fix-cli-login branch July 19, 2024 10:05
@thaJeztah thaJeztah changed the title [27.0 backport] fix: ctx cancellation on login prompt [27.1 backport] fix: ctx cancellation on login prompt Jul 19, 2024
renovate bot added a commit to earthly/dind that referenced this pull request Jul 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://togithub.com/docker/docker) | minor | `27.0.3`
-> `27.1.0` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

### [`v27.1.0`](https://togithub.com/moby/moby/releases/tag/v27.1.0)

[Compare
Source](https://togithub.com/docker/docker/compare/v27.0.3...v27.1.0)

#### 27.1.0

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.1.0
milestone](https://togithub.com/docker/cli/issues?q=is%3Aclosed+milestone%3A27.1.0)
- [moby/moby, 27.1.0
milestone](https://togithub.com/moby/moby/issues?q=is%3Aclosed+milestone%3A27.1.0)
- Deprecated and removed features, see [Deprecated
Features](https://togithub.com/docker/cli/blob/v27.1.0/docs/deprecated.md).
- Changes to the Engine API, see [API version
history](https://togithub.com/moby/moby/blob/v27.1.0/docs/api/version-history.md).

##### Bug fixes and enhancements

- rootless: add `Requires=dbus.socket` to prevent errors when starting
the daemon on a cgroup v2 host with systemd
[moby/moby#48141](https://togithub.com/moby/moby/pull/48141)
- containerd integration: `image tag` event is now properly emitted when
building images with Buildkit
[moby/moby#48182](https://togithub.com/moby/moby/pull/48182)
- cli: add OOMScoreAdj to docker service create and docker stack
[docker/cli#5274](https://togithub.com/docker/cli/pull/5274)
- cli: add support for `DOCKER_CUSTOM_HEADERS` env-var (experimental)
[docker/cli#5271](https://togithub.com/docker/cli/pull/5271)
- cli: containerd-integration: Fix `docker push` defaulting the
`--platform` flag to a value of `DOCKER_DEFAULT_PLATFORM` environment
variable on unsupported API versions
[docker/cli#5248](https://togithub.com/docker/cli/pull/5248)
- cli: fix: ctx cancellation on login prompt
[docker/cli#5260](https://togithub.com/docker/cli/pull/5260)
- cli: fix: wait for the container to exit before closing the stream
when sending a termination request to the CLI while attached to a
container [docker/cli#5250](https://togithub.com/docker/cli/pull/5250)

##### Deprecated

- the pkg/rootless/specconv package is deprecated, an will be removed in
the next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- the pkg/containerfs package is deprecated, an will be removed in the
next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- the pkg/directory package is deprecated, an will be removed in the
next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- api/types/system: remove deprecated Info.ExecutionDriver
[moby/moby#48184](https://togithub.com/moby/moby/pull/48184)

##### Packaging updates

- Update Buildx to
[v0.16.1](https://togithub.com/docker/buildx/releases/tag/v0.16.1).
[moby/docker-ce-packaging#1039](https://togithub.com/docker/docker-ce-packaging/pull/1039)
- Update Compose to
[v2.29.0](https://togithub.com/docker/compose/releases/tag/v2.29.0).
[moby/docker-ce-packaging#1038](https://togithub.com/docker/docker-ce-packaging/pull/1038)
- Update Containerd (static binaries only) to
[v1.7.20](https://togithub.com/containerd/containerd/releases/tag/v1.7.20).
[moby/moby#48191](https://togithub.com/moby/moby/pull/48191)
- Update BuildKit to
[v0.15.0](https://togithub.com/moby/buildkit/releases/tag/v0.15.0).
[moby/moby#48028](48175://togithub.com/moby/moby/pull/48175)
- Update Go runtime to 1.21.12, which contains security fixes for
[CVE-2024-24791](https://togithub.com/advisories/GHSA-hw49-2p59-3mhj)
[moby/moby#48120](48175://togithub.com/moby/moby/pull/48120)

**Full Changelog**:
moby/moby@v27.0.3...v27.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to earthly/dind that referenced this pull request Jul 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://togithub.com/docker/docker) | minor | `27.0.3`
-> `27.1.0` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

### [`v27.1.0`](https://togithub.com/moby/moby/releases/tag/v27.1.0)

[Compare
Source](https://togithub.com/docker/docker/compare/v27.0.3...v27.1.0)

#### 27.1.0

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.1.0
milestone](https://togithub.com/docker/cli/issues?q=is%3Aclosed+milestone%3A27.1.0)
- [moby/moby, 27.1.0
milestone](https://togithub.com/moby/moby/issues?q=is%3Aclosed+milestone%3A27.1.0)
- Deprecated and removed features, see [Deprecated
Features](https://togithub.com/docker/cli/blob/v27.1.0/docs/deprecated.md).
- Changes to the Engine API, see [API version
history](https://togithub.com/moby/moby/blob/v27.1.0/docs/api/version-history.md).

##### Bug fixes and enhancements

- rootless: add `Requires=dbus.socket` to prevent errors when starting
the daemon on a cgroup v2 host with systemd
[moby/moby#48141](https://togithub.com/moby/moby/pull/48141)
- containerd integration: `image tag` event is now properly emitted when
building images with Buildkit
[moby/moby#48182](https://togithub.com/moby/moby/pull/48182)
- cli: add OOMScoreAdj to docker service create and docker stack
[docker/cli#5274](https://togithub.com/docker/cli/pull/5274)
- cli: add support for `DOCKER_CUSTOM_HEADERS` env-var (experimental)
[docker/cli#5271](https://togithub.com/docker/cli/pull/5271)
- cli: containerd-integration: Fix `docker push` defaulting the
`--platform` flag to a value of `DOCKER_DEFAULT_PLATFORM` environment
variable on unsupported API versions
[docker/cli#5248](https://togithub.com/docker/cli/pull/5248)
- cli: fix: ctx cancellation on login prompt
[docker/cli#5260](https://togithub.com/docker/cli/pull/5260)
- cli: fix: wait for the container to exit before closing the stream
when sending a termination request to the CLI while attached to a
container [docker/cli#5250](https://togithub.com/docker/cli/pull/5250)

##### Deprecated

- the pkg/rootless/specconv package is deprecated, an will be removed in
the next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- the pkg/containerfs package is deprecated, an will be removed in the
next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- the pkg/directory package is deprecated, an will be removed in the
next release
[moby/moby#48185](https://togithub.com/moby/moby/pull/48185)
- api/types/system: remove deprecated Info.ExecutionDriver
[moby/moby#48184](https://togithub.com/moby/moby/pull/48184)

##### Packaging updates

- Update Buildx to
[v0.16.1](https://togithub.com/docker/buildx/releases/tag/v0.16.1).
[moby/docker-ce-packaging#1039](https://togithub.com/docker/docker-ce-packaging/pull/1039)
- Update Compose to
[v2.29.0](https://togithub.com/docker/compose/releases/tag/v2.29.0).
[moby/docker-ce-packaging#1038](https://togithub.com/docker/docker-ce-packaging/pull/1038)
- Update Containerd (static binaries only) to
[v1.7.20](https://togithub.com/containerd/containerd/releases/tag/v1.7.20).
[moby/moby#48191](https://togithub.com/moby/moby/pull/48191)
- Update BuildKit to
[v0.15.0](https://togithub.com/moby/buildkit/releases/tag/v0.15.0).
[moby/moby#48175](https://togithub.com/moby/moby/pull/48175)
- Update Go runtime to 1.21.12, which contains security fixes for
[CVE-2024-24791](https://togithub.com/advisories/GHSA-hw49-2p59-3mhj)
[moby/moby#48120](48175://togithub.com/moby/moby/pull/48120)

**Full Changelog**:
moby/moby@v27.0.3...v27.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants