diff --git a/pkg/webhook/core.oam.dev/v1beta1/application/validation.go b/pkg/webhook/core.oam.dev/v1beta1/application/validation.go index ab658345f..d080f59de 100644 --- a/pkg/webhook/core.oam.dev/v1beta1/application/validation.go +++ b/pkg/webhook/core.oam.dev/v1beta1/application/validation.go @@ -20,12 +20,14 @@ import ( "context" "fmt" "reflect" + "strings" "time" "github.com/kubevela/pkg/controller/sharding" "github.com/kubevela/pkg/util/singleton" authv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" @@ -36,6 +38,7 @@ import ( "github.com/oam-dev/kubevela/pkg/appfile" "github.com/oam-dev/kubevela/pkg/features" "github.com/oam-dev/kubevela/pkg/oam" + oamutil "github.com/oam-dev/kubevela/pkg/oam/util" ) // ValidateWorkflow validates the Application workflow @@ -452,6 +455,121 @@ func (h *ValidatingHandler) ValidateAnnotations(_ context.Context, app *v1beta1. return annotationsErrs } +// ValidateTraitConflicts validates TraitDefinition.spec.conflictsWith for traits +// attached to the same component. Kept outside ValidateComponents so it still +// runs when sharding skips component schematic validation. +func (h *ValidatingHandler) ValidateTraitConflicts(ctx context.Context, app *v1beta1.Application) field.ErrorList { + var errs field.ErrorList + defCtx := oamutil.SetNamespaceInCtx(ctx, app.Namespace) + + // Cache resolved TraitDefinitions across components so each trait type is fetched once. + defCache := make(map[string]*v1beta1.TraitDefinition) + getTraitDefinition := func(traitType string) (*v1beta1.TraitDefinition, error) { + if def, ok := defCache[traitType]; ok { + return def, nil + } + def := &v1beta1.TraitDefinition{} + if err := oamutil.GetCapabilityDefinition(defCtx, h.Client, def, traitType, app.GetAnnotations()); err != nil { + if errors.IsNotFound(err) { + // Existence is validated elsewhere; missing defs are skipped here. + defCache[traitType] = nil + return nil, nil + } + return nil, err + } + defCache[traitType] = def + return def, nil + } + + for compIdx, comp := range app.Spec.Components { + if len(comp.Traits) < 2 { + continue + } + + type attachedTrait struct { + traitIdx int + def *v1beta1.TraitDefinition + } + var attached []attachedTrait + for i, trait := range comp.Traits { + def, err := getTraitDefinition(trait.Type) + if err != nil { + // Fail closed so unresolved definitions cannot bypass conflict checks, but + // log so operators can distinguish transient API/cache failures from policy rejects. + klog.Errorf("Failed to resolve TraitDefinition %q for conflict validation: %v", trait.Type, err) + errs = append(errs, field.InternalError( + field.NewPath("spec", "components").Index(compIdx).Child("traits").Index(i).Child("type"), + err)) + continue + } + if def != nil { + attached = append(attached, attachedTrait{traitIdx: i, def: def}) + } + } + + for i := 0; i < len(attached); i++ { + for j := i + 1; j < len(attached); j++ { + first, second := attached[i], attached[j] + // Matching is unidirectional: either side declaring the other is enough. + if traitConflictsWith(first.def, second.def) || traitConflictsWith(second.def, first.def) { + errs = append(errs, field.Invalid( + field.NewPath("spec", "components").Index(compIdx).Child("traits").Index(second.traitIdx).Child("type"), + second.def.Name, + fmt.Sprintf("trait %q conflicts with trait %q on component %q", first.def.Name, second.def.Name, comp.Name))) + } + } + } + } + + return errs +} + +// traitConflictsWith reports whether def's conflictsWith rules match target. +func traitConflictsWith(def, target *v1beta1.TraitDefinition) bool { + for _, rule := range def.Spec.ConflictsWith { + if traitConflictRuleMatches(rule, target) { + return true + } + } + return false +} + +// traitConflictRuleMatches checks a single conflictsWith rule against a TraitDefinition. +// Supported rule forms (see TraitDefinitionSpec.ConflictsWith docs): +// - "*" matches any trait +// - "" matches the trait definition's name +// - "" matches the trait's referenced CRD name (definitionRef.name) +// - "*." matches any CRD in the given API group +// - "labelSelector:" matches the trait definition's labels against a label selector +func traitConflictRuleMatches(rule string, target *v1beta1.TraitDefinition) bool { + switch { + case rule == "*": + return true + case strings.HasPrefix(rule, "labelSelector:"): + selector, err := labels.Parse(strings.TrimPrefix(rule, "labelSelector:")) + if err != nil { + return false + } + return selector.Matches(labels.Set(target.GetLabels())) + case rule == target.Name: + return true + case target.Spec.Reference.Name != "" && rule == target.Spec.Reference.Name: + return true + case strings.HasPrefix(rule, "*."): + group := strings.TrimPrefix(rule, "*.") + refName := target.Spec.Reference.Name + if refName == "" { + return false + } + if dot := strings.Index(refName, "."); dot >= 0 { + return refName[dot+1:] == group + } + return false + default: + return false + } +} + // ValidateCreate validates the Application on creation func (h *ValidatingHandler) ValidateCreate(ctx context.Context, app *v1beta1.Application, req admission.Request) field.ErrorList { var errs field.ErrorList @@ -460,6 +578,7 @@ func (h *ValidatingHandler) ValidateCreate(ctx context.Context, app *v1beta1.App errs = append(errs, h.ValidateDefinitionPermissions(ctx, app, req)...) errs = append(errs, h.ValidateWorkflow(ctx, app)...) errs = append(errs, h.ValidateComponents(ctx, app)...) + errs = append(errs, h.ValidateTraitConflicts(ctx, app)...) return errs } diff --git a/pkg/webhook/core.oam.dev/v1beta1/application/validation_conflicts_test.go b/pkg/webhook/core.oam.dev/v1beta1/application/validation_conflicts_test.go new file mode 100644 index 000000000..d5f333db4 --- /dev/null +++ b/pkg/webhook/core.oam.dev/v1beta1/application/validation_conflicts_test.go @@ -0,0 +1,220 @@ +/* +Copyright 2026 The KubeVela 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 application + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/oam" +) + +var _ = Describe("Trait conflict validation", func() { + Describe("traitConflictRuleMatches", func() { + cueTrait := &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "scaler", + Labels: map[string]string{"team": "platform"}, + }, + } + crdTrait := &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "service"}, + Spec: v1beta1.TraitDefinitionSpec{ + Reference: common.DefinitionReference{Name: "services.k8s.io"}, + }, + } + nestedGroupTrait := &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "ingress"}, + Spec: v1beta1.TraitDefinitionSpec{ + Reference: common.DefinitionReference{Name: "ingresses.networking.k8s.io"}, + }, + } + + DescribeTable("rule matching", + func(rule string, target *v1beta1.TraitDefinition, want bool) { + Expect(traitConflictRuleMatches(rule, target)).To(Equal(want)) + }, + Entry("wildcard", "*", cueTrait, true), + Entry("definition name match", "scaler", cueTrait, true), + Entry("definition name miss", "ingress", cueTrait, false), + Entry("crd name match", "services.k8s.io", crdTrait, true), + Entry("crd name ignored for empty reference", "services.k8s.io", cueTrait, false), + Entry("group wildcard match", "*.k8s.io", crdTrait, true), + Entry("group wildcard does not match nested group suffix", "*.k8s.io", nestedGroupTrait, false), + Entry("group wildcard miss", "*.networking.k8s.io", crdTrait, false), + Entry("group wildcard ignored for empty reference", "*.k8s.io", cueTrait, false), + Entry("label selector match", "labelSelector:team=platform", cueTrait, true), + Entry("label selector miss", "labelSelector:team=edge", cueTrait, false), + Entry("invalid label selector", "labelSelector:@@@", cueTrait, false), + ) + }) + + Describe("ValidateTraitConflicts", func() { + var ( + scheme *runtime.Scheme + conflictA *v1beta1.TraitDefinition + conflictB *v1beta1.TraitDefinition + scaler *v1beta1.TraitDefinition + service *v1beta1.TraitDefinition + ingress *v1beta1.TraitDefinition + gateway *v1beta1.TraitDefinition + labelConflict *v1beta1.TraitDefinition + nsConflictA *v1beta1.TraitDefinition + baseObjects []runtime.Object + ) + + BeforeEach(func() { + scheme = runtime.NewScheme() + Expect(v1beta1.AddToScheme(scheme)).To(Succeed()) + + conflictA = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "conflict-a", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{ConflictsWith: []string{"conflict-b"}}, + } + conflictB = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "conflict-b", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{}, + } + scaler = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "scaler", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{}, + } + service = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "service", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{ + Reference: common.DefinitionReference{Name: "services.k8s.io"}, + }, + } + ingress = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress", + Namespace: oam.SystemDefinitionNamespace, + Labels: map[string]string{"feature": "expose"}, + }, + Spec: v1beta1.TraitDefinitionSpec{ + Reference: common.DefinitionReference{Name: "ingresses.networking.k8s.io"}, + ConflictsWith: []string{"*.networking.k8s.io"}, + }, + } + gateway = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{ + Reference: common.DefinitionReference{Name: "gateways.networking.k8s.io"}, + }, + } + labelConflict = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "label-conflict", Namespace: oam.SystemDefinitionNamespace}, + Spec: v1beta1.TraitDefinitionSpec{ConflictsWith: []string{"labelSelector:feature=expose"}}, + } + nsConflictA = &v1beta1.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "conflict-a", Namespace: "default"}, + Spec: v1beta1.TraitDefinitionSpec{ConflictsWith: []string{"scaler"}}, + } + + baseObjects = []runtime.Object{ + conflictA.DeepCopy(), conflictB.DeepCopy(), scaler.DeepCopy(), + service.DeepCopy(), ingress.DeepCopy(), gateway.DeepCopy(), labelConflict.DeepCopy(), + } + }) + + validateWith := func(objects []runtime.Object, traits []common.ApplicationTrait) int { + handler := &ValidatingHandler{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objects...).Build(), + } + app := &v1beta1.Application{ + ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"}, + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{{ + Name: "web", + Type: "webservice", + Traits: traits, + }}, + }, + } + return len(handler.ValidateTraitConflicts(context.Background(), app)) + } + + It("rejects unidirectional definition-name conflict", func() { + Expect(validateWith(baseObjects, []common.ApplicationTrait{ + {Type: "conflict-a"}, + {Type: "conflict-b"}, + })).To(Equal(1)) + }) + + It("allows non-conflicting traits", func() { + Expect(validateWith(baseObjects, []common.ApplicationTrait{ + {Type: "conflict-a"}, + {Type: "scaler"}, + })).To(Equal(0)) + }) + + It("allows a single trait", func() { + Expect(validateWith(baseObjects, []common.ApplicationTrait{ + {Type: "conflict-a"}, + })).To(Equal(0)) + }) + + It("rejects crd name conflict", func() { + objects := make([]runtime.Object, 0, len(baseObjects)) + for _, o := range baseObjects { + if td, ok := o.(*v1beta1.TraitDefinition); ok && td.Name == "conflict-a" { + td = td.DeepCopy() + td.Spec.ConflictsWith = []string{"services.k8s.io"} + objects = append(objects, td) + continue + } + objects = append(objects, o.DeepCopyObject()) + } + Expect(validateWith(objects, []common.ApplicationTrait{ + {Type: "conflict-a"}, + {Type: "service"}, + })).To(Equal(1)) + }) + + It("rejects group wildcard conflict", func() { + Expect(validateWith(baseObjects, []common.ApplicationTrait{ + {Type: "ingress"}, + {Type: "gateway"}, + })).To(Equal(1)) + }) + + It("rejects labelSelector conflict", func() { + Expect(validateWith(baseObjects, []common.ApplicationTrait{ + {Type: "label-conflict"}, + {Type: "ingress"}, + })).To(Equal(1)) + }) + + It("prefers namespaced TraitDefinition over system definition", func() { + objects := append([]runtime.Object{}, baseObjects...) + objects = append(objects, nsConflictA.DeepCopy()) + // System conflict-a only conflicts with conflict-b. The namespaced + // override conflicts with scaler and must win the lookup. + Expect(validateWith(objects, []common.ApplicationTrait{ + {Type: "conflict-a"}, + {Type: "scaler"}, + })).To(Equal(1)) + }) + }) +})