Skip to content

Commit

Permalink
updated error handling in getClustersUsingClusterClass
Browse files Browse the repository at this point in the history
  • Loading branch information
@killianmuldoon
killianmuldoon committed Nov 23, 2021
1 parent 8b253ec commit 350f5ec
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 88 deletions.
2 changes: 1 addition & 1 deletion internal/envtest/environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment {
if err := (&webhooks.Cluster{Client: mgr.GetClient()}).SetupWebhookWithManager(mgr); err != nil {
klog.Fatalf("unable to create webhook: %+v", err)
}
if err := (&webhooks.ClusterClass{}).SetupWebhookWithManager(mgr); err != nil {
if err := (&webhooks.ClusterClass{Client: mgr.GetClient()}).SetupWebhookWithManager(mgr); err != nil {
klog.Fatalf("unable to create webhook: %+v", err)
}
if err := (&clusterv1.Machine{}).SetupWebhookWithManager(mgr); err != nil {
Expand Down
58 changes: 28 additions & 30 deletions internal/webhooks/clusterclass.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,16 @@ func defaultNamespace(ref *corev1.ObjectReference, namespace string) {
}

// ValidateCreate implements validation for ClusterClass create.
func (webhook *ClusterClass) ValidateCreate(_ context.Context, obj runtime.Object) error {
func (webhook *ClusterClass) ValidateCreate(ctx context.Context, obj runtime.Object) error {
in, ok := obj.(*clusterv1.ClusterClass)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterClass but got a %T", obj))
}
return webhook.validate(nil, in)
return webhook.validate(ctx, nil, in)
}

// ValidateUpdate implements validation for ClusterClass update.
func (webhook *ClusterClass) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {
func (webhook *ClusterClass) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
newClusterClass, ok := newObj.(*clusterv1.ClusterClass)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterClass but got a %T", newObj))
Expand All @@ -99,7 +99,7 @@ func (webhook *ClusterClass) ValidateUpdate(_ context.Context, oldObj, newObj ru
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterClass but got a %T", oldObj))
}
return webhook.validate(oldClusterClass, newClusterClass)
return webhook.validate(ctx, oldClusterClass, newClusterClass)
}

// ValidateDelete implements validation for ClusterClass delete.
Expand All @@ -109,9 +109,9 @@ func (webhook *ClusterClass) ValidateDelete(ctx context.Context, obj runtime.Obj
return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterClass but got a %T", obj))
}

clusters, err := webhook.getClustersUsingClusterClass(clusterClass)
clusters, err := webhook.getClustersUsingClusterClass(ctx, clusterClass)
if err != nil {
return err
return apierrors.NewInternalError(errors.Wrapf(err, "could not retrieve Clusters using ClusterClass"))
}

if len(clusters) > 0 {
Expand All @@ -122,7 +122,7 @@ func (webhook *ClusterClass) ValidateDelete(ctx context.Context, obj runtime.Obj
return nil
}

func (webhook *ClusterClass) validate(old, new *clusterv1.ClusterClass) error {
func (webhook *ClusterClass) validate(ctx context.Context, old, new *clusterv1.ClusterClass) error {
// NOTE: ClusterClass and managed topologies are behind ClusterTopology feature gate flag; the web hook
// must prevent creating new objects new case the feature flag is disabled.
if !feature.Gates.Enabled(feature.ClusterTopology) {
Expand All @@ -131,7 +131,6 @@ func (webhook *ClusterClass) validate(old, new *clusterv1.ClusterClass) error {
"can be set only if the ClusterTopology feature flag is enabled",
)
}

var allErrs field.ErrorList

// Ensure all references are valid.
Expand All @@ -140,37 +139,36 @@ func (webhook *ClusterClass) validate(old, new *clusterv1.ClusterClass) error {
// Ensure all MachineDeployment classes are unique.
allErrs = append(allErrs, check.MachineDeploymentClassesAreUnique(new)...)

// Ensure spec changes are compatible.
allErrs = append(allErrs, check.ClusterClassesAreCompatible(old, new)...)

// Ensure no MachineDeploymentClass currently in use has been removed from the ClusterClass.
allErrs = append(allErrs, webhook.validateRemovedMachineDeploymentClassesAreNotUsed(old, new)...)
// If this is an update run additional validation.
if old != nil {
// Ensure spec changes are compatible.
allErrs = append(allErrs, check.ClusterClassesAreCompatible(old, new)...)

// Retrieve all clusters using the ClusterClass.
clusters, err := webhook.getClustersUsingClusterClass(ctx, old)
if err != nil {
allErrs = append(allErrs, field.InternalError(field.NewPath(""),
errors.Wrapf(err, "Clusters using ClusterClass %v can not be retrieved", old.Name)))
return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("ClusterClass").GroupKind(), new.Name, allErrs)
}

// Ensure no MachineDeploymentClass currently in use has been removed from the ClusterClass.
allErrs = append(allErrs, webhook.validateRemovedMachineDeploymentClassesAreNotUsed(clusters, old, new)...)
}
if len(allErrs) > 0 {
return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("ClusterClass").GroupKind(), new.Name, allErrs)
}
return nil
}

func (webhook *ClusterClass) validateRemovedMachineDeploymentClassesAreNotUsed(old, new *clusterv1.ClusterClass) field.ErrorList {
func (webhook *ClusterClass) validateRemovedMachineDeploymentClassesAreNotUsed(clusters []clusterv1.Cluster, old, new *clusterv1.ClusterClass) field.ErrorList {
var allErrs field.ErrorList
if old == nil {
return allErrs
}
removedClasses := webhook.removedMachineClasses(old, new)

removedClasses := webhook.removedMachineClasses(old, new)
// If no classes have been removed return early as no further checks are needed.
if len(removedClasses) == 0 {
return allErrs
return nil
}

// Retrieve all clusters using the ClusterClass.
clusters, err := webhook.getClustersUsingClusterClass(old)
if err != nil {
allErrs = append(allErrs, field.InternalError(field.NewPath(""),
errors.Wrapf(err, "Clusters using ClusterClass %v can not be retrieved", old.Name)))
}

// Error if any Cluster using the ClusterClass uses a MachineDeploymentClass that has been removed.
for _, c := range clusters {
for _, machineDeploymentTopology := range c.Spec.Topology.Workers.MachineDeployments {
Expand Down Expand Up @@ -207,17 +205,17 @@ func (webhook *ClusterClass) classNamesFromWorkerClass(w clusterv1.WorkersClass)
return classes
}

func (webhook *ClusterClass) getClustersUsingClusterClass(clusterClass *clusterv1.ClusterClass) ([]clusterv1.Cluster, error) {
func (webhook *ClusterClass) getClustersUsingClusterClass(ctx context.Context, clusterClass *clusterv1.ClusterClass) ([]clusterv1.Cluster, error) {
clusters := &clusterv1.ClusterList{}
clustersUsingClusterClass := []clusterv1.Cluster{}
err := webhook.Client.List(context.Background(), clusters,
err := webhook.Client.List(ctx, clusters,
client.MatchingLabels{
clusterv1.ClusterTopologyOwnedLabel: "",
},
client.InNamespace(clusterClass.Namespace),
)
if err != nil {
return nil, apierrors.NewInternalError(err)
return nil, err
}
for _, c := range clusters.Items {
if c.Spec.Topology.Class == clusterClass.Name {
Expand Down
Loading

0 comments on commit 350f5ec

Please sign in to comment.