From 8bdfc352ce91a903352cb14950a954634cf3a529 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 19 Feb 2015 22:03:36 -0800 Subject: [PATCH] minor fixups as I review secrets --- docs/design/secrets.md | 6 +-- pkg/api/testing/fuzzer.go | 3 +- pkg/api/types.go | 2 +- pkg/api/v1beta1/types.go | 2 +- pkg/api/v1beta2/types.go | 2 +- pkg/api/v1beta3/types.go | 2 +- pkg/api/validation/validation.go | 59 +++++++++++++-------------- pkg/api/validation/validation_test.go | 6 +-- 8 files changed, 39 insertions(+), 43 deletions(-) diff --git a/docs/design/secrets.md b/docs/design/secrets.md index ce02f930f9879..ac8776bd820cc 100644 --- a/docs/design/secrets.md +++ b/docs/design/secrets.md @@ -283,9 +283,9 @@ type Secret struct { type SecretType string const ( - SecretTypeOpaque SecretType = "opaque" // Opaque (arbitrary data; default) - SecretTypeKubernetesAuthToken SecretType = "kubernetes-auth" // Kubernetes auth token - SecretTypeDockerRegistryAuth SecretType = "docker-reg-auth" // Docker registry auth + SecretTypeOpaque SecretType = "Opaque" // Opaque (arbitrary data; default) + SecretTypeKubernetesAuthToken SecretType = "KubernetesAuth" // Kubernetes auth token + SecretTypeDockerRegistryAuth SecretType = "DockerRegistryAuth" // Docker registry auth // FUTURE: other type values ) diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index ed4b1a0e15f74..dbb0e05e2d3c7 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -179,7 +179,7 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer { func(vs *api.VolumeSource, c fuzz.Continue) { // Exactly one of the fields should be set. //FIXME: the fuzz can still end up nil. What if fuzz allowed me to say that? - fuzzOneOf(c, &vs.HostPath, &vs.EmptyDir, &vs.GCEPersistentDisk, &vs.GitRepo) + fuzzOneOf(c, &vs.HostPath, &vs.EmptyDir, &vs.GCEPersistentDisk, &vs.GitRepo, &vs.Secret) }, func(d *api.DNSPolicy, c fuzz.Continue) { policies := []api.DNSPolicy{api.DNSClusterFirst, api.DNSDefault} @@ -233,6 +233,7 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer { c.Fuzz(&s.ObjectMeta) s.Type = api.SecretTypeOpaque + c.Fuzz(&s.Data) }, func(ep *api.Endpoint, c fuzz.Continue) { // TODO: If our API used a particular type for IP fields we could just catch that here. diff --git a/pkg/api/types.go b/pkg/api/types.go index b4a7b5f609b39..3495d948218fb 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1342,7 +1342,7 @@ const MaxSecretSize = 1 * 1024 * 1024 type SecretType string const ( - SecretTypeOpaque SecretType = "opaque" // Default; arbitrary user-defined data + SecretTypeOpaque SecretType = "Opaque" // Default; arbitrary user-defined data ) type SecretList struct { diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 929ec181dd9a1..92259bc95aaf5 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -1125,7 +1125,7 @@ const MaxSecretSize = 1 * 1024 * 1024 type SecretType string const ( - SecretTypeOpaque SecretType = "opaque" // Default; arbitrary user-defined data + SecretTypeOpaque SecretType = "Opaque" // Default; arbitrary user-defined data ) type SecretList struct { diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 314aa4f9d5156..5a45eab9af893 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -1128,7 +1128,7 @@ const MaxSecretSize = 1 * 1024 * 1024 type SecretType string const ( - SecretTypeOpaque SecretType = "opaque" // Default; arbitrary user-defined data + SecretTypeOpaque SecretType = "Opaque" // Default; arbitrary user-defined data ) type SecretList struct { diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 45408842681b6..d7c3ec42ee920 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1279,7 +1279,7 @@ const MaxSecretSize = 1 * 1024 * 1024 type SecretType string const ( - SecretTypeOpaque SecretType = "opaque" // Default; arbitrary user-defined data + SecretTypeOpaque SecretType = "Opaque" // Default; arbitrary user-defined data ) type SecretList struct { diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 619d6cde9bf6a..95d4bcea9c1b7 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -116,6 +116,28 @@ func ValidateNamespaceName(name string, prefix bool) (bool, string) { return nameIsDNSSubdomain(name, prefix) } +// ValidateLimitRangeName can be used to check whether the given limit range name is valid. +// Prefix indicates this name will be used as part of generation, in which case +// trailing dashes are allowed. +func ValidateLimitRangeName(name string, prefix bool) (bool, string) { + return nameIsDNSSubdomain(name, prefix) +} + +// ValidateResourceQuotaName can be used to check whether the given +// resource quota name is valid. +// Prefix indicates this name will be used as part of generation, in which case +// trailing dashes are allowed. +func ValidateResourceQuotaName(name string, prefix bool) (bool, string) { + return nameIsDNSSubdomain(name, prefix) +} + +// ValidateSecretName can be used to check whether the given secret name is valid. +// Prefix indicates this name will be used as part of generation, in which case +// trailing dashes are allowed. +func ValidateSecretName(name string, prefix bool) (bool, string) { + return nameIsDNSSubdomain(name, prefix) +} + // nameIsDNSSubdomain is a ValidateNameFunc for names that must be a DNS subdomain. func nameIsDNSSubdomain(name string, prefix bool) (bool, string) { if prefix { @@ -815,16 +837,8 @@ func validateResourceName(value string, field string) errs.ValidationErrorList { // ValidateLimitRange tests if required fields in the LimitRange are set. func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - if len(limitRange.Name) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("name", limitRange.Name)) - } else if !util.IsDNSSubdomain(limitRange.Name) { - allErrs = append(allErrs, errs.NewFieldInvalid("name", limitRange.Name, dnsSubdomainErrorMsg)) - } - if len(limitRange.Namespace) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("namespace", limitRange.Namespace)) - } else if !util.IsDNSSubdomain(limitRange.Namespace) { - allErrs = append(allErrs, errs.NewFieldInvalid("namespace", limitRange.Namespace, dnsSubdomainErrorMsg)) - } + allErrs = append(allErrs, ValidateObjectMeta(&limitRange.ObjectMeta, true, ValidateLimitRangeName).Prefix("metadata")...) + // ensure resource names are properly qualified per docs/resources.md for i := range limitRange.Spec.Limits { limit := limitRange.Spec.Limits[i] @@ -841,21 +855,12 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList { // ValidateSecret tests if required fields in the Secret are set. func ValidateSecret(secret *api.Secret) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - if len(secret.Name) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("name", secret.Name)) - } else if !util.IsDNSSubdomain(secret.Name) { - allErrs = append(allErrs, errs.NewFieldInvalid("name", secret.Name, "")) - } - if len(secret.Namespace) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("namespace", secret.Namespace)) - } else if !util.IsDNSSubdomain(secret.Namespace) { - allErrs = append(allErrs, errs.NewFieldInvalid("namespace", secret.Namespace, "")) - } + allErrs = append(allErrs, ValidateObjectMeta(&secret.ObjectMeta, true, ValidateSecretName).Prefix("metadata")...) totalSize := 0 for key, value := range secret.Data { if !util.IsDNSSubdomain(key) { - allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("data[%v]", key), key, cIdentifierErrorMsg)) + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("data[%s]", key), key, cIdentifierErrorMsg)) } totalSize += len(value) @@ -893,16 +898,8 @@ func validateResourceRequirements(container *api.Container) errs.ValidationError // ValidateResourceQuota tests if required fields in the ResourceQuota are set. func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - if len(resourceQuota.Name) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("name", resourceQuota.Name)) - } else if !util.IsDNSSubdomain(resourceQuota.Name) { - allErrs = append(allErrs, errs.NewFieldInvalid("name", resourceQuota.Name, dnsSubdomainErrorMsg)) - } - if len(resourceQuota.Namespace) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("namespace", resourceQuota.Namespace)) - } else if !util.IsDNSSubdomain(resourceQuota.Namespace) { - allErrs = append(allErrs, errs.NewFieldInvalid("namespace", resourceQuota.Namespace, dnsSubdomainErrorMsg)) - } + allErrs = append(allErrs, ValidateObjectMeta(&resourceQuota.ObjectMeta, true, ValidateResourceQuotaName).Prefix("metadata")...) + for k := range resourceQuota.Spec.Hard { allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...) } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 9e7e3c7647cb8..9125c52c8b996 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2302,8 +2302,7 @@ func TestValidateLimitRange(t *testing.T) { for i := range errs { field := errs[i].(*errors.ValidationError).Field detail := errs[i].(*errors.ValidationError).Detail - if field != "name" && - field != "namespace" { + if field != "metadata.name" && field != "metadata.namespace" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) } if detail != v.D { @@ -2370,8 +2369,7 @@ func TestValidateResourceQuota(t *testing.T) { for i := range errs { field := errs[i].(*errors.ValidationError).Field detail := errs[i].(*errors.ValidationError).Detail - if field != "name" && - field != "namespace" { + if field != "metadata.name" && field != "metadata.namespace" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) } if detail != v.D {