Skip to content

SetupWithManager silently ignores Discovery errors when checking for TenantControlPlane CRD #340

Description

@jds9090

Summary

In SetupWithManager of KamajiControlPlaneReconciler, the result of the
API Discovery call is only checked for success — any error is silently
swallowed and treated as "CRD not found":

// controllers/kamajicontrolplane_controller.go
if _, rsErr := cs.Discovery().ServerResourcesForGroupVersion(kamajiv1alpha1.GroupVersion.String()); rsErr == nil {
    ctrlBuilder = ctrlBuilder.Owns(&kamajiv1alpha1.TenantControlPlane{})
}

Problem

This conflates two fundamentally different failure modes:

  • CRD not installed (404) — intentional and valid for ExternalClusterReference
    deployments where TenantControlPlane lives on a remote cluster, not the
    management cluster. Skipping Owns here is correct.
  • Transient API server error, network timeout, or RBAC denial — the CRD
    existence is unknown. Silently skipping Owns leads to a controller that
    starts successfully but never reacts to TenantControlPlane changes, causing
    stale status on KamajiControlPlane objects with no error signal.

Unlike the adjacent kubernetes.NewForConfig call, which propagates its error
consistently:

cs, csErr := kubernetes.NewForConfig(mgr.GetConfig())
if csErr != nil {
    return fmt.Errorf("%w: %s", ErrClientSetCreation, csErr.Error())
}

the Discovery error is never surfaced.

Expected Behavior

  • 404 / NotFound → skip Owns, continue (ExternalClusterReference case).
  • Any other error → return it from SetupWithManager, consistent with the
    existing ErrClientSetCreation pattern.
_, rsErr := cs.Discovery().ServerResourcesForGroupVersion(kamajiv1alpha1.GroupVersion.String())
switch {
case rsErr == nil:
    ctrlBuilder = ctrlBuilder.Owns(&kamajiv1alpha1.TenantControlPlane{})
case k8serrors.IsNotFound(rsErr):
    // CRD absent — valid for ExternalClusterReference deployments
default:
    return fmt.Errorf("%w: %s", ErrTenantControlPlaneDiscovery, rsErr.Error())
}

A new sentinel error ErrTenantControlPlaneDiscovery should be added to
controllers/errors.go alongside the existing ones.

Impact

Operators using local (non-ExternalClusterReference) deployments who experience
a transient API server issue at controller startup will end up with a degraded
controller that cannot be detected without inspecting watch registrations.
Restarting the controller is required to recover, but there is no indication
that a restart is needed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions