Uh oh!
There was an error while loading. Please reload this page.
runtimetest: add readonly path validation - #76
Conversation
wking
commented
May 25, 2016
On Tue, May 24, 2016 at 08:52:20PM -0700, Ma Shimiao wrote:
Travis is missing ‘spec’ in main.go around line 194 1. |
| return nil | ||
| } | ||
| func validateROPaths(spec, *rspec.Spec) error { |
7a11ef4 to
bee2090Comparewking
commented
May 25, 2016
On Tue, May 24, 2016 at 11:11:04PM -0700, Ma Shimiao wrote:
Ah, that's the piece I was forgetting. This looks good to me then. |
| } | ||
| func validateROPaths(spec *rspec.Spec) error { | ||
| fmt.Println("validating Readonly Paths") |
There was a problem hiding this comment.
using 'validating readonlyPaths' as Mrunal suggested match what is expected to be the property name.
bee2090 to
d55b619CompareMashimiao
commented
May 26, 2016
@liangchenye fixed. |
liangchenye
commented
May 26, 2016
LGTM |
| func validateROPaths(spec *rspec.Spec) error { | ||
| fmt.Println("validating readonlyPaths") | ||
| for _, v := range spec.Linux.ReadonlyPaths { | ||
| fi, err := os.Stat(v) |
There was a problem hiding this comment.
This stat-based approach should be changed to an attempted-write approach.
There was a problem hiding this comment.
And it's probably worth making “is this path readonly?” a separate function so it can be shared with the check in #81.
There was a problem hiding this comment.
@wking The spec says "readonlyPaths will set the provided paths as readonly inside the container."
It seems not to remount readonlyPaths and just change their permisson inside container. If this is true, I think current implementation is OK.
There was a problem hiding this comment.
On Thu, May 26, 2016 at 07:30:15PM -0700, Ma Shimiao wrote:
It seems not to remount readonlyPaths and just change their
permisson.
I think the spec wording leaves the implemenation ambiguous. And
attempting a read clearly covers all possible implementations, while
checking the mode only covers a single implementation.
This has the same problem as #78 -- mounting something readonly doesn't change the mode. We should check that it is actually readonly by attempting a write and making sure it fails with |
d55b619 to
3c250a9Compareliangchenye
commented
Jun 1, 2016
3c250a9 LGTM |
cyphar
commented
Jun 1, 2016
IANAM, but LGTM 3c250a9. |
mrunalp
commented
Jun 1, 2016
Looks good but needs rebase. |
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
3c250a9 to
447a73bCompareMashimiao
commented
Jun 2, 2016
@mrunalp rebased. |
mrunalp
commented
Jun 2, 2016
LGTM |
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com