The Wayback Machine - https://web.archive.org/web/20260502020300/https://github.com/docker/cli/pull/824
Skip to content

added check for empty source in bind mount#824

Merged
dnephin merged 1 commit intodocker:masterfrom
ethan-haynes:820-bind-mount-source-missing-error
Feb 21, 2018
Merged

added check for empty source in bind mount#824
dnephin merged 1 commit intodocker:masterfrom
ethan-haynes:820-bind-mount-source-missing-error

Conversation

@ethan-haynes
Copy link
Copy Markdown

@ethan-haynes ethan-haynes commented Jan 21, 2018

Provides a fix for #820
fixes #820

- What I did
Fixed the bug that allowed a user to use a bind mount without a source in the compose file. Without providing a source the loader would add the path of the compose file.
- How I did it
Added error handing and explicit check for empty source in compose file. The loader now throws an error if it detects a bind mount without a source.
- How to verify it

1: make a docker-compose.yml file

version: "3.4"
services:
  nginx:
    image: nginx:alpine
    volumes:
      - type: bind
        target: /app

2: run stack deploy commands

$: docker stack deploy -c docker-compose.yml nginx

The output should be the following error:
invalid mount config for type "bind": field Source must not be empty

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 21, 2018

Codecov Report

Merging #824 into master will decrease coverage by 1.25%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #824      +/-   ##
=========================================
- Coverage   52.95%   51.7%   -1.26%     
=========================================
  Files         244     244              
  Lines       15828   15827       -1     
=========================================
- Hits         8382    8183     -199     
- Misses       6892    7100     +208     
+ Partials      554     544      -10

@ethan-haynes ethan-haynes force-pushed the 820-bind-mount-source-missing-error branch from a4f863b to 28b7d94 Compare January 21, 2018 05:51
Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Small nits

Comment thread cli/compose/loader/loader.go Outdated
}

if volume.Source == "" {
return errors.New("invalid mount config for type \"bind\": field Source must not be empty")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return errors.New(`invalid mount config for type "bind": field Source must not be empty`)

is simpler 😄

Comment thread cli/compose/loader/loader_test.go Outdated
- type: bind
target: /app
`)
require.Error(t, err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

require.EqualError(t, err, `invalid mount config for type "bind": field Source must not be empty`)

Signed-off-by: Ethan Haynes <ethanhaynes@alumni.harvard.edu>

fixed error by removing punctuation

Signed-off-by: Ethan Haynes <ethanhaynes@alumni.harvard.edu>

removed period

Signed-off-by: Ethan Haynes <ethanhaynes@alumni.harvard.edu>

backtick string to escape double quotes in error messages

Signed-off-by: Ethan Haynes <ethanhaynes@alumni.harvard.edu>

simplified test for bind no source error message

Signed-off-by: Ethan Haynes <ethanhaynes@alumni.harvard.edu>
@ethan-haynes ethan-haynes force-pushed the 820-bind-mount-source-missing-error branch from 1bafe67 to e76d8c9 Compare January 23, 2018 00:34
Copy link
Copy Markdown
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

Copy link
Copy Markdown
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🌮
cc @dnephin

@dnephin dnephin merged commit cea4d37 into docker:master Feb 21, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.03.0 milestone Feb 21, 2018
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
…-missing-error

added check for empty source in bind mount
Upstream-commit: cea4d37
Component: cli
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.

path to compose file being set as source for bind mount when source is not provided in compose file

7 participants