Skip to content

Commit

Permalink
Add webhook warning for missing ClusterClass
Browse files Browse the repository at this point in the history
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
  • Loading branch information
killianmuldoon committed Jul 5, 2023
1 parent 730f63d commit 94e4093
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 44 deletions.
77 changes: 55 additions & 22 deletions internal/webhooks/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func (webhook *Cluster) ValidateDelete(_ context.Context, _ runtime.Object) (adm

func (webhook *Cluster) validate(ctx context.Context, oldCluster, newCluster *clusterv1.Cluster) (admission.Warnings, error) {
var allErrs field.ErrorList
var allWarnings admission.Warnings
// The Cluster name is used as a label value. This check ensures that names which are not valid label values are rejected.
if errs := validation.IsValidLabelValue(newCluster.Name); len(errs) != 0 {
for _, err := range errs {
Expand Down Expand Up @@ -191,7 +192,9 @@ func (webhook *Cluster) validate(ctx context.Context, oldCluster, newCluster *cl

// Validate the managed topology, if defined.
if newCluster.Spec.Topology != nil {
allErrs = append(allErrs, webhook.validateTopology(ctx, oldCluster, newCluster, topologyPath)...)
topologyWarnings, topologyErrs := webhook.validateTopology(ctx, oldCluster, newCluster, topologyPath)
allWarnings = append(allWarnings, topologyWarnings...)
allErrs = append(allErrs, topologyErrs...)
}

// On update.
Expand All @@ -206,16 +209,18 @@ func (webhook *Cluster) validate(ctx context.Context, oldCluster, newCluster *cl
}

if len(allErrs) > 0 {
return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Cluster").GroupKind(), newCluster.Name, allErrs)
return allWarnings, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Cluster").GroupKind(), newCluster.Name, allErrs)
}
return nil, nil
return allWarnings, nil
}

func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newCluster *clusterv1.Cluster, fldPath *field.Path) field.ErrorList {
func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newCluster *clusterv1.Cluster, fldPath *field.Path) (admission.Warnings, field.ErrorList) {
var allWarnings admission.Warnings

// NOTE: ClusterClass and managed topologies are behind ClusterTopology feature gate flag; the web hook
// must prevent the usage of Cluster.Topology in case the feature flag is disabled.
if !feature.Gates.Enabled(feature.ClusterTopology) {
return field.ErrorList{
return allWarnings, field.ErrorList{
field.Forbidden(
fldPath,
"can be set only if the ClusterTopology feature flag is enabled",
Expand All @@ -234,6 +239,8 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
"class cannot be empty",
),
)
// Return early if there is no defined class to validate.
return allWarnings, allErrs
}

// version should be valid.
Expand Down Expand Up @@ -268,18 +275,11 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
}

// Get the ClusterClass referenced in the Cluster.
clusterClass, clusterClassPollErr := webhook.pollClusterClassForCluster(ctx, newCluster)
if clusterClassPollErr != nil &&
// If the error is anything other than "NotFound" or "NotReconciled" return all errors at this point.
!(apierrors.IsNotFound(clusterClassPollErr) || errors.Is(clusterClassPollErr, errClusterClassNotReconciled)) {
allErrs = append(
allErrs, field.InternalError(
fldPath.Child("class"),
clusterClassPollErr))
return allErrs
}
clusterClass, warnings, clusterClassPollErr := webhook.validateClusterClassExistsAndIsReconciled(ctx, newCluster)
allWarnings = append(allWarnings, warnings...)

// If there's no error validate the Cluster based on the ClusterClass.
if clusterClassPollErr == nil {
// If there's no error validate the Cluster based on the ClusterClass.
allErrs = append(allErrs, ValidateClusterForClusterClass(newCluster, clusterClass)...)
}
if oldCluster != nil { // On update
Expand All @@ -290,13 +290,13 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
allErrs, field.InternalError(
fldPath.Child("class"),
clusterClassPollErr))
return allErrs
return allWarnings, allErrs
}

// Topology or Class can not be added on update unless ClusterTopologyUnsafeUpdateClassNameAnnotation is set.
if oldCluster.Spec.Topology == nil || oldCluster.Spec.Topology.Class == "" {
if _, ok := newCluster.Annotations[clusterv1.ClusterTopologyUnsafeUpdateClassNameAnnotation]; ok {
return allErrs
return allWarnings, allErrs
}

allErrs = append(
Expand All @@ -307,7 +307,7 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
),
)
// return early here if there is no class to compare.
return allErrs
return allWarnings, allErrs
}

// Version could only be increased.
Expand Down Expand Up @@ -372,14 +372,14 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
oldCluster.Spec.Topology.Class, newCluster.Spec.Topology.Class)))

// Return early with errors if the ClusterClass can't be retrieved.
return allErrs
return allWarnings, allErrs
}

// Check if the new and old ClusterClasses are compatible with one another.
allErrs = append(allErrs, check.ClusterClassesAreCompatible(oldClusterClass, clusterClass)...)
}
}
return allErrs
return allWarnings, allErrs
}

func validateMachineHealthChecks(cluster *clusterv1.Cluster, clusterClass *clusterv1.ClusterClass) field.ErrorList {
Expand Down Expand Up @@ -551,11 +551,44 @@ func ValidateClusterForClusterClass(cluster *clusterv1.Cluster, clusterClass *cl
return allErrs
}

// validateClusterClassExistsAndIsReconciled will try to get the ClusterClass referenced in the Cluster. If it does not exist or is not reconciled it will add a warning.
// In any other case it will return an error.
func (webhook *Cluster) validateClusterClassExistsAndIsReconciled(ctx context.Context, newCluster *clusterv1.Cluster) (*clusterv1.ClusterClass, admission.Warnings, error) {
var allWarnings admission.Warnings
clusterClass, clusterClassPollErr := webhook.pollClusterClassForCluster(ctx, newCluster)
if clusterClassPollErr != nil {
// Add a warning if the Class does not exist or if it has not been successfully reconciled.
switch {
case apierrors.IsNotFound(clusterClassPollErr):
allWarnings = append(allWarnings,
fmt.Sprintf(
"Cluster refers to ClusterClass %s in the topology but it does not exist. "+
"Cluster topology has not been fully validated. "+
"The ClusterClass must be created to reconcile the Cluster", newCluster.Spec.Topology.Class),
)
case errors.Is(clusterClassPollErr, errClusterClassNotReconciled):
allWarnings = append(allWarnings,
fmt.Sprintf(
"Cluster refers to ClusterClass %s but this object is not yet fully reconciled. "+
"Cluster topology has not been fully validated. ", newCluster.Spec.Topology.Class),
)
// If there's any other error return a generic warning with the error message.
default:
allWarnings = append(allWarnings,
fmt.Sprintf(
"Cluster refers to ClusterClass %s in the topology but it could not be retrieved. "+
"Cluster topology has not been fully validated. "+
": %s", newCluster.Spec.Topology.Class, clusterClassPollErr.Error()),
)
}
}
return clusterClass, allWarnings, clusterClassPollErr
}

// pollClusterClassForCluster will retry getting the ClusterClass referenced in the Cluster for two seconds.
func (webhook *Cluster) pollClusterClassForCluster(ctx context.Context, cluster *clusterv1.Cluster) (*clusterv1.ClusterClass, error) {
clusterClass := &clusterv1.ClusterClass{}
var clusterClassPollErr error
// TODO: Add a webhook warning if the ClusterClass is not up to date or not found.
_ = wait.PollUntilContextTimeout(ctx, 200*time.Millisecond, 2*time.Second, true, func(ctx context.Context) (bool, error) {
if clusterClassPollErr = webhook.Client.Get(ctx, client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Spec.Topology.Class}, clusterClass); clusterClassPollErr != nil {
return false, nil //nolint:nilerr
Expand Down
Loading

0 comments on commit 94e4093

Please sign in to comment.