From dfc56d7bcee6053c18f8161eb51b27994f51c131 Mon Sep 17 00:00:00 2001 From: atbagan Date: Mon, 18 May 2026 12:35:39 -0500 Subject: [PATCH 1/5] feat(controller): add ClusterCreationPolicy admission webhook and TC 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). --- cmd/main.go | 4 + go.mod | 5 + go.sum | 2 - .../webhook/clustercreationpolicy_webhook.go | 295 ++++++++++++++++++ internal/webhook/tenantcluster_webhook.go | 47 +++ 5 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 internal/webhook/clustercreationpolicy_webhook.go diff --git a/cmd/main.go b/cmd/main.go index 3f06e5e..782122c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -287,6 +287,10 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "ProviderConfig") os.Exit(1) } + if err = (&webhook.ClusterCreationPolicyValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ClusterCreationPolicy") + os.Exit(1) + } setupLog.Info("admission webhooks enabled") } else { setupLog.Info("admission webhooks disabled") diff --git a/go.mod b/go.mod index 00aa80a..b576261 100644 --- a/go.mod +++ b/go.mod @@ -74,3 +74,8 @@ require ( sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect sigs.k8s.io/yaml v1.6.0 // indirect ) + +// Replace directive for ADR-018 implementation chain. Points at the +// local butler-api checkout on branch feat/clustercreationpolicy-types. +// Removed at merge phase when butler-api PR tags are cut. +replace github.com/butlerdotdev/butler-api => ../butler-api diff --git a/go.sum b/go.sum index ffae5a0..e70b4df 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/butlerdotdev/butler-api v0.20.0 h1:dcDDiaejo5YN1a7ZZnsxJZhr/kDQk/IUcmPCYsT/PNA= -github.com/butlerdotdev/butler-api v0.20.0/go.mod h1:q/RjPFM/r2ELq4DqR78OiAxerKBvH5D4nTGsVLKbjqY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= diff --git a/internal/webhook/clustercreationpolicy_webhook.go b/internal/webhook/clustercreationpolicy_webhook.go new file mode 100644 index 0000000..b9bf057 --- /dev/null +++ b/internal/webhook/clustercreationpolicy_webhook.go @@ -0,0 +1,295 @@ +/* +Copyright 2026 The Butler Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + admissionv1 "k8s.io/api/admission/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + butlerv1alpha1 "github.com/butlerdotdev/butler-api/api/v1alpha1" +) + +// ClusterCreationPolicyValidator validates ClusterCreationPolicy +// resources on admission. See ADR-018 Decision section 7 and 8. +// +// Structural validation (exactly-one-of on spec.scope, enum values on +// OptionMode and OptionType) is enforced by the CRD schema before the +// webhook runs. This webhook layers referential integrity checks, +// mode-specific value requirements with operator-facing error messages, +// and intra-tier conflict detection that the schema cannot express. +// +// Provider-entry existence is intentionally not validated here. A +// provider outage at admission time would block legitimate policy +// authoring; stale references are surfaced by the deferred status +// reconciler instead (ADR-018 Deferred). +type ClusterCreationPolicyValidator struct { + Client client.Client + APIReader client.Reader +} + +// +kubebuilder:webhook:path=/validate-butler-butlerlabs-dev-v1alpha1-clustercreationpolicy,mutating=false,failurePolicy=fail,sideEffects=None,groups=butler.butlerlabs.dev,resources=clustercreationpolicies,verbs=create;update,versions=v1alpha1,name=vclustercreationpolicy.kb.io,admissionReviewVersions=v1 + +// Handle implements admission.Handler. +func (v *ClusterCreationPolicyValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + log.FromContext(ctx).Info("ccp admission request received", + "op", string(req.Operation), + "name", req.Name, + "user", req.UserInfo.Username, + ) + switch req.Operation { + case admissionv1.Create, admissionv1.Update: + policy := &butlerv1alpha1.ClusterCreationPolicy{} + if err := json.Unmarshal(req.Object.Raw, policy); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("decode ClusterCreationPolicy: %w", err)) + } + if err := v.validate(ctx, policy); err != nil { + return admission.Denied(err.Error()) + } + return admission.Allowed("") + default: + return admission.Allowed("") + } +} + +// validate runs the four referential and structural checks. Returns nil +// when the policy is admissible. +func (v *ClusterCreationPolicyValidator) validate(ctx context.Context, policy *butlerv1alpha1.ClusterCreationPolicy) error { + var errs field.ErrorList + + errs = append(errs, v.validateScopeReferences(ctx, policy)...) + errs = append(errs, validateOptionRules(policy)...) + conflictErrs, err := v.detectIntraTierConflict(ctx, policy) + if err != nil { + return fmt.Errorf("scan existing policies for conflict: %w", err) + } + errs = append(errs, conflictErrs...) + + if len(errs) == 0 { + return nil + } + return errs.ToAggregate() +} + +// validateScopeReferences checks that any referenced Team and +// environment exist. Uses APIReader (uncached) so a just-created Team +// is visible. +func (v *ClusterCreationPolicyValidator) validateScopeReferences(ctx context.Context, policy *butlerv1alpha1.ClusterCreationPolicy) field.ErrorList { + var errs field.ErrorList + scope := policy.Spec.Scope + + if scope.Team != nil { + team := &butlerv1alpha1.Team{} + if err := v.APIReader.Get(ctx, types.NamespacedName{Name: scope.Team.TeamRef.Name}, team); err != nil { + if apierrors.IsNotFound(err) { + errs = append(errs, field.NotFound( + field.NewPath("spec", "scope", "team", "teamRef", "name"), + scope.Team.TeamRef.Name, + )) + } else { + errs = append(errs, field.InternalError( + field.NewPath("spec", "scope", "team", "teamRef", "name"), + fmt.Errorf("fetch team: %w", err), + )) + } + } + return errs + } + + if scope.TeamAndEnvironment != nil { + team := &butlerv1alpha1.Team{} + teamPath := field.NewPath("spec", "scope", "teamAndEnvironment") + if err := v.APIReader.Get(ctx, types.NamespacedName{Name: scope.TeamAndEnvironment.TeamRef.Name}, team); err != nil { + if apierrors.IsNotFound(err) { + errs = append(errs, field.NotFound( + teamPath.Child("teamRef", "name"), + scope.TeamAndEnvironment.TeamRef.Name, + )) + return errs + } + errs = append(errs, field.InternalError( + teamPath.Child("teamRef", "name"), + fmt.Errorf("fetch team: %w", err), + )) + return errs + } + envFound := false + for _, env := range team.Spec.Environments { + if env.Name == scope.TeamAndEnvironment.EnvironmentName { + envFound = true + break + } + } + if !envFound { + errs = append(errs, field.NotFound( + teamPath.Child("environmentName"), + scope.TeamAndEnvironment.EnvironmentName, + )) + } + } + return errs +} + +// validateOptionRules enforces the mode-specific value requirements that +// produce better operator-facing error messages than CRD schema CEL. +// Per ADR-018 Decision section 7. +func validateOptionRules(policy *butlerv1alpha1.ClusterCreationPolicy) field.ErrorList { + var errs field.ErrorList + for optType, rule := range policy.Spec.Options { + path := field.NewPath("spec", "options").Key(string(optType)) + switch rule.Mode { + case butlerv1alpha1.OptionModePin, butlerv1alpha1.OptionModeAllowList: + if len(rule.Values) == 0 { + errs = append(errs, field.Required( + path.Child("values"), + fmt.Sprintf("at least one value is required when mode is %q", rule.Mode), + )) + } + case butlerv1alpha1.OptionModeDefault: + if rule.Default == "" { + errs = append(errs, field.Required( + path.Child("default"), + "default value is required when mode is \"default\"", + )) + } + case butlerv1alpha1.OptionModeRecommended: + if len(rule.Values) == 0 { + errs = append(errs, field.Required( + path.Child("values"), + "at least one value is required when mode is \"recommended\"", + )) + } + } + } + return errs +} + +// detectIntraTierConflict scans existing policies for a conflict with +// the policy being admitted. Conflict scope per ADR-018 Decision section +// 6 step 4: same tier (clusterWide vs team vs teamAndEnvironment), +// overlapping targetProviders, AND shared option-type keys. +func (v *ClusterCreationPolicyValidator) detectIntraTierConflict(ctx context.Context, policy *butlerv1alpha1.ClusterCreationPolicy) (field.ErrorList, error) { + var errs field.ErrorList + existing := &butlerv1alpha1.ClusterCreationPolicyList{} + if err := v.Client.List(ctx, existing); err != nil { + return nil, err + } + tier := tierOfScope(policy.Spec.Scope) + for i := range existing.Items { + other := &existing.Items[i] + if other.Name == policy.Name { + continue + } + if tierOfScope(other.Spec.Scope) != tier { + continue + } + if !sameScopeTarget(policy.Spec.Scope, other.Spec.Scope) { + continue + } + if !providersOverlap(policy.Spec.TargetProviders, other.Spec.TargetProviders) { + continue + } + shared := sharedOptionTypes(policy.Spec.Options, other.Spec.Options) + if len(shared) == 0 { + continue + } + errs = append(errs, field.Forbidden( + field.NewPath("spec", "options"), + fmt.Sprintf("conflicts with ClusterCreationPolicy %q on option types %v at the same tier", other.Name, shared), + )) + } + return errs, nil +} + +type scopeTier int + +const ( + stClusterWide scopeTier = iota + 1 + stTeam + stTeamAndEnv +) + +func tierOfScope(s butlerv1alpha1.PolicyScope) scopeTier { + switch { + case s.TeamAndEnvironment != nil: + return stTeamAndEnv + case s.Team != nil: + return stTeam + case s.ClusterWide != nil: + return stClusterWide + } + return 0 +} + +func sameScopeTarget(a, b butlerv1alpha1.PolicyScope) bool { + switch { + case a.ClusterWide != nil && b.ClusterWide != nil: + return true + case a.Team != nil && b.Team != nil: + return a.Team.TeamRef.Name == b.Team.TeamRef.Name + case a.TeamAndEnvironment != nil && b.TeamAndEnvironment != nil: + return a.TeamAndEnvironment.TeamRef.Name == b.TeamAndEnvironment.TeamRef.Name && + a.TeamAndEnvironment.EnvironmentName == b.TeamAndEnvironment.EnvironmentName + } + return false +} + +func providersOverlap(a, b []butlerv1alpha1.ProviderType) bool { + if len(a) == 0 || len(b) == 0 { + return true + } + for _, x := range a { + for _, y := range b { + if x == y { + return true + } + } + } + return false +} + +func sharedOptionTypes(a, b map[butlerv1alpha1.OptionType]butlerv1alpha1.OptionRule) []butlerv1alpha1.OptionType { + var shared []butlerv1alpha1.OptionType + for k := range a { + if _, ok := b[k]; ok { + shared = append(shared, k) + } + } + return shared +} + +// SetupWebhookWithManager registers the ClusterCreationPolicy validating +// webhook with the manager. +func (v *ClusterCreationPolicyValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { + v.Client = mgr.GetClient() + v.APIReader = mgr.GetAPIReader() + mgr.GetWebhookServer().Register( + "/validate-butler-butlerlabs-dev-v1alpha1-clustercreationpolicy", + &admission.Webhook{Handler: v}, + ) + return nil +} diff --git a/internal/webhook/tenantcluster_webhook.go b/internal/webhook/tenantcluster_webhook.go index 211dac8..1b02859 100644 --- a/internal/webhook/tenantcluster_webhook.go +++ b/internal/webhook/tenantcluster_webhook.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" butlerv1alpha1 "github.com/butlerdotdev/butler-api/api/v1alpha1" + policypkg "github.com/butlerdotdev/butler-api/pkg/policy" ) const defaultProviderConfigNamespace = "butler-system" @@ -229,6 +230,14 @@ func (v *TenantClusterValidator) validateCreateUpdate(ctx context.Context, tc, o } } + // ADR-018: enforce ClusterCreationPolicy at admission time. Runs + // after PC fetch because provider type drives field-map resolution. + policyErrs, err := v.validatePolicy(ctx, tc, pc.Spec.Provider) + if err != nil { + return nil, err + } + allErrs = append(allErrs, policyErrs...) + // Enforce per-team cluster limit. Create-only: on update the TC is // already counted and re-simulation would deadlock operations when // the cap is reduced below existing usage. @@ -650,6 +659,44 @@ func (v *TenantClusterValidator) validateEnvironment(ctx context.Context, tc, ol return allErrs, nil } +// validatePolicy resolves applicable ClusterCreationPolicy resources for +// the TC's (team, environment, provider) tuple and rejects when the TC +// spec violates a pin or allowList rule. default and recommended modes +// are presentation-only and do not enforce here. See ADR-018 Decision +// section 7. +func (v *TenantClusterValidator) validatePolicy(ctx context.Context, tc *butlerv1alpha1.TenantCluster, providerType butlerv1alpha1.ProviderType) (field.ErrorList, error) { + var errs field.ErrorList + + teamName := "" + if tc.Spec.TeamRef != nil { + teamName = tc.Spec.TeamRef.Name + } + envName := tc.Labels[butlerv1alpha1.LabelEnvironment] + + policies := &butlerv1alpha1.ClusterCreationPolicyList{} + if err := v.Client.List(ctx, policies); err != nil { + return nil, fmt.Errorf("list ClusterCreationPolicy: %w", err) + } + if len(policies.Items) == 0 { + return errs, nil + } + + resCtx := policypkg.ResolutionContext{ + TeamName: teamName, + EnvironmentName: envName, + ProviderType: providerType, + } + rules, sources := policypkg.ResolveWithSources(resCtx, policies.Items) + for optType, rule := range rules { + ok, fieldPath, msg := policypkg.ValidateAgainstRule(tc, providerType, optType, rule, sources[optType]) + if !ok { + path := field.NewPath(fieldPath) + errs = append(errs, field.Forbidden(path, msg)) + } + } + return errs, nil +} + // SetupWebhookWithManager registers the TenantCluster validating webhook // with the manager. Uses the raw admission.Handler path (not // CustomValidator) so the handler can read UserInfo from the admission From 6c488ca4f98b415a5c058e8b723aa795a1494f7f Mon Sep 17 00:00:00 2001 From: atbagan Date: Mon, 18 May 2026 19:42:59 -0500 Subject: [PATCH 2/5] fix(controller): pin mode requires exactly one value 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. --- internal/webhook/clustercreationpolicy_webhook.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/webhook/clustercreationpolicy_webhook.go b/internal/webhook/clustercreationpolicy_webhook.go index b9bf057..b5b6062 100644 --- a/internal/webhook/clustercreationpolicy_webhook.go +++ b/internal/webhook/clustercreationpolicy_webhook.go @@ -162,11 +162,19 @@ func validateOptionRules(policy *butlerv1alpha1.ClusterCreationPolicy) field.Err for optType, rule := range policy.Spec.Options { path := field.NewPath("spec", "options").Key(string(optType)) switch rule.Mode { - case butlerv1alpha1.OptionModePin, butlerv1alpha1.OptionModeAllowList: + case butlerv1alpha1.OptionModePin: + if len(rule.Values) != 1 { + errs = append(errs, field.Invalid( + path.Child("values"), + rule.Values, + fmt.Sprintf("mode \"pin\" requires exactly one value, got %d", len(rule.Values)), + )) + } + case butlerv1alpha1.OptionModeAllowList: if len(rule.Values) == 0 { errs = append(errs, field.Required( path.Child("values"), - fmt.Sprintf("at least one value is required when mode is %q", rule.Mode), + "at least one value is required when mode is \"allowList\"", )) } case butlerv1alpha1.OptionModeDefault: From ae79e680f4b79e64326d0503d8542850f2c84782 Mon Sep 17 00:00:00 2001 From: atbagan Date: Mon, 18 May 2026 20:07:04 -0500 Subject: [PATCH 3/5] refactor(controller): rename ClusterWide scope to PlatformWide 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. --- internal/webhook/clustercreationpolicy_webhook.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/webhook/clustercreationpolicy_webhook.go b/internal/webhook/clustercreationpolicy_webhook.go index b5b6062..54a8e47 100644 --- a/internal/webhook/clustercreationpolicy_webhook.go +++ b/internal/webhook/clustercreationpolicy_webhook.go @@ -198,7 +198,7 @@ func validateOptionRules(policy *butlerv1alpha1.ClusterCreationPolicy) field.Err // detectIntraTierConflict scans existing policies for a conflict with // the policy being admitted. Conflict scope per ADR-018 Decision section -// 6 step 4: same tier (clusterWide vs team vs teamAndEnvironment), +// 6 step 4: same tier (platformWide vs team vs teamAndEnvironment), // overlapping targetProviders, AND shared option-type keys. func (v *ClusterCreationPolicyValidator) detectIntraTierConflict(ctx context.Context, policy *butlerv1alpha1.ClusterCreationPolicy) (field.ErrorList, error) { var errs field.ErrorList @@ -236,7 +236,7 @@ func (v *ClusterCreationPolicyValidator) detectIntraTierConflict(ctx context.Con type scopeTier int const ( - stClusterWide scopeTier = iota + 1 + stPlatformWide scopeTier = iota + 1 stTeam stTeamAndEnv ) @@ -247,15 +247,15 @@ func tierOfScope(s butlerv1alpha1.PolicyScope) scopeTier { return stTeamAndEnv case s.Team != nil: return stTeam - case s.ClusterWide != nil: - return stClusterWide + case s.PlatformWide != nil: + return stPlatformWide } return 0 } func sameScopeTarget(a, b butlerv1alpha1.PolicyScope) bool { switch { - case a.ClusterWide != nil && b.ClusterWide != nil: + case a.PlatformWide != nil && b.PlatformWide != nil: return true case a.Team != nil && b.Team != nil: return a.Team.TeamRef.Name == b.Team.TeamRef.Name From 7ad6f0c155ae36b1ebf29ae9d428b45a15df6d6b Mon Sep 17 00:00:00 2001 From: atbagan Date: Mon, 18 May 2026 22:09:08 -0500 Subject: [PATCH 4/5] fix(webhook): reject unknown OptionType keys in ClusterCreationPolicy 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. --- .../webhook/clustercreationpolicy_webhook.go | 24 ++++ .../clustercreationpolicy_webhook_test.go | 115 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 internal/webhook/clustercreationpolicy_webhook_test.go diff --git a/internal/webhook/clustercreationpolicy_webhook.go b/internal/webhook/clustercreationpolicy_webhook.go index 54a8e47..37242a5 100644 --- a/internal/webhook/clustercreationpolicy_webhook.go +++ b/internal/webhook/clustercreationpolicy_webhook.go @@ -154,6 +154,17 @@ func (v *ClusterCreationPolicyValidator) validateScopeReferences(ctx context.Con return errs } +// validOptionTypes is the closed v1 set per ADR-018 Decision section 4. +// CRD OpenAPI v3 schema enforces enum validation on map values but not +// on map keys, so kubebuilder Enum on OptionType does not stop unknown +// keys from passing CRD admission. The webhook closes that gap. +var validOptionTypes = map[butlerv1alpha1.OptionType]bool{ + butlerv1alpha1.OptionTypeImage: true, + butlerv1alpha1.OptionTypeNetwork: true, + butlerv1alpha1.OptionTypeCluster: true, + butlerv1alpha1.OptionTypeStorageContainer: true, +} + // validateOptionRules enforces the mode-specific value requirements that // produce better operator-facing error messages than CRD schema CEL. // Per ADR-018 Decision section 7. @@ -161,6 +172,19 @@ func validateOptionRules(policy *butlerv1alpha1.ClusterCreationPolicy) field.Err var errs field.ErrorList for optType, rule := range policy.Spec.Options { path := field.NewPath("spec", "options").Key(string(optType)) + if !validOptionTypes[optType] { + errs = append(errs, field.NotSupported( + path, + string(optType), + []string{ + string(butlerv1alpha1.OptionTypeImage), + string(butlerv1alpha1.OptionTypeNetwork), + string(butlerv1alpha1.OptionTypeCluster), + string(butlerv1alpha1.OptionTypeStorageContainer), + }, + )) + continue + } switch rule.Mode { case butlerv1alpha1.OptionModePin: if len(rule.Values) != 1 { diff --git a/internal/webhook/clustercreationpolicy_webhook_test.go b/internal/webhook/clustercreationpolicy_webhook_test.go new file mode 100644 index 0000000..d15abb8 --- /dev/null +++ b/internal/webhook/clustercreationpolicy_webhook_test.go @@ -0,0 +1,115 @@ +/* +Copyright 2026 The Butler Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "strings" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + butlerv1alpha1 "github.com/butlerdotdev/butler-api/api/v1alpha1" +) + +// TestValidateOptionRules_UnknownOptionType verifies the CRD schema gap +// where kubebuilder Enum on OptionType applies to map values but not to +// map keys. The webhook closes that gap. +func TestValidateOptionRules_UnknownOptionType(t *testing.T) { + cases := []struct { + name string + policy *butlerv1alpha1.ClusterCreationPolicy + wantErr bool + errContains string + }{ + { + name: "known option type passes", + policy: &butlerv1alpha1.ClusterCreationPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "ok-image-pin"}, + Spec: butlerv1alpha1.ClusterCreationPolicySpec{ + Scope: butlerv1alpha1.PolicyScope{ + PlatformWide: &butlerv1alpha1.PlatformWideScope{}, + }, + TargetProviders: []butlerv1alpha1.ProviderType{butlerv1alpha1.ProviderTypeNutanix}, + Options: map[butlerv1alpha1.OptionType]butlerv1alpha1.OptionRule{ + butlerv1alpha1.OptionTypeImage: { + Mode: butlerv1alpha1.OptionModePin, + Values: []string{"img-uuid"}, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "unknown option type rejected", + policy: &butlerv1alpha1.ClusterCreationPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "bad-unknown-type"}, + Spec: butlerv1alpha1.ClusterCreationPolicySpec{ + Scope: butlerv1alpha1.PolicyScope{ + PlatformWide: &butlerv1alpha1.PlatformWideScope{}, + }, + TargetProviders: []butlerv1alpha1.ProviderType{butlerv1alpha1.ProviderTypeNutanix}, + Options: map[butlerv1alpha1.OptionType]butlerv1alpha1.OptionRule{ + butlerv1alpha1.OptionType("unknownOptionType"): { + Mode: butlerv1alpha1.OptionModePin, + Values: []string{"v"}, + }, + }, + }, + }, + wantErr: true, + errContains: "Unsupported value", + }, + { + name: "pin with two values rejected", + policy: &butlerv1alpha1.ClusterCreationPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "bad-pin-two-values"}, + Spec: butlerv1alpha1.ClusterCreationPolicySpec{ + Scope: butlerv1alpha1.PolicyScope{ + PlatformWide: &butlerv1alpha1.PlatformWideScope{}, + }, + Options: map[butlerv1alpha1.OptionType]butlerv1alpha1.OptionRule{ + butlerv1alpha1.OptionTypeImage: { + Mode: butlerv1alpha1.OptionModePin, + Values: []string{"a", "b"}, + }, + }, + }, + }, + wantErr: true, + errContains: "exactly one value", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := validateOptionRules(tc.policy) + if tc.wantErr { + if len(errs) == 0 { + t.Fatalf("expected validation error, got none") + } + if tc.errContains != "" && !strings.Contains(errs.ToAggregate().Error(), tc.errContains) { + t.Errorf("error message %q does not contain %q", errs.ToAggregate().Error(), tc.errContains) + } + return + } + if len(errs) > 0 { + t.Fatalf("expected no error, got: %s", errs.ToAggregate().Error()) + } + }) + } +} From 62fbe2324f511d21c1f404d8e3bce487c39e5d55 Mon Sep 17 00:00:00 2001 From: atbagan Date: Tue, 19 May 2026 08:33:39 -0500 Subject: [PATCH 5/5] chore: bump butler-api to v0.21.0 Remove local replace directive now that butler-api v0.21.0 is published with ClusterCreationPolicy types and resolver. --- go.mod | 7 +------ go.sum | 2 ++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index b576261..1d666e4 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/butlerdotdev/butler-controller go 1.24.6 require ( - github.com/butlerdotdev/butler-api v0.20.0 + github.com/butlerdotdev/butler-api v0.21.0 github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 github.com/prometheus/client_golang v1.22.0 @@ -74,8 +74,3 @@ require ( sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect sigs.k8s.io/yaml v1.6.0 // indirect ) - -// Replace directive for ADR-018 implementation chain. Points at the -// local butler-api checkout on branch feat/clustercreationpolicy-types. -// Removed at merge phase when butler-api PR tags are cut. -replace github.com/butlerdotdev/butler-api => ../butler-api diff --git a/go.sum b/go.sum index e70b4df..2854157 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/butlerdotdev/butler-api v0.21.0 h1:zVI4doowHOHWUMVV6s+N0LcrbGasvNA5kP26Sf+kWnc= +github.com/butlerdotdev/butler-api v0.21.0/go.mod h1:q/RjPFM/r2ELq4DqR78OiAxerKBvH5D4nTGsVLKbjqY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=