-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
add cli integration for masked and readonly paths #1347
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,8 @@ type containerOptions struct { | |
storageOpt opts.ListOpts | ||
labelsFile opts.ListOpts | ||
loggingOpts opts.ListOpts | ||
maskedPaths opts.ListOpts | ||
readonlyPaths opts.ListOpts | ||
privileged bool | ||
pidMode string | ||
utsMode string | ||
|
@@ -150,7 +152,9 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { | |
linkLocalIPs: opts.NewListOpts(nil), | ||
links: opts.NewListOpts(opts.ValidateLink), | ||
loggingOpts: opts.NewListOpts(nil), | ||
maskedPaths: opts.NewListOpts(nil), | ||
publish: opts.NewListOpts(nil), | ||
readonlyPaths: opts.NewListOpts(nil), | ||
securityOpt: opts.NewListOpts(nil), | ||
storageOpt: opts.NewListOpts(nil), | ||
sysctls: opts.NewMapOpts(nil, opts.ValidateSysctl), | ||
|
@@ -183,6 +187,8 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { | |
flags.StringVarP(&copts.user, "user", "u", "", "Username or UID (format: <name|uid>[:<group|gid>])") | ||
flags.StringVarP(&copts.workingDir, "workdir", "w", "", "Working directory inside the container") | ||
flags.BoolVar(&copts.autoRemove, "rm", false, "Automatically remove the container when it exits") | ||
flags.Var(&copts.maskedPaths, "masked-paths", "Denote the paths to be masked for the container, empty implies none") | ||
flags.Var(&copts.readonlyPaths, "readonly-paths", "Denote the paths to be set as readonly for the container, empty implies none") | ||
|
||
// Security | ||
flags.Var(&copts.capAdd, "cap-add", "Add Linux capabilities") | ||
|
@@ -471,6 +477,16 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err | |
return nil, err | ||
} | ||
|
||
maskedPathOpts, err := parsePaths(copts.maskedPaths.GetAll()) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
readonlyPathOpts, err := parsePaths(copts.readonlyPaths.GetAll()) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Healthcheck | ||
var healthConfig *container.HealthConfig | ||
haveHealthSettings := copts.healthCmd != "" || | ||
|
@@ -614,6 +630,8 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err | |
Sysctls: copts.sysctls.GetAll(), | ||
Runtime: copts.runtime, | ||
Mounts: mounts, | ||
MaskedPaths: maskedPathOpts, | ||
ReadonlyPaths: readonlyPathOpts, | ||
} | ||
|
||
if copts.autoRemove && !hostConfig.RestartPolicy.IsNone() { | ||
|
@@ -777,6 +795,18 @@ func parseDevice(device string) (container.DeviceMapping, error) { | |
return deviceMapping, nil | ||
} | ||
|
||
func parsePaths(paths []string) ([]string, error) { | ||
for _, p := range paths { | ||
if p == "none" { | ||
if len(paths) > 1 { | ||
return nil, errors.New("Passing 'none' and other paths is not allowed") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: error string should not be capitalized |
||
} | ||
return []string{}, nil | ||
} | ||
} | ||
return paths, nil | ||
} | ||
|
||
// validateDeviceCgroupRule validates a device cgroup rule string format | ||
// It will make sure 'val' is in the form: | ||
// 'type major:minor mode' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,19 @@ func TestRunWithContentTrust(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestRunMaskedPaths(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WIP? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Especially we need to make sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah was about to add tests but waited also im confused as to if the tests from docker/docker will be moved here too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jessfraz new tests from |
||
result := icmd.RunCmd( | ||
icmd.Command("docker", "run", image), | ||
fixtures.WithConfig(dir.Path()), | ||
fixtures.WithTrust, | ||
fixtures.WithNotary, | ||
) | ||
result.Assert(t, icmd.Expected{ | ||
Err: fmt.Sprintf("Tagging %s@sha", image[:len(image)-7]), | ||
}) | ||
|
||
} | ||
|
||
// TODO: create this with registry API instead of engine API | ||
func createRemoteImage(t *testing.T) string { | ||
image := "registry:5000/alpine:test-run-pulls" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty implies default, not "none"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It implies none, of "paths to be masked for the containers"