diff --git a/api/v1/ytsaurus_webhook.go b/api/v1/ytsaurus_webhook.go index 92d6389e7..6d6271663 100644 --- a/api/v1/ytsaurus_webhook.go +++ b/api/v1/ytsaurus_webhook.go @@ -19,6 +19,7 @@ package v1 import ( "context" "fmt" + "slices" "strings" corev1 "k8s.io/api/core/v1" @@ -561,32 +562,41 @@ func (r *baseValidator) validateCommonSpec(spec *CommonSpec) field.ErrorList { func (r *baseValidator) validateUpdateSelectors(newYtsaurus *Ytsaurus) field.ErrorList { var allErrors field.ErrorList - hasNothing := false planPath := field.NewPath("spec").Child("updatePlan") + exclusiveClass := consts.ComponentClassUnspecified if newYtsaurus.Spec.UpdatePlan != nil { if len(newYtsaurus.Spec.UpdatePlan) == 0 { allErrors = append(allErrors, field.Required(planPath, "updatePlan should not be empty")) } - for i, updateSelector := range newYtsaurus.Spec.UpdatePlan { + for i, entry := range newYtsaurus.Spec.UpdatePlan { entryPath := planPath.Index(i) - if updateSelector.Class != consts.ComponentClassUnspecified && (updateSelector.Component.Type != "" || updateSelector.Component.Name != "") { - allErrors = append(allErrors, field.Invalid(entryPath.Child("component"), updateSelector.Component, "Only one of component or class should be specified")) + if entry.Class != consts.ComponentClassUnspecified && (entry.Component.Type != "" || entry.Component.Name != "") { + allErrors = append(allErrors, field.Invalid(entryPath.Child("component"), entry.Component, "Only one of component or class should be specified")) } - if updateSelector.Class == consts.ComponentClassUnspecified && updateSelector.Component.Type == "" && updateSelector.Component.Name == "" { - allErrors = append(allErrors, field.Invalid(entryPath.Child("component"), updateSelector.Component, "Either component or class should be specified")) + if entry.Class == consts.ComponentClassUnspecified && entry.Component.Type == "" && entry.Component.Name == "" { + allErrors = append(allErrors, field.Invalid(entryPath.Child("component"), entry.Component, "Either component or class should be specified")) } - if updateSelector.Component.Name != "" && updateSelector.Component.Type == "" { - allErrors = append(allErrors, field.Required(entryPath.Child("component").Child("type"), "component.type must be set when component.name is provided")) + if entry.Component.Type != "" && !slices.Contains(consts.LocalComponentTypes, entry.Component.Type) { + allErrors = append(allErrors, field.Invalid(entryPath.Child("component").Child("type"), entry.Component.Type, "Unknown component type")) } - if updateSelector.Class == consts.ComponentClassNothing { - hasNothing = true + if exclusiveClass != consts.ComponentClassUnspecified { + allErrors = append(allErrors, field.Invalid(entryPath, entry, fmt.Sprintf("Update plan already contains class %s", exclusiveClass))) } - allErrors = append(allErrors, validateUpdateModeForSelector(updateSelector, entryPath)...) - } - if hasNothing && len(newYtsaurus.Spec.UpdatePlan) > 1 { - allErrors = append(allErrors, field.Invalid(planPath, newYtsaurus.Spec.UpdatePlan, "updatePlan should contain only one Nothing class")) + + switch entry.Class { + case consts.ComponentClassNothing, consts.ComponentClassEverything: + exclusiveClass = entry.Class + if i > 0 { + allErrors = append(allErrors, field.Invalid(entryPath.Child("class"), entry.Class, "Should be first and only entry")) + } + case consts.ComponentClassStateless, consts.ComponentClassUnspecified: + default: + allErrors = append(allErrors, field.Invalid(entryPath.Child("class"), entry.Class, "Unknown class")) + } + + allErrors = append(allErrors, validateUpdateModeForSelector(entry, entryPath.Child("updateMode"))...) } } diff --git a/test/webhooks/ytsaurus_webhooks_test.go b/test/webhooks/ytsaurus_webhooks_test.go index c46683182..60a667b2b 100644 --- a/test/webhooks/ytsaurus_webhooks_test.go +++ b/test/webhooks/ytsaurus_webhooks_test.go @@ -355,6 +355,41 @@ var _ = Describe("Test for Ytsaurus webhooks", func() { Expect(k8sClient.Delete(ctx, ytsaurus)).Should(Succeed()) }) + DescribeTable("Forbidden update plans", + func(plan []ytv1.ComponentUpdateSelector, msg string) { + ytsaurus.Spec.UpdatePlan = plan + Expect(k8sClient.Create(ctx, ytsaurus)).To(MatchError(ContainSubstring(msg))) + }, + Entry("empty entry", []ytv1.ComponentUpdateSelector{ + {}, + }, "Either component or class should be specified"), + Entry("both class and component", []ytv1.ComponentUpdateSelector{ + {Class: consts.ComponentClassStateless, Component: ytv1.Component{Type: consts.MasterType}}, + }, "Only one of component or class should be specified"), + Entry("unknown type", []ytv1.ComponentUpdateSelector{ + {Component: ytv1.Component{Type: "Unknown"}}, + }, "Unknown component type"), + Entry("unknown class", []ytv1.ComponentUpdateSelector{ + {Class: "Unknown"}, + }, "Unsupported value"), + Entry("nothing, something", []ytv1.ComponentUpdateSelector{ + {Class: consts.ComponentClassNothing}, + {Component: ytv1.Component{Type: consts.MasterType}}, + }, "Update plan already contains class Nothing"), + Entry("everything, something", []ytv1.ComponentUpdateSelector{ + {Class: consts.ComponentClassEverything}, + {Component: ytv1.Component{Type: consts.MasterType}}, + }, "Update plan already contains class Everything"), + Entry("something, nothing", []ytv1.ComponentUpdateSelector{ + {Component: ytv1.Component{Type: consts.MasterType}}, + {Class: consts.ComponentClassNothing}, + }, "Should be first and only entry"), + Entry("something, everything", []ytv1.ComponentUpdateSelector{ + {Component: ytv1.Component{Type: consts.MasterType}}, + {Class: consts.ComponentClassEverything}, + }, "Should be first and only entry"), + ) + It("Should accept component update without updateMode (backward compatibility)", func() { ytsaurus := newYtsaurus() ytsaurus.Spec.QueueAgents = &ytv1.QueueAgentSpec{InstanceSpec: ytv1.InstanceSpec{InstanceCount: 1}}