diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 91a7c8bb431..caaee1b17df 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -418,6 +418,12 @@ const ( DeletePropagationForeground DeletionPropagation = "Foreground" ) +const ( + // DryRunAll means to complete all processing stages, but don't + // persist changes to storage. + DryRunAll = "All" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // DeleteOptions may be provided when deleting an API object. @@ -456,8 +462,9 @@ type DeleteOptions struct { // When present, indicates that modifications should not be // persisted. An invalid or unrecognized dryRun directive will - // result in a BadRequest response and no further processing of - // the request. + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed // +optional DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,5,rep,name=dryRun"` } @@ -470,8 +477,9 @@ type CreateOptions struct { // When present, indicates that modifications should not be // persisted. An invalid or unrecognized dryRun directive will - // result in a BadRequest response and no further processing of - // the request. + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed // +optional DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"` @@ -488,8 +496,9 @@ type UpdateOptions struct { // When present, indicates that modifications should not be // persisted. An invalid or unrecognized dryRun directive will - // result in a BadRequest response and no further processing of - // the request. + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed // +optional DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"` } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD index bccaaa0a925..ea9cc91b057 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD @@ -20,6 +20,7 @@ go_library( importpath = "k8s.io/apimachinery/pkg/apis/meta/v1/validation", deps = [ "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", ], diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go index 96e5f42b735..81f86fb3068 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go @@ -18,6 +18,7 @@ package validation import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -84,6 +85,25 @@ func ValidateDeleteOptions(options *metav1.DeleteOptions) field.ErrorList { *options.PropagationPolicy != metav1.DeletePropagationOrphan { allErrs = append(allErrs, field.NotSupported(field.NewPath("propagationPolicy"), options.PropagationPolicy, []string{string(metav1.DeletePropagationForeground), string(metav1.DeletePropagationBackground), string(metav1.DeletePropagationOrphan), "nil"})) } + allErrs = append(allErrs, validateDryRun(field.NewPath("dryRun"), options.DryRun)...) + return allErrs +} + +func ValidateCreateOptions(options *metav1.CreateOptions) field.ErrorList { + return validateDryRun(field.NewPath("dryRun"), options.DryRun) +} + +func ValidateUpdateOptions(options *metav1.UpdateOptions) field.ErrorList { + return validateDryRun(field.NewPath("dryRun"), options.DryRun) +} + +var allowedDryRunValues = sets.NewString(metav1.DryRunAll) + +func validateDryRun(fldPath *field.Path, dryRun []string) field.ErrorList { + allErrs := field.ErrorList{} + if !allowedDryRunValues.HasAll(dryRun...) { + allErrs = append(allErrs, field.NotSupported(fldPath, dryRun, allowedDryRunValues.List())) + } return allErrs } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go index 9766fa7e73f..a7046a54892 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go @@ -17,6 +17,7 @@ limitations under the License. package validation import ( + "fmt" "strings" "testing" @@ -92,3 +93,35 @@ func TestValidateLabels(t *testing.T) { } } } + +func TestValidDryRun(t *testing.T) { + tests := [][]string{ + {}, + {"All"}, + {"All", "All"}, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%v", test), func(t *testing.T) { + if errs := validateDryRun(field.NewPath("dryRun"), test); len(errs) != 0 { + t.Errorf("%v should be a valid dry-run value: %v", test, errs) + } + }) + } +} + +func TestInvalidDryRun(t *testing.T) { + tests := [][]string{ + {"False"}, + {"All", "False"}, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%v", test), func(t *testing.T) { + if len(validateDryRun(field.NewPath("dryRun"), test)) == 0 { + t.Errorf("%v shouldn't be a valid dry-run value", test) + } + }) + } + +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 0df164c89cb..a7f19e953bf 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -57,6 +57,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 9ce7392cf6d..c838546a793 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -83,12 +84,16 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte options := &metav1.CreateOptions{} values := req.URL.Query() - // TODO: replace with content type negotiation? if err := metainternalversion.ParameterCodec.DecodeParameters(values, scope.MetaGroupVersion, options); err != nil { err = errors.NewBadRequest(err.Error()) scope.err(err, w, req) return } + if errs := validation.ValidateCreateOptions(options); len(errs) > 0 { + err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "CreateOptions"}, "", errs) + scope.err(err, w, req) + return + } defaultGVK := scope.Kind original := r.New() diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index 6afba303665..fc37e71b2d3 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -24,7 +24,9 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" @@ -97,6 +99,11 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco } } } + if errs := validation.ValidateDeleteOptions(options); len(errs) > 0 { + err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs) + scope.err(err, w, req) + return + } trace.Step("About to check admission control") if admit != nil && admit.Handles(admission.Delete) { @@ -266,6 +273,11 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco } } } + if errs := validation.ValidateDeleteOptions(options); len(errs) > 0 { + err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs) + scope.err(err, w, req) + return + } result, err := finishRequest(timeout, func() (runtime.Object, error) { return r.DeleteCollection(ctx, options, &listOptions) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index c1b9da27f9e..65588638b68 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -96,6 +97,11 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface scope.err(err, w, req) return } + if errs := validation.ValidateUpdateOptions(options); len(errs) > 0 { + err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "UpdateOptions"}, "", errs) + scope.err(err, w, req) + return + } ae := request.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 228d0922a86..a164ca203fc 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -72,6 +73,11 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac scope.err(err, w, req) return } + if errs := validation.ValidateUpdateOptions(options); len(errs) > 0 { + err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "UpdateOptions"}, "", errs) + scope.err(err, w, req) + return + } s, err := negotiation.NegotiateInputSerializer(req, false, scope.Serializer) if err != nil {