Repository navigation
feat(controller): add ClusterCreationPolicy webhook + TenantCluster enforcement - #105
Merged
Merged
Conversation
…enforcement Implements ADR-018 enforcement on butler-controller. Two surfaces: ClusterCreationPolicyValidator (new file internal/webhook/clustercreationpolicy_webhook.go): - Referenced Team exists when scope is team or teamAndEnvironment - Referenced environment exists on the Team when scope is teamAndEnvironment - Mode-specific value requirements with operator-facing error messages (pin/allowList require non-empty Values; default requires non-empty Default; recommended requires non-empty Values) - Intra-tier conflict detection scoped to overlapping targetProviders AND shared option-type keys, naming the conflicting policy Provider-entry existence is intentionally not validated at admission time. A provider outage would block legitimate policy authoring; stale references surface via the deferred status reconciler. TenantClusterValidator extended (internal/webhook/tenantcluster_webhook.go): - validatePolicy method calls into butler-api/pkg/policy to resolve applicable rules for (team, environment, provider) and enforce pin/allowList rejections via the fieldmap. Runs after ProviderConfig fetch because provider type drives field dispatch. - Rejections name the policy via ResolveWithSources and include the TenantCluster spec field path. Webhook registered in cmd/main.go alongside the four existing validators. go.mod carries a replace directive to ../butler-api during the implementation chain. The directive will be removed at merge phase once butler-api ships a tagged release containing pkg/policy. Depends on butlerdotdev/butler-api#45 (types + pkg/policy resolver).
Original validateOptionRules treated pin and allowList identically: non-empty Values required. By the ADR-018 name "pin" means a single fixed value, not a set; the webhook now enforces len(Values) == 1 for pin and continues to require non-empty Values for allowList. Companion to the form-side fix in butler-console PR #76. ADR amendment to clarify the semantics will land separately.
2 tasks done
atbagan
added a commit
that referenced
this pull request
May 19, 2026
The original §5 mode-semantics text allowed pin to carry one or many Values, with the modal switching between a read-only single entry and a list. That made pin and allowList only differ by the optional Default field, which is a weak distinction the form implementation surfaced as a real UX problem (operators added multiple values to a "pin" rule when they expected a singleton). Update §5 pin definition: Values must have length 1; the modal renders the entry as a single read-only selection. Use allowList when the value count is unbounded; use pin only when the single- value semantics are deliberate. Mode-behavior table row for pin updated to "Yes (single value)" and "Renders read-only" so the orthogonal grid reads cleanly: pin: single value, enforcing default: single value, advisory allowList: multi value, enforcing recommended: multi value, advisory butler-controller webhook (PR #105) enforces len(Values) == 1 for pin. butler-console form (PR #76) renders pin as a single-select. No design change beyond clarifying intent that was already present in the name "pin".
Companion to butler-api PR #45 commit 76b3e2b which renames the scope variant across the API surface. The webhook references the renamed field and tier name. - scope.ClusterWide -> scope.PlatformWide on type access - stClusterWide -> stPlatformWide internal tier enum - Comment text in intra-tier conflict detection Conflict detection logic, mode-specific validation, and referential integrity checks are unchanged.
The CRD's OpenAPI v3 schema enforces enum validation on map values but not on map keys. A policy with an unknown OptionType key (for example "unknownOptionType") passes CRD admission silently. Extending ClusterCreationPolicyValidator.validateOptionRules to reject unknown OptionType keys closes the admission gap. The runtime resolver already ignores unknown keys (iterates Options by typed key), so this fix is admission-only, not affecting runtime correctness. Three-pass review on this PR caught that the fix promised in ADR-018 e2e testing did not make it into the implementation. Now landed. Added clustercreationpolicy_webhook_test.go with table-driven cases covering the unknown-key rejection, a known-key happy path, and the existing pin-singleton rule for symmetry. Platform finding worth capturing: kubebuilder enum markers on map keys are advisory only in CRD OpenAPI v3 schema. Webhook validation is required for enforcement of map-key sets.
Remove local replace directive now that butler-api v0.21.0 is published with ClusterCreationPolicy types and resolver.
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
1 participant
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.
Third PR in the five-repo ADR-018 implementation chain.
What this adds
validatePolicymethod resolves applicable policies viabutler-api/pkg/policy, uses the fieldmap to readtc.Spec.InfrastructureOverride.{Nutanix,Harvester}.*, rejects on pin or allowList violation with the policy name and field path.cmd/main.go.Depends on
pkg/policyresolver and fieldmap)Replace directives in flight
go.modcarriesreplace github.com/butlerdotdev/butler-api => ../butler-apiduring the implementation chain. The directive is removed at merge phase once butler-api ships a tagged release containingpkg/policy.Implementation chain status
Verification
go build ./...cleango vet ./...cleango test ./internal/webhook/...passes (existing webhook tests unaffected)