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

volume: move validating volume dest from client to server. #11231

Merged
merged 3 commits into from
Aug 17, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,10 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
}

if err = parse.ValidateVolumeCtrDir(vol.Dest); err != nil {
return errors.Wrapf(err, "error validating destination path for named volume %q mounted at %q", vol.Name, vol.Dest)
Copy link
Member

Choose a reason for hiding this comment

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

This looks like you created a stutter. Does vol.Dest get mentioned in the err twice?
Don't start error messages with the word "error" otherwise they are printed on error with

Error: error ...

I know there are other errors in options.go with this problem, please fix them while you are at it.

I would like to eventually go through the error messages in all of podman and remove any that begin with "error".

Also see what this error message looks like when it happens. Is this information the user needs or does it hide the error amongst a flood of words.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes this adds stutter i verified let me fix this :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rhatdan resolved for options.go i'll create a seperate PR cleaning this in entire project.

}

ctr.config.NamedVolumes = append(ctr.config.NamedVolumes, &ContainerNamedVolume{
Name: vol.Name,
Dest: vol.Dest,
Expand Down
3 changes: 0 additions & 3 deletions pkg/specgen/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
return nil, nil, nil, errors.New("host directory cannot be empty")
}
}
if err := parse.ValidateVolumeCtrDir(dest); err != nil {
return nil, nil, nil, err
}

cleanDest := filepath.Clean(dest)

Expand Down