Repository navigation
fix(compliance): admin-less UNKNOWN reasons say how to read the zones - #568
Merged
Merged
Conversation
Found on the admin-less fanon lab, the first live run of the check under rbac.scope=namespace. With the default zone label every rule read. With another zoneLabel, zones and skew were UNKNOWN, and: - The reason named the refused node list but not the fix. Each of the four node-list reasons now ends with the chart's rbac.nodeReads=true, and the custom-label one also says a zoneLabel that is the rack key reads the brokers' racks. - kates compliance check cut every note line at the terminal width, so the reason stopped at "cluster-scop...". A note line now wraps, three lines at most, so a reason reads whole and a long subject list stays short. - monitoring.podMonitor named "PodMonitors in any namespace" as its source when it could read only the Kafka namespace's. It now names that one. The book's compliance limits say rbac.nodeReads lifts the UNKNOWN.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The first live run of
kates compliance checkon an admin-less cluster: thefanonkind lab, Kates installed withrbac.scope=namespaceby an account that isn't cluster admin. Its service account may list neither nodes nor PodMonitors cluster-wide; it may list EndpointSlices and PodMonitors inkafka.zoneLabel, all 37 rules read: 36 PASS, 0 UNKNOWN. Zones came from the EndpointSlices (alpha: 1, gamma: 1, sigma: 1), andmonitoring.podMonitorpassed onkafka/krafter-kafka.zoneLabel: example.com/zone,brokers.zones,brokers.skew,controllers.zonesandcontrollers.skewwere UNKNOWN, as designed. But:...may not list nodes, which are cluster-scop....monitoring.podMonitor's source saidPodMonitors in any namespacewhen onlykafkawas read.What changed
ComplianceCheck: the four "may not list nodes" reasons end withThe chart's rbac.nodeReads=true lets it list nodes(feat(chart): opt-in rbac.nodeReads for conformity zones #538's opt-in ClusterRole). The custom-label one also says azoneLabelthat is the rack key reads the brokers' racks instead (feat(conformity): zones from broker racks when nodes are refused #537).ComplianceCheck:monitoring.podMonitortakes its source through the existingSupplier<String>overload ofrule(), so it readsKubernetes: PodMonitors in kafkaafter the namespace fallback.cli/output: a table note line wraps instead of being cut, three lines at most, the third ending in…when there is more.TableNotesis only used bykates compliance check, so no other table changes.docs/book/compliance.md, the limits list:rbac.nodeReads=truelifts the UNKNOWN for anotherzoneLabel.No MCP ref points into
ComplianceCheck.javaoroutput.go.Tests
ComplianceCheckTest: the two zone-reason assertions carry the new text; the PodMonitor fallback test asserts the source for both the UNKNOWN and the PASS case.TestTableNotes: a 400-character subject list wraps to three lines that each fit, the last ending in an ellipsis. NewTestTableNotesWrapAReason: a 150-character reason comes out whole.Local runs:
cd kates && ./mvnw test -Dquarkus.grpc.codegen.skip=true -Dquarkus.http.test-port=0: BUILD SUCCESS, 1790 tests, 0 failures, 0 errors, 3 skippedspotless:check(JDK 25): cleancd cli && go vet ./output ./cmd,go test -count=1 ./...: all passgo test -count=1 ./cmd -run 'TestMCPCaveat|Anchor': passgolangci-lint run --new-from-rev=origin/main ./...: 0 issuesscripts/check-book-style.sh: OKOnly CI can run the integration tests, including
ComplianceNamespacedLiveTest, the native image, Docker, CodeQL and the Quarto book render.Deliberately not done
monitoringone): that reason has no chart fix to point to, and Kates' own admin-less deploy keeps its PodMonitors inkafka.topics.minInsyncReplicasfailing onstrimzi.cruisecontrol.metrics, which Strimzi creates withmin.insync.replicas1. That isn't admin-less and gets its own change (anexcludein the examples, or the chart's Cruise Control topic config).