Skip to content

Commit

Permalink
enhancement(CollaSet): forbid to update scaleStrategy.context (#84)
Browse files Browse the repository at this point in the history 
* enhancement(CollaSet): forbid to update scaleStrategy.context

* fix golint issue
  • Loading branch information
@wu8685
wu8685 authored Sep 1, 2023
1 parent 6b8f76a commit 4dbcf84
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 13 deletions.
29 changes: 20 additions & 9 deletions pkg/webhook/server/generic/collaset/collaset_validating_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,22 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) (
return admission.Errored(http.StatusBadRequest, err)
}

if err := h.validate(cls); err != nil {
var oldCls *appsv1alpha1.CollaSet
if req.Operation == admissionv1.Update || req.Operation == admissionv1.Delete {
oldCls = &appsv1alpha1.CollaSet{}
if err := h.Decoder.DecodeRaw(req.OldObject, oldCls); err != nil {
return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to unmarshal old object: %s", err))
}
}

if err := h.validate(cls, oldCls); err != nil {
return admission.Errored(http.StatusUnprocessableEntity, err)
}

return admission.Allowed("")
}

func (h *ValidatingHandler) validate(cls *appsv1alpha1.CollaSet) error {
func (h *ValidatingHandler) validate(cls, oldCls *appsv1alpha1.CollaSet) error {
var allErrs field.ErrorList
fSpec := field.NewPath("spec")

Expand All @@ -81,20 +89,24 @@ func (h *ValidatingHandler) validate(cls *appsv1alpha1.CollaSet) error {
}
allErrs = append(allErrs, h.validatePodTemplateSpec(cls, fSpec)...)
allErrs = append(allErrs, h.validateSelector(cls, fSpec)...)
allErrs = append(allErrs, h.validateScaleStrategy(cls, fSpec)...)
allErrs = append(allErrs, h.validateScaleStrategy(cls, oldCls, fSpec)...)
allErrs = append(allErrs, h.validateUpdateStrategy(cls, fSpec)...)

return allErrs.ToAggregate()
}

func (h *ValidatingHandler) validateScaleStrategy(cls *appsv1alpha1.CollaSet, fSpec *field.Path) field.ErrorList {
func (h *ValidatingHandler) validateScaleStrategy(cls, oldCls *appsv1alpha1.CollaSet, fSpec *field.Path) field.ErrorList {
var allErrs field.ErrorList

if cls.Spec.ScaleStrategy.OperationDelaySeconds != nil && *cls.Spec.ScaleStrategy.OperationDelaySeconds < 0 {
allErrs = append(allErrs, field.Invalid(fSpec.Child("scaleStrategy", "operationDelaySeconds"),
*cls.Spec.ScaleStrategy.OperationDelaySeconds, "operationDelaySeconds should not be smaller than 0"))
}

if oldCls != nil && oldCls.Spec.ScaleStrategy.Context != cls.Spec.ScaleStrategy.Context {
allErrs = append(allErrs, field.Forbidden(fSpec.Child("scaleStrategy", "context"), "scaleStrategy.context is not allowed to be changed"))
}

return allErrs
}

Expand All @@ -105,11 +117,10 @@ func (h *ValidatingHandler) validateUpdateStrategy(cls *appsv1alpha1.CollaSet, f
appsv1alpha1.CollaSetInPlaceOnlyPodUpdateStrategyType,
appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType:
default:
allErrs = append(allErrs, field.Invalid(fSpec.Child("updateStrategy", "podUpdatePolicy"),
cls.Spec.UpdateStrategy.PodUpdatePolicy, fmt.Sprintf("podUpdatePolicy should be one of %v",
[]appsv1alpha1.PodUpdateStrategyType{appsv1alpha1.CollaSetRecreatePodUpdateStrategyType,
appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType,
appsv1alpha1.CollaSetInPlaceOnlyPodUpdateStrategyType})))
allErrs = append(allErrs, field.NotSupported(fSpec.Child("updateStrategy", "podUpdatePolicy"),
cls.Spec.UpdateStrategy.PodUpdatePolicy, []string{string(appsv1alpha1.CollaSetRecreatePodUpdateStrategyType),
string(appsv1alpha1.CollaSetInPlaceIfPossiblePodUpdateStrategyType),
string(appsv1alpha1.CollaSetInPlaceOnlyPodUpdateStrategyType)}))
}

if cls.Spec.UpdateStrategy.RollingUpdate != nil && cls.Spec.UpdateStrategy.RollingUpdate.ByPartition != nil &&
Expand Down
72 changes: 68 additions & 4 deletions pkg/webhook/server/generic/collaset/collaset_validating_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

type testCase struct {
cls *appsv1alpha1.CollaSet
old *appsv1alpha1.CollaSet
messageKeyWords string
}

Expand Down Expand Up @@ -69,7 +70,7 @@ func TestValidatingCollaSet(t *testing.T) {

for i, tc := range successCases {
mutatingHandler.setDetaultCollaSet(tc.cls)
if err := validatingHandler.validate(tc.cls); err != nil {
if err := validatingHandler.validate(tc.cls, tc.old); err != nil {
t.Fatalf("got unexpected err for %d case: %s", i, err)
}
}
Expand Down Expand Up @@ -191,7 +192,7 @@ func TestValidatingCollaSet(t *testing.T) {
},
},
"invalid-update-policy": {
messageKeyWords: "podUpdatePolicy should be one of",
messageKeyWords: "supported values: \"ReCreate\", \"InPlaceIfPossible\", \"InPlaceOnly\"",
cls: &appsv1alpha1.CollaSet{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Expand Down Expand Up @@ -330,16 +331,79 @@ func TestValidatingCollaSet(t *testing.T) {
},
},
},
"context-change-forbidden": {
messageKeyWords: "scaleStrategy.context is not allowed to be changed",
cls: &appsv1alpha1.CollaSet{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: appsv1alpha1.CollaSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "foo",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"app": "foo",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "foo",
Image: "image:v1",
},
},
},
},
ScaleStrategy: appsv1alpha1.ScaleStrategy{
Context: "context",
},
},
},
old: &appsv1alpha1.CollaSet{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: appsv1alpha1.CollaSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "foo",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"app": "foo",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "foo",
Image: "image:v1",
},
},
},
},
ScaleStrategy: appsv1alpha1.ScaleStrategy{
Context: "context-old",
},
},
},
},
}

for key, tc := range failureCases {
mutatingHandler.setDetaultCollaSet(tc.cls)
err := validatingHandler.validate(tc.cls)
err := validatingHandler.validate(tc.cls, tc.old)
if err == nil {
t.Fatalf("expected err, got nil in case %s", key)
}
if !strings.Contains(err.Error(), tc.messageKeyWords) {
t.Fatalf("can not find message key words [%s] in case %s", tc.messageKeyWords, key)
t.Fatalf("can not find message key words [%s] in case %s, got %s", tc.messageKeyWords, key, err)
}
}
}
Expand Down

0 comments on commit 4dbcf84

Please sign in to comment.