validation: add hostname validation #72
Conversation
| @@ -163,9 +165,15 @@ func checkLinux(spec rspec.Linux, rootfs string) { | |||
| for index := 0; index < len(spec.Namespaces); index++ { | |||
| if !namespaceValid(spec.Namespaces[index]) { | |||
| logrus.Fatalf("Namespace %s is invalid.", spec.Namespaces[index]) | |||
| } else if spec.Namespces[index].Type == spec.UTSNamespace { | |||
| utsExist = true | |||
There was a problem hiding this comment.
s/spec.UTS/rspec.UTS
I think we don't need this variable, we can just popup an error here if hostname is empty.
Mashimiao
force-pushed
the
add-hostname-bundle-validation
branch
4 times, most recently
from
f32459e
to
82ba2b0
Compare
| @@ -163,6 +163,8 @@ func checkLinux(spec rspec.Linux, rootfs string) { | |||
| for index := 0; index < len(spec.Namespaces); index++ { | |||
| if !namespaceValid(spec.Namespaces[index]) { | |||
| logrus.Fatalf("Namespace %s is invalid.", spec.Namespaces[index]) | |||
| } else if spec.Namespaces[index].Type == rspec.UTSNamespace && rspec.Spec.Hostname != "" { | |||
There was a problem hiding this comment.
This probably needs the whole rspec.Spec, not its current rspec.Linux. Then you can check spec.Hostname and spec.Linux.Namespaces. With cross-platform options like ‘hostname’ depending on platform-specific options like UTS namespaces, this kind of cross-cutting is unavoidable.
Mashimiao
force-pushed
the
add-hostname-bundle-validation
branch
from
82ba2b0
to
9fe6309
Compare
| @@ -163,6 +163,8 @@ func checkLinux(spec rspec.Linux, rootfs string) { | |||
| for index := 0; index < len(spec.Namespaces); index++ { | |||
| if !namespaceValid(spec.Namespaces[index]) { | |||
| logrus.Fatalf("Namespace %s is invalid.", spec.Namespaces[index]) | |||
| } else if spec.Namespaces[index].Type == rspec.UTSNamespace && hostname != "" { | |||
There was a problem hiding this comment.
@Mashimiao sorry, your first commit is right.
On Linux, you can only set this if your bundle creates a new UTS namespace.
So host name could be empty even a config file has a UTS namespace.
We should get an error when hostname is empty and no UTS namespace is created.
There was a problem hiding this comment.
On Thu, May 19, 2016 at 08:30:00PM -0700, 梁辰晔 (Liang Chenye) wrote:
So host name could be empty even a config file has a UTS namespace.
Correct. Maybe the UTS namespace is for a NIS domain name.
We should get an error when hostname is empty and no UTS namespace
is created
I think you mean, when the hostname is not empty and no UTS
namespace is created. All other combinations (UTS and no hostname,
neither UTS nor hostname, both UTS and hostname) are valid.
There was a problem hiding this comment.
Yes, when hostname is not empty.
@Mashimiao 's first commit did the right thing and I mislead him to squash code lines.
|
add a WIP link in #66 |
Mashimiao
force-pushed
the
add-hostname-bundle-validation
branch
from
9fe6309
to
b62e1cc
Compare
|
@liangchenye @wking fixed. |
|
b62e1cc looks good to me.
|
|
LGTM |
| @@ -152,7 +152,9 @@ func checkProcess(process rspec.Process, rootfs string) { | |||
| } | |||
|
|
|||
| //Linux only | |||
| func checkLinux(spec rspec.Linux, rootfs string) { | |||
| func checkLinux(spec rspec.Linux, hostname string, rootfs string) { | |||
| utsExist := false | |||
There was a problem hiding this comment.
nit: utsExist --> utsExists
Mashimiao
force-pushed
the
add-hostname-bundle-validation
branch
from
b62e1cc
to
b625601
Compare
| } | ||
| } | ||
|
|
||
| if !utsExists && hostname != "" { | ||
| logrus.Fatalf("New UTS namespace should to be created.") |
There was a problem hiding this comment.
"hostname requires a new UTS namespace to be specified as well"
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Mashimiao
force-pushed
the
add-hostname-bundle-validation
branch
from
b625601
to
124c7ff
Compare
|
LGTM |
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com