github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-21 19:01:49 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #358 from thockin/valid2

Browse Source 
Add validation of VolumeMounts
This commit is contained in:
brendandburns 2014-07-08 21:58:32 -07:00
commit 242627eaf0
5 changed files with 255 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pkg/api/types.go
View File

@ -83,7 +83,10 @@ type VolumeMount struct {
// Optional: Defaults to false (read-write).
ReadOnly bool `yaml:"readOnly,omitempty" json:"readOnly,omitempty"`
// Required.
// Exactly one of the following must be set. If both are set, prefer MountPath.
// DEPRECATED: Path will be removed in a future version of the API.
MountPath string `yaml:"mountPath,omitempty" json:"mountPath,omitempty"`
Path string `yaml:"path,omitempty" json:"path,omitempty"`
// One of: "LOCAL" (local volume) or "HOST" (external mount from the host). Default: LOCAL.
MountType string `yaml:"mountType,omitempty" json:"mountType,omitempty"`
}

105
pkg/api/validation.go
View File

@ -18,15 +18,12 @@ package api
import (
"fmt"
"strings"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/golang/glog"
)
var (
supportedManifestVersions = util.NewStringSet("v1beta1", "v1beta2")
)
// ValidationErrorEnum is a type of validation error.
type ValidationErrorEnum string
@ -35,6 +32,7 @@ const (
ErrTypeInvalid ValidationErrorEnum = "invalid value"
ErrTypeNotSupported ValidationErrorEnum = "unsupported value"
ErrTypeDuplicate ValidationErrorEnum = "duplicate value"
ErrTypeNotFound ValidationErrorEnum = "not found"
)
// ValidationError is an implementation of the 'error' interface, which represents an error of validation.
@ -61,6 +59,10 @@ func makeDuplicateError(field string, value interface{}) ValidationError {
return ValidationError{ErrTypeDuplicate, field, value}
}
func makeNotFoundError(field string, value interface{}) ValidationError {
return ValidationError{ErrTypeNotFound, field, value}
}
func validateVolumes(volumes []Volume) (util.StringSet, error) {
allNames := util.StringSet{}
for i := range volumes {
@ -76,6 +78,38 @@ func validateVolumes(volumes []Volume) (util.StringSet, error) {
return allNames, nil
}
var supportedPortProtocols util.StringSet = util.NewStringSet("TCP", "UDP")
func validatePorts(ports []Port) error {
allNames := util.StringSet{}
for i := range ports {
port := &ports[i] // so we can set default values
if len(port.Name) > 0 {
if len(port.Name) > 63 || !util.IsDNSLabel(port.Name) {
return makeInvalidError("Port.Name", port.Name)
}
if allNames.Has(port.Name) {
return makeDuplicateError("Port.name", port.Name)
}
allNames.Insert(port.Name)
}
if !util.IsValidPortNum(port.ContainerPort) {
return makeInvalidError("Port.ContainerPort", port.ContainerPort)
}
if port.HostPort == 0 {
port.HostPort = port.ContainerPort
} else if !util.IsValidPortNum(port.HostPort) {
return makeInvalidError("Port.HostPort", port.HostPort)
}
if len(port.Protocol) == 0 {
port.Protocol = "TCP"
} else if !supportedPortProtocols.Has(strings.ToUpper(port.Protocol)) {
return makeNotSupportedError("Port.Protocol", port.Protocol)
}
}
return nil
}
func validateEnv(vars []EnvVar) error {
for i := range vars {
ev := &vars[i] // so we can set default values
@ -95,6 +129,50 @@ func validateEnv(vars []EnvVar) error {
return nil
}
func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) error {
for i := range mounts {
mnt := &mounts[i] // so we can set default values
if len(mnt.Name) == 0 {
return makeInvalidError("VolumeMount.Name", mnt.Name)
}
if !volumes.Has(mnt.Name) {
return makeNotFoundError("VolumeMount.Name", mnt.Name)
}
if len(mnt.MountPath) == 0 {
// Backwards compat.
if len(mnt.Path) == 0 {
return makeInvalidError("VolumeMount.MountPath", mnt.MountPath)
}
glog.Warning("DEPRECATED: VolumeMount.Path has been replaced by VolumeMount.MountPath")
mnt.MountPath = mnt.Path
mnt.Path = ""
}
}
return nil
}
// AccumulateUniquePorts runs an extraction function on each Port of each Container,
// accumulating the results and returning an error if any ports conflict.
func AccumulateUniquePorts(containers []Container, accumulator map[int]bool, extract func(*Port) int) error {
for ci := range containers {
ctr := &containers[ci]
for pi := range ctr.Ports {
port := extract(&ctr.Ports[pi])
if accumulator[port] {
return makeDuplicateError("Port", port)
}
accumulator[port] = true
}
}
return nil
}
// Checks for colliding Port.HostPort values across a slice of containers.
func checkHostPortConflicts(containers []Container) error {
allPorts := map[int]bool{}
return AccumulateUniquePorts(containers, allPorts, func(p *Port) int { return p.HostPort })
}
func validateContainers(containers []Container, volumes util.StringSet) error {
allNames := util.StringSet{}
for i := range containers {
@ -109,15 +187,26 @@ func validateContainers(containers []Container, volumes util.StringSet) error {
if len(ctr.Image) == 0 {
return makeInvalidError("Container.Image", ctr.Name)
}
if err := validatePorts(ctr.Ports); err != nil {
return err
}
if err := validateEnv(ctr.Env); err != nil {
return err
}
// TODO(thockin): finish validation.
if err := validateVolumeMounts(ctr.VolumeMounts, volumes); err != nil {
return err
}
}
return nil
// Check for colliding ports across all containers.
// TODO(thockin): This really is dependent on the network config of the host (IP per pod?)
// and the config of the new manifest. But we have not specced that out yet, so we'll just
// make some assumptions for now. As of now, pods share a network namespace, which means that
// every Port.HostPort across the whole pod must be unique.
return checkHostPortConflicts(containers)
}
var supportedManifestVersions util.StringSet = util.NewStringSet("v1beta1", "v1beta2")
// ValidateManifest tests that the specified ContainerManifest has valid data.
// This includes checking formatting and uniqueness. It also canonicalizes the
// structure by setting default values and implementing any backwards-compatibility
@ -127,7 +216,7 @@ func ValidateManifest(manifest *ContainerManifest) error {
if len(manifest.Version) == 0 {
return makeInvalidError("ContainerManifest.Version", manifest.Version)
}
if !supportedManifestVersions.Has(manifest.Version) {
if !supportedManifestVersions.Has(strings.ToLower(manifest.Version)) {
return makeNotSupportedError("ContainerManifest.Version", manifest.Version)
}
if !util.IsDNSSubdomain(manifest.ID) {

102
pkg/api/validation_test.go
View File

@ -50,6 +50,52 @@ func TestValidateVolumes(t *testing.T) {
}
}
func TestValidatePorts(t *testing.T) {
successCase := []Port{
{Name: "abc", ContainerPort: 80, HostPort: 80, Protocol: "TCP"},
{Name: "123", ContainerPort: 81, HostPort: 81},
{Name: "easy", ContainerPort: 82, Protocol: "TCP"},
{Name: "as", ContainerPort: 83, Protocol: "UDP"},
{Name: "do-re-me", ContainerPort: 84},
{Name: "baby-you-and-me", ContainerPort: 82, Protocol: "tcp"},
{ContainerPort: 85},
}
err := validatePorts(successCase)
if err != nil {
t.Errorf("expected success: %v", err)
}
nonCanonicalCase := []Port{
{ContainerPort: 80},
}
err = validatePorts(nonCanonicalCase)
if err != nil {
t.Errorf("expected success: %v", err)
}
if nonCanonicalCase[0].HostPort != 80 || nonCanonicalCase[0].Protocol != "TCP" {
t.Errorf("expected default values: %+v", nonCanonicalCase[0])
}
errorCases := map[string][]Port{
"name > 63 characters": {{Name: strings.Repeat("a", 64), ContainerPort: 80}},
"name not a DNS label": {{Name: "a.b.c", ContainerPort: 80}},
"name not unique": {
{Name: "abc", ContainerPort: 80},
{Name: "abc", ContainerPort: 81},
},
"zero container port": {{ContainerPort: 0}},
"invalid container port": {{ContainerPort: 65536}},
"invalid host port": {{ContainerPort: 80, HostPort: 65536}},
"invalid protocol": {{ContainerPort: 80, Protocol: "ICMP"}},
}
for k, v := range errorCases {
err := validatePorts(v)
if err == nil {
t.Errorf("expected failure for %s", k)
}
}
}
func TestValidateEnv(t *testing.T) {
successCase := []EnvVar{
{Name: "abc", Value: "value"},
@ -82,6 +128,42 @@ func TestValidateEnv(t *testing.T) {
}
}
func TestValidateVolumeMounts(t *testing.T) {
volumes := util.NewStringSet("abc", "123", "abc-123")
successCase := []VolumeMount{
{Name: "abc", MountPath: "/foo"},
{Name: "123", MountPath: "/foo"},
{Name: "abc-123", MountPath: "/bar"},
}
if err := validateVolumeMounts(successCase, volumes); err != nil {
t.Errorf("expected success: %v", err)
}
nonCanonicalCase := []VolumeMount{
{Name: "abc", Path: "/foo"},
}
err := validateVolumeMounts(nonCanonicalCase, volumes)
if err != nil {
t.Errorf("expected success: %v", err)
}
if nonCanonicalCase[0].MountPath != "/foo" {
t.Errorf("expected canonicalized values: %+v", nonCanonicalCase[0])
}
errorCases := map[string][]VolumeMount{
"empty name": {{Name: "", MountPath: "/foo"}},
"name not found": {{Name: "", MountPath: "/foo"}},
"empty mountpath": {{Name: "abc", MountPath: ""}},
}
for k, v := range errorCases {
err := validateVolumeMounts(v, volumes)
if err == nil {
t.Errorf("expected failure for %s", k)
}
}
}
func TestValidateContainers(t *testing.T) {
volumes := util.StringSet{}
@ -103,9 +185,16 @@ func TestValidateContainers(t *testing.T) {
{Name: "abc", Image: "image"},
},
"zero-length image": {{Name: "abc", Image: ""}},
"host port not unique": {
{Name: "abc", Image: "image", Ports: []Port{{ContainerPort: 80, HostPort: 80}}},
{Name: "def", Image: "image", Ports: []Port{{ContainerPort: 81, HostPort: 80}}},
},
"invalid env var name": {
{Name: "abc", Image: "image", Env: []EnvVar{{Name: "ev.1"}}},
},
"unknown volume name": {
{Name: "abc", Image: "image", VolumeMounts: []VolumeMount{{Name: "anything", MountPath: "/foo"}}},
},
}
for k, v := range errorCases {
if err := validateContainers(v, volumes); err == nil {
@ -117,8 +206,8 @@ func TestValidateContainers(t *testing.T) {
func TestValidateManifest(t *testing.T) {
successCases := []ContainerManifest{
{Version: "v1beta1", ID: "abc"},
{Version: "v1beta1", ID: "123"},
{Version: "v1beta1", ID: "abc.123.do-re-mi"},
{Version: "v1beta2", ID: "123"},
{Version: "V1BETA1", ID: "abc.123.do-re-mi"},
{
Version: "v1beta1",
ID: "abc",
@ -131,11 +220,20 @@ func TestValidateManifest(t *testing.T) {
WorkingDir: "/tmp",
Memory: 1,
CPU: 1,
Ports: []Port{
{Name: "p1", ContainerPort: 80, HostPort: 8080},
{Name: "p2", ContainerPort: 81},
{ContainerPort: 82},
},
Env: []EnvVar{
{Name: "ev1", Value: "val1"},
{Name: "ev2", Value: "val2"},
{Key: "EV3", Value: "val3"},
},
VolumeMounts: []VolumeMount{
{Name: "vol1", MountPath: "/foo"},
{Name: "vol1", Path: "/bar"},
},
},
},
},

36
pkg/kubelet/kubelet.go
View File

@ -318,15 +318,13 @@ func makePortsAndBindings(container *api.Container) (map[docker.Port]struct{}, m
// Some of this port stuff is under-documented voodoo.
// See http://stackoverflow.com/questions/20428302/binding-a-port-to-a-host-interface-using-the-rest-api
var protocol string
switch port.Protocol {
case "udp":
switch strings.ToUpper(port.Protocol) {
case "UDP":
protocol = "/udp"
case "tcp":
case "TCP":
protocol = "/tcp"
default:
if len(port.Protocol) != 0 {
glog.Infof("Unknown protocol: %s, defaulting to tcp.", port.Protocol)
}
glog.Infof("Unknown protocol '%s': defaulting to TCP", port.Protocol)
protocol = "/tcp"
}
dockerPort := docker.Port(strconv.Itoa(interiorPort) + protocol)
@ -770,6 +768,20 @@ func (kl *Kubelet) SyncManifests(config []api.ContainerManifest) error {
return err
}
// Check that all Port.HostPort values are unique across all manifests.
func checkHostPortConflicts(allManifests []api.ContainerManifest, newManifest *api.ContainerManifest) error {
allPorts := map[int]bool{}
extract := func(p *api.Port) int { return p.HostPort }
for i := range allManifests {
manifest := &allManifests[i]
err := api.AccumulateUniquePorts(manifest.Containers, allPorts, extract)
if err != nil {
return err
}
}
return api.AccumulateUniquePorts(newManifest.Containers, allPorts, extract)
}
// syncLoop is the main loop for processing changes. It watches for changes from
// four channels (file, etcd, server, and http) and creates a union of them. For
// any new change seen, will run a sync against desired state and running state. If
@ -787,14 +799,26 @@ func (kl *Kubelet) syncLoop(updateChannel <-chan manifestUpdate, handler SyncHan
}
allManifests := []api.ContainerManifest{}
allIds := util.StringSet{}
for src, srcManifests := range last {
for i := range srcManifests {
m := &srcManifests[i]
if allIds.Has(m.ID) {
glog.Warningf("Manifest from %s has duplicate ID, ignoring: %v", src, m.ID)
continue
}
allIds.Insert(m.ID)
if err := api.ValidateManifest(m); err != nil {
glog.Warningf("Manifest from %s failed validation, ignoring: %v", src, err)
continue
}
// Check for host-wide HostPort conflicts.
if err := checkHostPortConflicts(allManifests, m); err != nil {
glog.Warningf("Manifest from %s failed validation, ignoring: %v", src, err)
continue
}
}
// TODO(thockin): There's no reason to collect manifests by value. Don't pessimize.
allManifests = append(allManifests, srcManifests...)
}

26
pkg/kubelet/kubelet_test.go
View File

@ -572,7 +572,32 @@ func TestMakePortsAndBindings(t *testing.T) {
}
}
}
}
func TestCheckHostPortConflicts(t *testing.T) {
successCaseAll := []api.ContainerManifest{
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 80}}}}},
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 81}}}}},
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 82}}}}},
}
successCaseNew := api.ContainerManifest{
Containers: []api.Container{{Ports: []api.Port{{HostPort: 83}}}},
}
if err := checkHostPortConflicts(successCaseAll, &successCaseNew); err != nil {
t.Errorf("Expected success: %v", err)
}
failureCaseAll := []api.ContainerManifest{
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 80}}}}},
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 81}}}}},
{Containers: []api.Container{{Ports: []api.Port{{HostPort: 82}}}}},
}
failureCaseNew := api.ContainerManifest{
Containers: []api.Container{{Ports: []api.Port{{HostPort: 81}}}},
}
if err := checkHostPortConflicts(failureCaseAll, &failureCaseNew); err == nil {
t.Errorf("Expected failure")
}
}
func TestExtractFromNonExistentFile(t *testing.T) {
@ -597,7 +622,6 @@ func TestExtractFromBadDataFile(t *testing.T) {
if err == nil {
t.Error("Unexpected non-error.")
}
}
func TestExtractFromValidDataFile(t *testing.T) {