runtimetest: add posixValidations #510
Conversation
cmd/runtimetest/main.go
Outdated
| return spec, nil | ||
| } | ||
|
|
||
| // should be included by other platform specified process validation |
There was a problem hiding this comment.
The “fix process validation” commit should remove this comment too.
cmd/runtimetest/main.go
Outdated
| } | ||
| func validateUser(spec *rspec.Spec) error { | ||
| if runtime.GOOS == "windows" { | ||
| return nil |
There was a problem hiding this comment.
Perhaps this function should be validatePOSIXUser to match the spec and then used in both linuxValidatations and a new solarisValidations? Or a new posixValidations which is included for both linux and solaris platforms? The spec has Windows-specific user stuff, but all of its RFC 2119 language is currently config-oriented and covered by the JSON Schema validation.
There was a problem hiding this comment.
Or a new posixValidations which is included for both linux and solaris platforms?
I think it would be better. I'm going to do that.
Signed-off-by: Zhou Hao <[email protected]>
zhouhao3
force-pushed
the
user-check-runtiemtest
branch
from
6c9942a to
12076d7
Compare
Signed-off-by: Zhou Hao <[email protected]>
zhouhao3
force-pushed
the
user-check-runtiemtest
branch
from
12076d7 to
45b2686
Compare
zhouhao3
changed the title
zhouhao3
force-pushed
the
user-check-runtiemtest
branch
from
6a6fee7 to
84a0824
Compare
cmd/runtimetest/main.go
Outdated
| return spec, nil | ||
| } | ||
|
|
||
| func validateUser(spec *rspec.Spec) error { |
There was a problem hiding this comment.
This should be validatePosixUser to make it extra-clear that it's not covering Windows users.
|
|
||
| func validateMounts(spec *rspec.Spec) error { | ||
| if runtime.GOOS == "windows" { | ||
| logrus.Warnf("mounts validation not yet implemented for OS %q", runtime.GOOS) |
There was a problem hiding this comment.
We want to keep this. mounts is specified for all platforms, and we just aren't testing the Windows-specific parts of that yet.
There was a problem hiding this comment.
I think this can change to validatePosixMounts. Because this is only validated against POSIX-platform Mounts.
There was a problem hiding this comment.
I think this can change to
validatePosixMounts. Because this is only validated against POSIX-platform Mounts.
It's also checking the mount order, which is a cross-platform requirement. Windows checks for mount order could involve extending cmd/runtimetest/mount to support Windows, in which case we'd want to keep this valdation generic, with per-platform guards for the POSIX-specific type checks, etc. Or we can duplicate the order checks in a separate validateWindowsMounts, in which case calling all of the current logic validatePosixMounts is fine.
Signed-off-by: Zhou Hao <[email protected]>
zhouhao3
force-pushed
the
user-check-runtiemtest
branch
from
84a0824 to
8765570
Compare
|
LGTM |
1 similar comment
|
LGTM |
4 participants
Signed-off-by: Zhou Hao [email protected]