Merge pull request #9416 from croomes/resource-name-validation

Add name validation rules for different object kinds
pull/9425/head
Adam Reese 3 years ago committed by GitHub
commit 213a7df2dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -65,7 +65,7 @@ var (
// ValidName is a regular expression for resource names.
//
// DEPRECATED: This will be removed in Helm 4, and is no longer used here. See
// pkg/chartutil.ValidateName for the replacement.
// pkg/lint/rules.validateMetadataNameFunc for the replacement.
//
// According to the Kubernetes help text, the regular expression it uses is:
//

@ -96,6 +96,9 @@ func ValidateReleaseName(name string) error {
//
// The Kubernetes documentation is here, though it is not entirely correct:
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
//
// Deprecated: remove in Helm 4. Name validation now uses rules defined in
// pkg/lint/rules.validateMetadataNameFunc()
func ValidateMetadataName(name string) error {
if name == "" || len(name) > maxMetadataNameLen || !validName.MatchString(name) {
return errInvalidKubernetesName

@ -28,6 +28,9 @@ import (
"strings"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/api/validation"
apipath "k8s.io/apimachinery/pkg/api/validation/path"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apimachinery/pkg/util/yaml"
"helm.sh/helm/v3/pkg/chart/loader"
@ -202,18 +205,68 @@ func validateYamlContent(err error) error {
return errors.Wrap(err, "unable to parse YAML")
}
// validateMetadataName uses the correct validation function for the object
// Kind, or if not set, defaults to the standard definition of a subdomain in
// DNS (RFC 1123), used by most resources.
func validateMetadataName(obj *K8sYamlStruct) error {
if len(obj.Metadata.Name) == 0 || len(obj.Metadata.Name) > 253 {
return fmt.Errorf("object name must be between 0 and 253 characters: %q", obj.Metadata.Name)
fn := validateMetadataNameFunc(obj)
allErrs := field.ErrorList{}
for _, msg := range fn(obj.Metadata.Name, false) {
allErrs = append(allErrs, field.Invalid(field.NewPath("metadata").Child("name"), obj.Metadata.Name, msg))
}
// This will return an error if the characters do not abide by the standard OR if the
// name is left empty.
if err := chartutil.ValidateMetadataName(obj.Metadata.Name); err != nil {
return errors.Wrapf(err, "object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
if len(allErrs) > 0 {
return errors.Wrapf(allErrs.ToAggregate(), "object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
}
return nil
}
// validateMetadataNameFunc will return a name validation function for the
// object kind, if defined below.
//
// Rules should match those set in the various api validations:
// https://github.com/kubernetes/kubernetes/blob/v1.20.0/pkg/apis/core/validation/validation.go#L205-L274
// https://github.com/kubernetes/kubernetes/blob/v1.20.0/pkg/apis/apps/validation/validation.go#L39
// ...
//
// Implementing here to avoid importing k/k.
//
// If no mapping is defined, returns NameIsDNSSubdomain. This is used by object
// kinds that don't have special requirements, so is the most likely to work if
// new kinds are added.
func validateMetadataNameFunc(obj *K8sYamlStruct) validation.ValidateNameFunc {
switch strings.ToLower(obj.Kind) {
case "pod", "node", "secret", "endpoints", "resourcequota", // core
"controllerrevision", "daemonset", "deployment", "replicaset", "statefulset", // apps
"autoscaler", // autoscaler
"cronjob", "job", // batch
"lease", // coordination
"endpointslice", // discovery
"networkpolicy", "ingress", // networking
"podsecuritypolicy", // policy
"priorityclass", // scheduling
"podpreset", // settings
"storageclass", "volumeattachment", "csinode": // storage
return validation.NameIsDNSSubdomain
case "service":
return validation.NameIsDNS1035Label
case "namespace":
return validation.ValidateNamespaceName
case "serviceaccount":
return validation.ValidateServiceAccountName
case "certificatesigningrequest":
// No validation.
// https://github.com/kubernetes/kubernetes/blob/v1.20.0/pkg/apis/certificates/validation/validation.go#L137-L140
return func(name string, prefix bool) []string { return nil }
case "role", "clusterrole", "rolebinding", "clusterrolebinding":
// https://github.com/kubernetes/kubernetes/blob/v1.20.0/pkg/apis/rbac/validation/validation.go#L32-L34
return func(name string, prefix bool) []string {
return apipath.IsValidPathSegmentName(name)
}
default:
return validation.NameIsDNSSubdomain
}
}
func validateNoCRDHooks(manifest []byte) error {
if crdHookSearch.Match(manifest) {
return errors.New("manifest is a crd-install hook. This hook is no longer supported in v3 and all CRDs should also exist the crds/ directory at the top level of the chart")

@ -17,13 +17,12 @@ limitations under the License.
package rules
import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"github.com/Masterminds/goutils"
"helm.sh/helm/v3/internal/test/ensure"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/chartutil"
@ -122,39 +121,84 @@ func TestMultiTemplateFail(t *testing.T) {
}
func TestValidateMetadataName(t *testing.T) {
names := map[string]bool{
"": false,
"foo": true,
"foo.bar1234baz.seventyone": true,
"FOO": false,
"123baz": true,
"foo.BAR.baz": false,
"one-two": true,
"-two": false,
"one_two": false,
"a..b": false,
"%^&#$%*@^*@&#^": false,
}
// The length checker should catch this first. So this is not true fuzzing.
tooLong, err := goutils.RandomAlphaNumeric(300)
if err != nil {
t.Fatalf("Randomizer failed to initialize: %s", err)
tests := []struct {
obj *K8sYamlStruct
wantErr bool
}{
// Most kinds use IsDNS1123Subdomain.
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: ""}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "foo.bar1234baz.seventyone"}}, false},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "FOO"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "foo.BAR.baz"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "one-two"}}, false},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "-two"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "one_two"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "a..b"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "%^&#$%*@^*@&#^"}}, true},
{&K8sYamlStruct{Kind: "Pod", Metadata: k8sYamlMetadata{Name: "operator:pod"}}, true},
{&K8sYamlStruct{Kind: "ServiceAccount", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "ServiceAccount", Metadata: k8sYamlMetadata{Name: "foo.bar1234baz.seventyone"}}, false},
{&K8sYamlStruct{Kind: "ServiceAccount", Metadata: k8sYamlMetadata{Name: "FOO"}}, true},
{&K8sYamlStruct{Kind: "ServiceAccount", Metadata: k8sYamlMetadata{Name: "operator:sa"}}, true},
// Service uses IsDNS1035Label.
{&K8sYamlStruct{Kind: "Service", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "Service", Metadata: k8sYamlMetadata{Name: "123baz"}}, true},
{&K8sYamlStruct{Kind: "Service", Metadata: k8sYamlMetadata{Name: "foo.bar"}}, true},
// Namespace uses IsDNS1123Label.
{&K8sYamlStruct{Kind: "Namespace", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "Namespace", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "Namespace", Metadata: k8sYamlMetadata{Name: "foo.bar"}}, true},
{&K8sYamlStruct{Kind: "Namespace", Metadata: k8sYamlMetadata{Name: "foo-bar"}}, false},
// CertificateSigningRequest has no validation.
{&K8sYamlStruct{Kind: "CertificateSigningRequest", Metadata: k8sYamlMetadata{Name: ""}}, false},
{&K8sYamlStruct{Kind: "CertificateSigningRequest", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "CertificateSigningRequest", Metadata: k8sYamlMetadata{Name: "%^&#$%*@^*@&#^"}}, false},
// RBAC uses path validation.
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "foo.bar"}}, false},
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "operator:role"}}, false},
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "operator/role"}}, true},
{&K8sYamlStruct{Kind: "Role", Metadata: k8sYamlMetadata{Name: "operator%role"}}, true},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "foo.bar"}}, false},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "operator:role"}}, false},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "operator/role"}}, true},
{&K8sYamlStruct{Kind: "ClusterRole", Metadata: k8sYamlMetadata{Name: "operator%role"}}, true},
{&K8sYamlStruct{Kind: "RoleBinding", Metadata: k8sYamlMetadata{Name: "operator:role"}}, false},
{&K8sYamlStruct{Kind: "ClusterRoleBinding", Metadata: k8sYamlMetadata{Name: "operator:role"}}, false},
// Unknown Kind
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: ""}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "foo.bar1234baz.seventyone"}}, false},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "FOO"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "123baz"}}, false},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "foo.BAR.baz"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "one-two"}}, false},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "-two"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "one_two"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "a..b"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "%^&#$%*@^*@&#^"}}, true},
{&K8sYamlStruct{Kind: "FutureKind", Metadata: k8sYamlMetadata{Name: "operator:pod"}}, true},
// No kind
{&K8sYamlStruct{Metadata: k8sYamlMetadata{Name: "foo"}}, false},
{&K8sYamlStruct{Metadata: k8sYamlMetadata{Name: "operator:pod"}}, true},
}
names[tooLong] = false
for input, expectPass := range names {
obj := K8sYamlStruct{Metadata: k8sYamlMetadata{Name: input}}
if err := validateMetadataName(&obj); (err == nil) != expectPass {
st := "fail"
if expectPass {
st = "succeed"
for _, tt := range tests {
t.Run(fmt.Sprintf("%s/%s", tt.obj.Kind, tt.obj.Metadata.Name), func(t *testing.T) {
if err := validateMetadataName(tt.obj); (err != nil) != tt.wantErr {
t.Errorf("validateMetadataName() error = %v, wantErr %v", err, tt.wantErr)
}
t.Errorf("Expected %q to %s", input, st)
if err != nil {
t.Log(err)
}
}
})
}
}

Loading…
Cancel
Save