Skip to content

Validate template parameter name and description length #2691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Validate template parameter name and description length #2691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a2c679
Validate template paramter name and description length
mfojtik Jun 2, 2015
267b04b
Update template validation tests to exercise long fields
mfojtik Jun 3, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions pkg/template/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,26 @@ import (

var parameterNameExp = regexp.MustCompile(`^[a-zA-Z0-9\_]+$`)

// totalParametersSizeLimitB is the total size limit for the template
// parameters names and descriptions.
// TODO: This constant was copied from k8s totalAnnotationSizeLimitB, we
// should make the upstream public with a better name and re-use it
const totalParametersSizeLimitB int = 64 * 1024 // 64 kB

// ValidateParametersSize validates the total size of all parameters names,
// descriptions.
func ValidateParametersSize(params []api.Parameter) fielderrors.ValidationErrorList {
allErrs := fielderrors.ValidationErrorList{}
var totalSize int64
for i := range params {
totalSize += (int64)(len(params[i].Name)) + (int64)(len(params[i].Description))
Copy link
Contributor

@liggitt liggitt Jun 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we said we cared about value length when the template was going to be stored, and we didn't care when it was just going to be processed. @smarterclayton, what was the reason for validating the total size again?

Copy link
Contributor

@smarterclayton smarterclayton Jun 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storage size is all we care about. We should have a separate validation path for process (i believe) that should not be so restrictive.

----- Original Message -----

@@ -12,6 +12,26 @@ import (

var parameterNameExp = regexp.MustCompile(^[a-zA-Z0-9\_]+$)

+// totalParametersSizeLimitB is the total size limit for the template
+// parameters names and descriptions.
+// TODO: This constant was copied from k8s totalAnnotationSizeLimitB, we
+// should make the upstream public with a better name and re-use it
+const totalParametersSizeLimitB int = 64 * 1024 // 64 kB
+
+// ValidateParametersSize validates the total size of all parameters
names,
+// descriptions.
+func ValidateParametersSize(params []api.Parameter)
fielderrors.ValidationErrorList {

  • allErrs := fielderrors.ValidationErrorList{}
  • var totalSize int64
  • for i := range params {
  •   totalSize += (int64)(len(params[i].Name)) +
    (int64)(len(params[i].Description))

I thought we said we cared about value length when the template was going to
be stored, and we didn't care when it was just going to be processed.
@smarterclayton, what was the reason for validating the total size again?

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/2691/files#r31666672

}
if totalSize > (int64)(totalParametersSizeLimitB) {
allErrs = append(allErrs, fielderrors.NewFieldTooLong("parameters", "", totalParametersSizeLimitB))
}
return allErrs
}

// ValidateParameter tests if required fields in the Parameter are set.
func ValidateParameter(param *api.Parameter) (errs fielderrors.ValidationErrorList) {
if len(param.Name) == 0 {
Expand Down Expand Up @@ -48,6 +68,7 @@ func validateTemplateBody(template *api.Template) (errs fielderrors.ValidationEr
paramErr := ValidateParameter(&template.Parameters[i])
errs = append(errs, paramErr.PrefixIndex(i).Prefix("parameters")...)
}
errs = append(errs, ValidateParametersSize(template.Parameters)...)
errs = append(errs, validation.ValidateLabels(template.ObjectLabels, "labels")...)
return
}
21 changes: 21 additions & 0 deletions pkg/template/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package validation

import (
"strings"
"testing"

kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
Expand All @@ -17,6 +18,26 @@ func makeParameter(name, value string) *api.Parameter {
}
}

func TestValidateParameterSize(t *testing.T) {
var tests = []struct {
Parameters []api.Parameter
IsValidExpected bool
}{
{[]api.Parameter{}, true},
{[]api.Parameter{{Name: "short", Description: "short"}}, true},
{[]api.Parameter{{Name: strings.Repeat("a", 64*1024), Description: "short"}}, false},
{[]api.Parameter{{Name: strings.Repeat("a", 32*1024), Description: strings.Repeat("b", 33*1024)}}, false},
}
for _, test := range tests {
if test.IsValidExpected && len(ValidateParametersSize(test.Parameters)) != 0 {
t.Errorf("Expected zero validation errors on valid parameter size.")
}
if !test.IsValidExpected && len(ValidateParametersSize(test.Parameters)) == 0 {
t.Errorf("Expected some validation errors on invalid parameter size.")
}
}
}

func TestValidateParameter(t *testing.T) {
var tests = []struct {
ParameterName string
Expand Down