Skip to content
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

🌱 Allow cluster class compatible changes #5213

Merged
merged 1 commit into from
Sep 8, 2021
Merged

🌱 Allow cluster class compatible changes #5213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
148 changes: 104 additions & 44 deletions api/v1alpha4/clusterclass_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1alpha4

import (
"fmt"
"reflect"
"strings"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -112,24 +111,24 @@ func (in *ClusterClass) validate(old *ClusterClass) error {
return nil
}

func (in ClusterClass) validateAllRefs() field.ErrorList {
func (in *ClusterClass) validateAllRefs() field.ErrorList {
var allErrs field.ErrorList

allErrs = append(allErrs, in.Spec.Infrastructure.validate(in.Namespace, field.NewPath("spec", "infrastructure"))...)
allErrs = append(allErrs, in.Spec.ControlPlane.LocalObjectTemplate.validate(in.Namespace, field.NewPath("spec", "controlPlane"))...)
allErrs = append(allErrs, in.Spec.Infrastructure.isValid(in.Namespace, field.NewPath("spec", "infrastructure"))...)
allErrs = append(allErrs, in.Spec.ControlPlane.LocalObjectTemplate.isValid(in.Namespace, field.NewPath("spec", "controlPlane"))...)
if in.Spec.ControlPlane.MachineInfrastructure != nil {
allErrs = append(allErrs, in.Spec.ControlPlane.MachineInfrastructure.validate(in.Namespace, field.NewPath("spec", "controlPlane", "machineInfrastructure"))...)
allErrs = append(allErrs, in.Spec.ControlPlane.MachineInfrastructure.isValid(in.Namespace, field.NewPath("spec", "controlPlane", "machineInfrastructure"))...)
}

for i, class := range in.Spec.Workers.MachineDeployments {
allErrs = append(allErrs, class.Template.Bootstrap.validate(in.Namespace, field.NewPath("spec", "workers", fmt.Sprintf("machineDeployments[%v]", i), "template", "bootstrap"))...)
allErrs = append(allErrs, class.Template.Infrastructure.validate(in.Namespace, field.NewPath("spec", "workers", fmt.Sprintf("machineDeployments[%v]", i), "template", "infrastructure"))...)
allErrs = append(allErrs, class.Template.Bootstrap.isValid(in.Namespace, field.NewPath("spec", "workers", fmt.Sprintf("machineDeployments[%v]", i), "template", "bootstrap"))...)
allErrs = append(allErrs, class.Template.Infrastructure.isValid(in.Namespace, field.NewPath("spec", "workers", fmt.Sprintf("machineDeployments[%v]", i), "template", "infrastructure"))...)
}

return allErrs
}

func (in ClusterClass) validateCompatibleSpecChanges(old *ClusterClass) field.ErrorList {
func (in *ClusterClass) validateCompatibleSpecChanges(old *ClusterClass) field.ErrorList {
var allErrs field.ErrorList

// in case of create, no changes to verify
Expand All @@ -138,33 +137,32 @@ func (in ClusterClass) validateCompatibleSpecChanges(old *ClusterClass) field.Er
return nil
}

// Ensure that the old MachineDeployments still exist.
allErrs = append(allErrs, in.validateMachineDeploymentsChanges(old)...)

if !reflect.DeepEqual(in.Spec.Infrastructure, old.Spec.Infrastructure) {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("spec", "infrastructure"),
in.Spec.Infrastructure,
"cannot be changed.",
),
)
}

if !reflect.DeepEqual(in.Spec.ControlPlane, old.Spec.ControlPlane) {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("spec", "controlPlane"),
in.Spec.Infrastructure,
"cannot be changed.",
),
)
// Validate changes to MachineDeployments.
allErrs = append(allErrs, in.validateMachineDeploymentsCompatibleChanges(old)...)

// Validate InfrastructureClusterTemplate changes in a compatible way.
allErrs = append(allErrs, in.Spec.Infrastructure.isCompatibleWith(
old.Spec.Infrastructure,
field.NewPath("spec", "infrastructure"),
)...)

// Validate control plane changes in a compatible way.
allErrs = append(allErrs, in.Spec.ControlPlane.LocalObjectTemplate.isCompatibleWith(
old.Spec.ControlPlane.LocalObjectTemplate,
field.NewPath("spec", "controlPlane"),
)...)

if in.Spec.ControlPlane.MachineInfrastructure != nil && old.Spec.ControlPlane.MachineInfrastructure != nil {
allErrs = append(allErrs, in.Spec.ControlPlane.MachineInfrastructure.isCompatibleWith(
*old.Spec.ControlPlane.MachineInfrastructure,
field.NewPath("spec", "controlPlane", "machineInfrastructure"),
)...)
}

return allErrs
}

func (in ClusterClass) validateMachineDeploymentsChanges(old *ClusterClass) field.ErrorList {
func (in *ClusterClass) validateMachineDeploymentsCompatibleChanges(old *ClusterClass) field.ErrorList {
var allErrs field.ErrorList

// Ensure no MachineDeployment class was removed.
Expand All @@ -181,25 +179,26 @@ func (in ClusterClass) validateMachineDeploymentsChanges(old *ClusterClass) fiel
}
}

// Ensure no previous MachineDeployment class was modified.
for _, class := range in.Spec.Workers.MachineDeployments {
// Ensure previous MachineDeployment class was modified in a compatible way.
fabriziopandini marked this conversation as resolved.
Show resolved Hide resolved
for i, class := range in.Spec.Workers.MachineDeployments {
for _, oldClass := range old.Spec.Workers.MachineDeployments {
if class.Class == oldClass.Class && !reflect.DeepEqual(class, oldClass) {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("spec", "workers", "machineDeployments"),
class,
"cannot be changed.",
),
)
if class.Class == oldClass.Class {
// NOTE: class.Template.Metadata and class.Template.Bootstrap are allowed to change;
// class.Template.Bootstrap are ensured syntactically correct by validateAllRefs.

// Validates class.Template.Infrastructure template changes in a compatible way
allErrs = append(allErrs, class.Template.Infrastructure.isCompatibleWith(
oldClass.Template.Infrastructure,
field.NewPath("spec", "workers", "machineDeployments").Index(i).Child("template", "infrastructure"),
)...)
}
}
}

return allErrs
}

func (r LocalObjectTemplate) validate(namespace string, pathPrefix *field.Path) field.ErrorList {
func (r *LocalObjectTemplate) isValid(namespace string, pathPrefix *field.Path) field.ErrorList {
var allErrs field.ErrorList

// check if ref is not nil.
Expand Down Expand Up @@ -261,7 +260,68 @@ func (r LocalObjectTemplate) validate(namespace string, pathPrefix *field.Path)
field.Invalid(
pathPrefix.Child("ref", "apiVersion"),
r.Ref.APIVersion,
"cannot be empty",
"value cannot be empty",
),
)
}

return allErrs
}

// isCompatibleWith checks if a reference is compatible with the old one.
// NOTE: this func assumes that r.isValid() is called before, thus both ref are defined and syntactically valid;
// also namespace are enforced to be the same of the ClusterClass.
func (r *LocalObjectTemplate) isCompatibleWith(old LocalObjectTemplate, pathPrefix *field.Path) field.ErrorList {
var allErrs field.ErrorList

// Check for nil Ref here to avoid panic.
if r.Ref == nil || old.Ref == nil {
return allErrs
}

// Ensure that the API Group and Kind does not change, while instead we allow version to change.
// TODO: In the future we might want to relax this requirement with some sort of opt-in behavior (e.g. annotation).

gv, err := schema.ParseGroupVersion(r.Ref.APIVersion)
if err != nil {
// NOTE this should never happen.
allErrs = append(allErrs,
field.Invalid(
pathPrefix.Child("ref", "apiVersion"),
r.Ref.APIVersion,
fmt.Sprintf("must be a valid apiVersion: %v", err),
),
)
}

oldGV, err := schema.ParseGroupVersion(old.Ref.APIVersion)
if err != nil {
// NOTE this should never happen.
allErrs = append(allErrs,
field.Invalid(
pathPrefix.Child("ref", "apiVersion"),
old.Ref.APIVersion,
fmt.Sprintf("must be a valid apiVersion: %v", err),
),
)
}

if gv.Group != oldGV.Group {
allErrs = append(allErrs,
field.Invalid(
pathPrefix.Child("ref", "apiVersion"),
r.Ref.APIVersion,
"apiGroup cannot be changed",
),
)
}

if r.Ref.Kind != old.Ref.Kind {
allErrs = append(allErrs,
field.Invalid(
pathPrefix.Child("ref", "kind"),
r.Ref.Kind,
"value cannot be changed",
),
)
}
Expand All @@ -270,15 +330,15 @@ func (r LocalObjectTemplate) validate(namespace string, pathPrefix *field.Path)
}

// classNames returns the set of MachineDeployment class names.
func (w WorkersClass) classNames() sets.String {
func (w *WorkersClass) classNames() sets.String {
classes := sets.NewString()
for _, class := range w.MachineDeployments {
classes.Insert(class.Class)
}
return classes
}

func (w WorkersClass) validateUniqueClasses(pathPrefix *field.Path) field.ErrorList {
func (w *WorkersClass) validateUniqueClasses(pathPrefix *field.Path) field.ErrorList {
var allErrs field.ErrorList

classes := sets.NewString()
Expand Down
Loading