Skip to content

Conversation

@vvoland
Copy link
Collaborator

@vvoland vvoland commented Jul 11, 2023

- What I did
Handle error when parsing an invalid volume spec.

This was discovered when analyzing issue reported by @trungutt where
docker run -v $empty_var:/vol alpine would result in anonymous volume being mounted at /vol.

- How I did it

- How to verify it
CI tests:
unit: TestParseVolumeWithEmptySource
e2e: TestCreateWithEmptySourceVolume
e2e: TestCreateWithEmptyVolumeSpec

- Description for the changelog

  • Fix container being created with unexpected volume configuration when volume specification can't be parsed correctly

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

@vvoland vvoland added this to the 25.0.0 milestone Jul 11, 2023
@vvoland vvoland force-pushed the dont-ignore-volume-parse-err branch 2 times, most recently from 94f6d36 to 6ad527b Compare July 11, 2023 16:55
@codecov-commenter
Copy link

Codecov Report

Merging #4415 (6ad527b) into master (df04aca) will decrease coverage by 0.01%.
The diff coverage is 25.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4415      +/-   ##
==========================================
- Coverage   59.41%   59.40%   -0.01%     
==========================================
  Files         288      288              
  Lines       24776    24778       +2     
==========================================
- Hits        14720    14719       -1     
- Misses       9171     9173       +2     
- Partials      885      886       +1     

@thaJeztah
Copy link
Member

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the dont-ignore-volume-parse-err branch from 6ad527b to fe7afb7 Compare July 12, 2023 07:46
@vvoland
Copy link
Collaborator Author

vvoland commented Jul 12, 2023

Yes, this also fixes that issue:

$ ./build/docker run  -v "" alpine
docker: invalid volume specification: invalid empty volume spec.
See 'docker run --help'.

Added e2e test for it: TestCreateWithEmptyVolumeSpec

@vvoland vvoland marked this pull request as ready for review July 12, 2023 09:57
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ❤️

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.

Empty string "--volume" causes unhelpful error messages

3 participants