feat(components): check the ingress class the chart requests on OpenShift (budctl 0.3.2)

install.sh

@@ -16,7 +16,7 @@ set -eu
 # tag that disagrees with it. Pinned rather than resolved from the GitHub
 # API at runtime: the unauthenticated API allows 60 requests per hour per IP,
 # which one NATed customer site can exhaust between two engineers.
-VERSION="${BUDCTL_VERSION:-0.3.1}"
+VERSION="${BUDCTL_VERSION:-0.3.2}"
 REPO="BudEcosystem/budctl"
 
 die() { echo "install.sh: $*" >&2; exit 1; }

internal/checks/components.go

@@ -112,16 +112,22 @@ func init() {
 
 	engine.Register(&engine.Check{
 		ID: "components.ingress", Group: "components", Severity: engine.Block,
-		// DependsOn platform: OpenShift has no IngressClass to find — the
-		// Ingress Operator owns the path — and reporting a vanilla-only
-		// sub-check as PASS there (or the reverse) is the exact wrongness
-		// FRD-020 §5.0 exists to prevent.
-		DependsOn: []string{"platform"},
+		// DependsOn platform: on OpenShift the Ingress Operator owns the path and
+		// its router serves the openshift-default IngressClass, so the health
+		// question is asked of the operator rather than of a controller Service —
+		// reporting a vanilla-only sub-check as PASS there (or the reverse) is the
+		// exact wrongness FRD-020 §5.0 exists to prevent. DependsOn config: the
+		// class the chart's Ingresses request comes from the render, and
+		// `--only components` must still produce one.
+		DependsOn: []string{"platform", "config"},
 		Run: func(ctx context.Context, c *engine.Ctx) engine.Result {
 			ch := engine.Lookup("components.ingress")
 			if c.Kube == nil {
 				return ch.Skip("cluster unreachable: the ingress path was not inspected")
 			}
+			if unserved := componentUnservedIngressObjects(ctx, c); len(unserved) > 0 {
+				return componentUnservedIngressResult(ctx, c, ch, unserved)
+			}
 			if c.Platform.IsOpenShift() {
 				return openShiftIngress(ctx, c, ch)
 			}
@@ -369,10 +375,125 @@ func openShiftIngress(ctx context.Context, c *engine.Ctx, ch *engine.Check) engi
 		}
 	}
 
+	// A healthy router serves only the Ingresses that name its class. The Bud
+	// charts default to className traefik, and an Ingress naming a class this
+	// cluster does not have is turned into a Route by nothing — the install
+	// completes and not one hostname answers.
+	controllers := map[string]string{}
+	routerClasses, inventory := []string{}, []string{}
+	for _, cl := range c.Kube.List(ctx, "ingressclasses.networking.k8s.io", "") {
+		ctrl := cl.DigString("spec", "controller")
+		controllers[cl.Name()] = ctrl
+		inventory = append(inventory, fmt.Sprintf("%s (%s)", cl.Name(), ctrl))
+		if ctrl == openShiftIngressToRoute {
+			routerClasses = append(routerClasses, cl.Name())
+		}
+	}
+	routerClasses = Sorted(routerClasses)
+	ev.Output += "\ningressclasses: " + firstNonEmpty(strings.Join(Sorted(inventory), ", "), "none")
+	useClass := "openshift-default"
+	if len(routerClasses) > 0 {
+		useClass = routerClasses[0]
+	}
+
+	detail := []string{}
+	requested := componentRequestedIngressClasses(c)
+	switch {
+	case len(requested) > 0:
+		var missing, foreign, served []string
+		for _, want := range requested {
+			ctrl, ok := controllers[want]
+			switch {
+			case !ok:
+				missing = append(missing, want)
+			case ctrl != openShiftIngressToRoute:
+				foreign = append(foreign, fmt.Sprintf("%s (%s)", want, ctrl))
+			default:
+				served = append(served, want)
+			}
+		}
+		if len(missing) > 0 {
+			return ch.Fail(
+				fmt.Sprintf("the chart's Ingresses request IngressClass %s, which does not exist on this cluster: the OpenShift router turns an Ingress into a Route only for its own class, so those hostnames are never served",
+					stQuoteList(missing)),
+				fmt.Sprintf("set ingress.className: %s in the values of every chart that publishes an Ingress (bud, keycloak, seaweedfs, argocd), and ingress.isTraefik: false so no Traefik objects are rendered", useClass),
+				"IngressClasses on this cluster: "+firstNonEmpty(strings.Join(Sorted(inventory), ", "), "none"),
+			).WithEvidence(ev)
+		}
+		if len(foreign) > 0 {
+			return ch.FailAs(engine.Risk,
+				fmt.Sprintf("the chart's Ingresses request %s, served by a controller other than the OpenShift router: this check verified the router, not that controller, so those hostnames are unverified",
+					strings.Join(foreign, ", ")),
+				fmt.Sprintf("set ingress.className: %s so the router serves them, or confirm that controller is running and exposed on 80 and 443", useClass),
+			).WithEvidence(ev)
+		}
+		detail = append(detail, fmt.Sprintf("the chart's Ingresses request %s, which the OpenShift router serves (%s)", strings.Join(served, ", "), openShiftIngressToRoute))
+	case len(Rendered(c)) == 0:
+		why := "no --values"
+		if c.Opts.ChartDir != "" || len(c.Opts.ValuesFiles) > 0 {
+			why = "the chart did not render with the supplied values (config.render carries the error)"
+		}
+		detail = append(detail, fmt.Sprintf("%s: the class the chart's Ingresses request was not compared with this cluster's IngressClasses. The Bud charts default to ingress.className: traefik and ingress.isTraefik: true; on OpenShift set ingress.className: %s and ingress.isTraefik: false", why, useClass))
+	}
+
 	return ch.Pass(
 		fmt.Sprintf("the OpenShift Ingress Operator is Available and the default IngressController serves %d %s on *.%s", replicas, Plural(replicas, "replica", "replicas"), domain),
+		detail...,
 	).WithEvidence(ev).
-		Bounds("Route admission is not evaluated: the IngressController's domain and any routeSelector decide whether a given hostname is admitted, and `domains.*` checks DNS and the inbound path rather than admission policy.")
+		Bounds("Route admission is not evaluated: the IngressController's domain and any routeSelector decide whether a given hostname is admitted, and `domains.*` checks DNS and the inbound path rather than admission policy. Only the chart passed with --chart is rendered, so the class keycloak, seaweedfs and argocd request is not compared.")
+}
+
+// openShiftIngressToRoute is the controller behind OpenShift's own IngressClass:
+// the component that turns an Ingress into a Route the router serves.
+const openShiftIngressToRoute = "openshift.io/ingress-to-route"
+
+// componentUnservedIngressObjects lists the rendered objects that belong to
+// Traefik's own API when this cluster does not serve it. The Bud charts render
+// Traefik Middlewares unless ingress.isTraefik is false, and on a cluster without
+// Traefik's CRDs — OpenShift, or any cluster on ingress-nginx — the sync stops at
+// the first one with "no matches for kind Middleware".
+func componentUnservedIngressObjects(ctx context.Context, c *engine.Ctx) []string {
+	out := []string{}
+	for _, o := range Rendered(c) {
+		av := o.APIVersion()
+		group, _, _ := strings.Cut(av, "/")
+		if group != "traefik.io" && group != "traefik.containo.us" {
+			continue
+		}
+		if c.Kube.HasAPIVersion(ctx, av) {
+			continue
+		}
+		out = append(out, fmt.Sprintf("%s/%s (%s)", o.Kind(), o.Name(), av))
+	}
+	return Sorted(out)
+}
+
+func componentUnservedIngressResult(ctx context.Context, c *engine.Ctx, ch *engine.Check, unserved []string) engine.Result {
+	className := "a class this cluster has"
+	if c.Platform.IsOpenShift() {
+		className = "openshift-default"
+	} else if names := componentClassNames(componentClassStatesByName(ctx, c)); len(names) > 0 {
+		className = "one of " + strings.Join(names, ", ")
+	}
+	return ch.Fail(
+		fmt.Sprintf("the chart renders %d Traefik %s, but this cluster serves no Traefik API: the sync stops at the first one with \"no matches for kind\", so the Application never finishes applying",
+			len(unserved), Plural(len(unserved), "object", "objects")),
+		fmt.Sprintf("set ingress.isTraefik: false in the values of every chart that publishes an Ingress (bud, keycloak, seaweedfs, argocd), and ingress.className to %s", className),
+		unserved...,
+	).WithEvidence(engine.Evidence{
+		What:   "rendered objects in the traefik.io / traefik.containo.us API groups, against the API versions this cluster serves",
+		Output: strings.Join(unserved, "\n"),
+	})
+}
+
+// componentClassStatesByName is the IngressClass inventory without the endpoint
+// matching, for remedies that only need the names.
+func componentClassStatesByName(ctx context.Context, c *engine.Ctx) []componentClassState {
+	out := []componentClassState{}
+	for _, cl := range c.Kube.List(ctx, "ingressclasses.networking.k8s.io", "") {
+		out = append(out, componentClassState{name: cl.Name(), controller: cl.DigString("spec", "controller")})
+	}
+	return out
 }
 
 func vanillaIngress(ctx context.Context, c *engine.Ctx, ch *engine.Check) engine.Result {
@@ -382,7 +503,7 @@ func vanillaIngress(ctx context.Context, c *engine.Ctx, ch *engine.Check) engine
 			"no IngressClass exists: every Ingress the install creates stays unclaimed, so not one hostname the chart publishes is ever reachable",
 			"install an ingress controller before applying the ApplicationSets — neither of them provides one. Any controller is acceptable: "+
 				"`helm install ingress-nginx ingress-nginx/ingress-nginx -n ingress-nginx --create-namespace`, k3s's bundled Traefik, or the cloud provider's. "+
-				"Then set global.ingress.className to its class, or mark the class default with the `ingressclass.kubernetes.io/is-default-class: \"true\"` annotation.",
+				"Then set ingress.className to its class, or mark the class default with the `ingressclass.kubernetes.io/is-default-class: \"true\"` annotation.",
 		).WithEvidence(engine.Evidence{What: "kubectl get ingressclass", Output: "No resources found"})
 	}
 
@@ -434,7 +555,7 @@ func vanillaIngress(ctx context.Context, c *engine.Ctx, ch *engine.Check) engine
 			if !ok {
 				return ch.Fail(
 					fmt.Sprintf("the chart's Ingresses request IngressClass %q, which does not exist in this cluster: those Ingresses are claimed by no controller and none of their hostnames answers", want),
-					fmt.Sprintf("either set global.ingress.className in your values to a class that exists (%s), or install the controller that provides %q", strings.Join(componentClassNames(states), ", "), want),
+					fmt.Sprintf("either set ingress.className in your values to a class that exists (%s), or install the controller that provides %q", strings.Join(componentClassNames(states), ", "), want),
 				).WithEvidence(ev)
 			}
 			scope = append(scope, st)
@@ -483,7 +604,7 @@ func vanillaIngress(ctx context.Context, c *engine.Ctx, ch *engine.Check) engine
 		detail = append(detail, "other classes with no ready controller: "+strings.Join(controllerDown, "; "))
 	}
 	if !componentAnyDefault(states) && len(requested) == 0 {
-		detail = append(detail, "no IngressClass is marked default; set global.ingress.className in values so the chart's Ingresses name one explicitly")
+		detail = append(detail, "no IngressClass is marked default; set ingress.className in values so the chart's Ingresses name one explicitly")
 	}
 	return ch.Pass(
 		fmt.Sprintf("%s %s a controller with ready endpoints: %s",

internal/checks/components_test.go

@@ -768,8 +768,9 @@ func TestComponentsIngressOpenShiftPublishesRouterAddress(t *testing.T) {
 	}
 }
 
-// An OpenShift cluster must never be judged by IngressClass: seeding a dead one
-// alongside a healthy operator must not change the verdict.
+// An OpenShift cluster is not judged by an IngressClass the chart does not
+// request: seeding a dead one alongside a healthy operator must not change the
+// verdict.
 func TestComponentsIngressOpenShiftIgnoresIngressClasses(t *testing.T) {
 	f := openShift().
 		with("clusteroperators.config.openshift.io", "", componentsClusterOperator("ingress", true, false)).
@@ -780,6 +781,133 @@ func TestComponentsIngressOpenShiftIgnoresIngressClasses(t *testing.T) {
 	assertStatus(t, run(t, f, "components.ingress"), "PASS")
 }
 
+// componentsOpenShiftRouter is a healthy OpenShift ingress path with the
+// IngressClass the Ingress Operator creates for its router.
+func componentsOpenShiftRouter() *fakeCluster {
+	return openShift().
+		with("clusteroperators.config.openshift.io", "", componentsClusterOperator("ingress", true, false)).
+		with("ingresscontrollers.operator.openshift.io", "openshift-ingress-operator",
+			ingressController("default", "apps.ocp.example.com", 1)).
+		with("ingressclasses.networking.k8s.io", "", ingressClass("openshift-default", openShiftIngressToRoute))
+}
+
+func componentsTraefikMiddleware(name string) adapters.Object {
+	return adapters.Object{
+		"apiVersion": "traefik.io/v1alpha1", "kind": "Middleware",
+		"metadata": map[string]any{"name": name, "namespace": "bud"},
+		"spec":     map[string]any{"redirectScheme": map[string]any{"scheme": "https"}},
+	}
+}
+
+// The Bud charts default to className traefik. On OpenShift that Ingress is
+// turned into a Route by nothing, so a perfectly healthy router serves none of
+// the chart's hostnames — and the operator check alone called this PASS.
+func TestComponentsIngressOpenShiftBlocksWhenChartRequestsAClassTheRouterDoesNotServe(t *testing.T) {
+	r, _ := componentsRun(t, componentsOpenShiftRouter(), "components.ingress", func(c *engine.Ctx) {
+		c.Set(engine.KeyRenderedObjects, []adapters.Object{componentsIngressObject("bud-bud", "traefik", false)})
+	})
+	assertStatus(t, r, "BLOCK")
+	componentsAssertSummaryHas(t, r, `"traefik"`)
+	for _, want := range []string{"ingress.className: openshift-default", "ingress.isTraefik: false"} {
+		if !strings.Contains(r.Remedy, want) {
+			t.Fatalf("remedy does not name %q: %s", want, r.Remedy)
+		}
+	}
+}
+
+// The tcs openshift values: className openshift-default, served by the router.
+func TestComponentsIngressOpenShiftPassesWhenChartRequestsTheRouterClass(t *testing.T) {
+	r, _ := componentsRun(t, componentsOpenShiftRouter(), "components.ingress", func(c *engine.Ctx) {
+		c.Set(engine.KeyRenderedObjects, []adapters.Object{
+			componentsIngressObject("bud-bud", "openshift-default", false),
+			componentsIngressObject("bud-novu", "openshift-default", false),
+		})
+	})
+	assertStatus(t, r, "PASS")
+	componentsAssertDetailHas(t, r, "request openshift-default, which the OpenShift router serves")
+}
+
+// A class served by a controller installed beside the router exists, but the
+// router's health says nothing about it: unverified, not passed.
+func TestComponentsIngressOpenShiftRisksOnAClassServedByAnotherController(t *testing.T) {
+	f := componentsOpenShiftRouter().
+		with("ingressclasses.networking.k8s.io", "",
+			ingressClass("openshift-default", openShiftIngressToRoute),
+			ingressClass("nginx", "k8s.io/ingress-nginx"))
+	r, _ := componentsRun(t, f, "components.ingress", func(c *engine.Ctx) {
+		c.Set(engine.KeyRenderedObjects, []adapters.Object{componentsIngressObject("bud-bud", "nginx", false)})
+	})
+	assertStatus(t, r, "RISK")
+	componentsAssertSummaryHas(t, r, "k8s.io/ingress-nginx")
+}
+
+// Without --values the class cannot be compared, and the pass has to say what
+// the values must set rather than imply the chart defaults will work.
+func TestComponentsIngressOpenShiftPassWithoutValuesNamesTheClassToSet(t *testing.T) {
+	r := run(t, componentsOpenShiftRouter(), "components.ingress")
+	assertStatus(t, r, "PASS")
+	componentsAssertDetailHas(t, r, "ingress.className: openshift-default and ingress.isTraefik: false")
+}
+
+// --values given but the chart refused to render: "no --values" would send the
+// operator looking for a flag they already passed.
+func TestComponentsIngressOpenShiftSaysTheRenderFailedWhenValuesWereGiven(t *testing.T) {
+	f := componentsOpenShiftRouter().withOpts(func(o *engine.Options) {
+		o.ChartDir = "infra/charts/bud"
+		o.ValuesFiles = []string{"values.openshift.yaml"}
+	})
+	r := run(t, f, "components.ingress")
+	assertStatus(t, r, "PASS")
+	componentsAssertDetailHas(t, r, "the chart did not render with the supplied values")
+}
+
+// ingress.isTraefik defaults to true, so the charts render Traefik Middlewares.
+// Wherever Traefik's CRDs are not installed — OpenShift, or ingress-nginx — the
+// sync stops at the first one, before a single hostname is published.
+func TestComponentsIngressBlocksOnTraefikObjectsTheClusterDoesNotServe(t *testing.T) {
+	cases := []struct {
+		name      string
+		cluster   *fakeCluster
+		class     string
+		className string
+	}{
+		{"openshift", componentsOpenShiftRouter(), "openshift-default", "openshift-default"},
+		{"vanilla on ingress-nginx", componentsHealthyBase(), "nginx", "nginx"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			r, _ := componentsRun(t, tc.cluster, "components.ingress", func(c *engine.Ctx) {
+				c.Set(engine.KeyRenderedObjects, []adapters.Object{
+					componentsTraefikMiddleware("bud-https-redirect"),
+					componentsIngressObject("bud-bud", tc.class, false),
+				})
+			})
+			assertStatus(t, r, "BLOCK")
+			componentsAssertSummaryHas(t, r, "Traefik")
+			componentsAssertDetailHas(t, r, "Middleware/bud-https-redirect (traefik.io/v1alpha1)")
+			if !strings.Contains(r.Remedy, "ingress.isTraefik: false") || !strings.Contains(r.Remedy, tc.className) {
+				t.Fatalf("remedy must turn Traefik objects off and name a class this cluster has (%s): %s", tc.className, r.Remedy)
+			}
+		})
+	}
+}
+
+// Where Traefik's API is served, its Middlewares are exactly what the chart
+// needs — they must not be reported.
+func TestComponentsIngressAcceptsTraefikObjectsWhereTraefikIsServed(t *testing.T) {
+	f := componentsHealthyBase().
+		with("ingressclasses.networking.k8s.io", "", componentsDefaultIngressClass("traefik", "traefik.io/ingress-controller")).
+		with("endpointslices.discovery.k8s.io", "", componentsEndpointSlice("kube-system", "traefik", 2, 0))
+	f.apiGroups["traefik.io/v1alpha1"] = true
+	r, _ := componentsRun(t, f, "components.ingress", func(c *engine.Ctx) {
+		c.Set(engine.KeyRenderedObjects, []adapters.Object{
+			componentsTraefikMiddleware("bud-https-redirect"),
+			componentsIngressObject("bud-bud", "traefik", false),
+		})
+	})
+	assertStatus(t, r, "PASS")
+}
+
 // ---------------------------------------------------------------------------
 // components.metrics-server
 // ---------------------------------------------------------------------------