This is an automated email from the ASF dual-hosted git repository. astefanutti pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit 6430399d143ff45dde5b584e90571f5fd2ee5a47 Author: Antonin Stefanutti <[email protected]> AuthorDate: Tue Aug 31 12:27:18 2021 +0200 feat(native): Lenient trait matching --- pkg/controller/integration/build_kit.go | 37 +------- pkg/controller/integration/build_kit_test.go | 14 +-- pkg/controller/integration/kits.go | 122 ++++++++++++++++----------- pkg/trait/quarkus.go | 24 ++++-- pkg/trait/trait_catalog.go | 10 +-- pkg/trait/trait_test.go | 6 +- 6 files changed, 104 insertions(+), 109 deletions(-) diff --git a/pkg/controller/integration/build_kit.go b/pkg/controller/integration/build_kit.go index a0fee46..aab2ba2 100644 --- a/pkg/controller/integration/build_kit.go +++ b/pkg/controller/integration/build_kit.go @@ -24,8 +24,6 @@ import ( v1 "github.com/apache/camel-k/pkg/apis/camel/v1" "github.com/apache/camel-k/pkg/trait" - "github.com/apache/camel-k/pkg/util" - "github.com/apache/camel-k/pkg/util/defaults" "github.com/apache/camel-k/pkg/util/kubernetes" ) @@ -97,7 +95,7 @@ kits: for _, kit := range env.IntegrationKits { kit := kit for i, k := range existingKits { - match, err := action.kitMatches(&kit, &k) + match, err := kitMatches(&kit, &k) if err != nil { return nil, err } @@ -128,36 +126,3 @@ kits: return integration, nil } - - -// kitMatches returns whether the v1.IntegrationKit match -func (action *buildKitAction) kitMatches(k1 *v1.IntegrationKit, k2 *v1.IntegrationKit) (bool, error) { - version := k1.Status.Version - if version == "" { - // Defaults with the version that is going to be set during the kit initialization - version = defaults.Version - } - if version != k2.Status.Version { - return false, nil - } - if len(k1.Spec.Dependencies) != len(k2.Spec.Dependencies) { - return false, nil - } - if len(k1.Spec.Traits) != len(k2.Spec.Traits) { - return false, nil - } - for name, kt1 := range k1.Spec.Traits { - kt2, ok := k2.Spec.Traits[name] - if !ok { - return false, nil - } - match, err := hasMatchingTrait(&kt1, &kt2) - if !match || err != nil { - return false, err - } - } - if !util.StringSliceContains(k1.Spec.Dependencies, k2.Spec.Dependencies) { - return false, nil - } - return true, nil -} diff --git a/pkg/controller/integration/build_kit_test.go b/pkg/controller/integration/build_kit_test.go index 430dc38..8c9f749 100644 --- a/pkg/controller/integration/build_kit_test.go +++ b/pkg/controller/integration/build_kit_test.go @@ -109,9 +109,7 @@ func TestLookupKitForIntegration_DiscardKitsInError(t *testing.T) { func TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T) { c, err := test.NewFakeClient( - // // Should be discarded because it does not contain the required traits - // &v1.IntegrationKit{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), @@ -134,10 +132,8 @@ func TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T) Phase: v1.IntegrationKitPhaseReady, }, }, - // // Should be discarded because it contains a subset of the required traits but // with different configuration value - // &v1.IntegrationKit{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), @@ -165,10 +161,8 @@ func TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T) Phase: v1.IntegrationKitPhaseReady, }, }, - // // Should NOT be discarded because it contains a subset of the required traits and // same configuration values - // &v1.IntegrationKit{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), @@ -259,7 +253,7 @@ func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t *testing.T) { }, } - integrationKitSpec := &v1.IntegrationKit{ + kit := &v1.IntegrationKit{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), Kind: v1.IntegrationKitKind, @@ -276,7 +270,7 @@ func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t *testing.T) { a := buildKitAction{} a.InjectLogger(log.Log) - ok, err := hasMatchingTraits(integration, integrationKitSpec) + ok, err := hasMatchingTraits(integration.Spec.Traits, kit.Spec.Traits) assert.Nil(t, err) assert.False(t, ok) } @@ -303,7 +297,7 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t *testing.T) { }, } - integrationKitSpec := &v1.IntegrationKit{ + kit := &v1.IntegrationKit{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), Kind: v1.IntegrationKitKind, @@ -327,7 +321,7 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t *testing.T) { a := buildKitAction{} a.InjectLogger(log.Log) - ok, err := hasMatchingTraits(integration, integrationKitSpec) + ok, err := hasMatchingTraits(integration.Spec.Traits, kit.Spec.Traits) assert.Nil(t, err) assert.True(t, ok) } diff --git a/pkg/controller/integration/kits.go b/pkg/controller/integration/kits.go index 2a7eab0..49927da 100644 --- a/pkg/controller/integration/kits.go +++ b/pkg/controller/integration/kits.go @@ -22,7 +22,7 @@ import ( "encoding/json" "reflect" - k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" @@ -32,11 +32,12 @@ import ( "github.com/apache/camel-k/pkg/platform" "github.com/apache/camel-k/pkg/trait" "github.com/apache/camel-k/pkg/util" + "github.com/apache/camel-k/pkg/util/defaults" ) func lookupKitsForIntegration(ctx context.Context, c ctrl.Reader, integration *v1.Integration, options ...ctrl.ListOption) ([]v1.IntegrationKit, error) { pl, err := platform.GetCurrent(ctx, c, integration.Namespace) - if err != nil && !k8serrors.IsNotFound(err) { + if err != nil && !errors.IsNotFound(err) { return nil, err } @@ -106,7 +107,7 @@ func integrationMatches(integration *v1.Integration, kit *v1.IntegrationKit) (bo // // A kit can be used only if it contains a subset of the traits and related configurations // declared on integration. - if match, err := hasMatchingTraits(integration, kit); !match || err != nil { + if match, err := hasMatchingTraits(integration.Spec.Traits, kit.Spec.Traits); !match || err != nil { return false, err } if !util.StringSliceContains(kit.Spec.Dependencies, integration.Status.Dependencies) { @@ -115,89 +116,110 @@ func integrationMatches(integration *v1.Integration, kit *v1.IntegrationKit) (bo return true, nil } -// hasMatchingTraits compares the traits defined on the v1.Integration with those defined on the v1.IntegrationKit -func hasMatchingTraits(integration *v1.Integration, kit *v1.IntegrationKit) (bool, error) { - catalog := trait.NewCatalog(nil) +// kitMatches returns whether the two v1.IntegrationKit match +func kitMatches(kit1 *v1.IntegrationKit, kit2 *v1.IntegrationKit) (bool, error) { + version := kit1.Status.Version + if version == "" { + // Defaults with the version that is going to be set during the kit initialization + version = defaults.Version + } + if version != kit2.Status.Version { + return false, nil + } + if len(kit1.Spec.Dependencies) != len(kit2.Spec.Dependencies) { + return false, nil + } + if match, err := hasMatchingTraits(kit1.Spec.Traits, kit2.Spec.Traits); !match || err != nil { + return false, err + } + if !util.StringSliceContains(kit1.Spec.Dependencies, kit2.Spec.Dependencies) { + return false, nil + } + return true, nil +} - traitCount := 0 - for name, itTrait := range integration.Spec.Traits { - t := catalog.GetTrait(name) +func hasMatchingTraits(traits1 map[string]v1.TraitSpec, traits2 map[string]v1.TraitSpec) (bool, error) { + catalog := trait.NewCatalog(nil) + for _, t := range catalog.AllTraits() { if t != nil && !t.InfluencesKit() { // We don't store the trait configuration if the trait cannot influence the kit behavior continue } - traitCount++ - kitTrait, ok := kit.Spec.Traits[name] - if !ok { - // skip it because trait configured on integration is not defined on kit - return false, nil + id := string(t.ID()) + t1, ok1 := traits1[id] + t2, ok2 := traits2[id] + + if !ok1 && !ok2 { + continue } if ct, ok := t.(trait.ComparableTrait); ok { - comparable, err := hasComparableTrait(ct, &itTrait, &kitTrait) - if !comparable || err != nil { + if comparable, err := hasComparableTrait(ct, &t1, &t2); err != nil { return false, err + } else if comparable { + continue } - } else { - match, err := hasMatchingTrait(&itTrait, &kitTrait) - if !match || err != nil { + } else if ok1 && ok2 { + if match, err := hasMatchingTrait(&t1, &t2); err != nil { return false, err + } else if match { + continue } } - } - - // Check the number of influencing traits matches - if len(kit.Spec.Traits) != traitCount { return false, nil } return true, nil } -func hasComparableTrait(c trait.ComparableTrait, itTrait *v1.TraitSpec, kitTrait *v1.TraitSpec) (bool, error) { - it := reflect.New(reflect.TypeOf(c).Elem()).Interface() - data, err := json.Marshal(itTrait.Configuration) - if err != nil { - return false, err - } - err = json.Unmarshal(data, &it) - if err != nil { - return false, err +func hasComparableTrait(t trait.ComparableTrait, ts1 *v1.TraitSpec, ts2 *v1.TraitSpec) (bool, error) { + t1 := reflect.New(reflect.TypeOf(t).Elem()).Interface() + if ts1.Configuration.RawMessage != nil { + data, err := json.Marshal(ts1.Configuration) + if err != nil { + return false, err + } + err = json.Unmarshal(data, &t1) + if err != nil { + return false, err + } } - kt := reflect.New(reflect.TypeOf(c).Elem()).Interface() - data, err = json.Marshal(kitTrait.Configuration) - if err != nil { - return false, err - } - err = json.Unmarshal(data, &kt) - if err != nil { - return false, err + t2 := reflect.New(reflect.TypeOf(t).Elem()).Interface() + if ts2.Configuration.RawMessage != nil { + data, err := json.Marshal(ts2.Configuration) + if err != nil { + return false, err + } + err = json.Unmarshal(data, &t2) + if err != nil { + return false, err + } } - return kt.(trait.ComparableTrait).Matches(it.(trait.Trait)), nil + return t2.(trait.ComparableTrait).Matches(t1.(trait.Trait)), nil } -func hasMatchingTrait(itTrait *v1.TraitSpec, kitTrait *v1.TraitSpec) (bool, error) { - data, err := json.Marshal(itTrait.Configuration) +func hasMatchingTrait(ts1 *v1.TraitSpec, ts2 *v1.TraitSpec) (bool, error) { + data, err := json.Marshal(ts1.Configuration) if err != nil { return false, err } - itConf := make(map[string]interface{}) - err = json.Unmarshal(data, &itConf) + t1 := make(map[string]interface{}) + err = json.Unmarshal(data, &t1) if err != nil { return false, err } - data, err = json.Marshal(kitTrait.Configuration) + data, err = json.Marshal(ts2.Configuration) if err != nil { return false, err } - kitConf := make(map[string]interface{}) - err = json.Unmarshal(data, &kitConf) + t2 := make(map[string]interface{}) + err = json.Unmarshal(data, &t2) if err != nil { return false, err } - for ck, cv := range kitConf { - iv, ok := itConf[ck] + for ck, cv := range t2 { + iv, ok := t1[ck] if !ok { // skip it because trait configured on kit has a value that is not defined // in integration trait diff --git a/pkg/trait/quarkus.go b/pkg/trait/quarkus.go index 138d3a1..629ba82 100644 --- a/pkg/trait/quarkus.go +++ b/pkg/trait/quarkus.go @@ -88,16 +88,30 @@ func (t *quarkusTrait) Matches(trait Trait) bool { return false } -types: - for _, p1 := range t.PackageTypes { - for _, p2 := range qt.PackageTypes { - if p1 == p2 { - continue types + contains := func(types []quarkusPackageType, t quarkusPackageType) bool { + for _, ti := range qt.PackageTypes { + if t == ti { + return true } } return false } + if len(t.PackageTypes) == 0 && len(qt.PackageTypes) != 0 && !contains(qt.PackageTypes, fastJarPackageType) { + return false + } + +types: + for _, pt := range t.PackageTypes { + if pt == fastJarPackageType && len(qt.PackageTypes) == 0 { + continue + } + if contains(qt.PackageTypes, pt) { + continue types + } + return false + } + return true } diff --git a/pkg/trait/trait_catalog.go b/pkg/trait/trait_catalog.go index f3c31c5..3f3d3ac 100644 --- a/pkg/trait/trait_catalog.go +++ b/pkg/trait/trait_catalog.go @@ -54,7 +54,7 @@ func NewCatalog(c client.Client) *Catalog { traits: traitList, } - for _, t := range catalog.allTraits() { + for _, t := range catalog.AllTraits() { if c != nil { t.InjectClient(c) } @@ -62,7 +62,7 @@ func NewCatalog(c client.Client) *Catalog { return &catalog } -func (c *Catalog) allTraits() []Trait { +func (c *Catalog) AllTraits() []Trait { return append([]Trait(nil), c.traits...) } @@ -79,7 +79,7 @@ func (c *Catalog) traitsFor(environment *Environment) []Trait { // so care must be taken while changing the lists order. func (c *Catalog) TraitsForProfile(profile v1.TraitProfile) []Trait { var res []Trait - for _, t := range c.allTraits() { + for _, t := range c.AllTraits() { if t.IsAllowedInProfile(profile) { res = append(res, t) } @@ -142,7 +142,7 @@ func (c *Catalog) apply(environment *Environment) error { // GetTrait returns the trait with the given ID func (c *Catalog) GetTrait(id string) Trait { - for _, t := range c.allTraits() { + for _, t := range c.AllTraits() { if t.ID() == ID(id) { return t } @@ -153,7 +153,7 @@ func (c *Catalog) GetTrait(id string) Trait { // ComputeTraitsProperties returns all key/value configuration properties that can be used to configure traits func (c *Catalog) ComputeTraitsProperties() []string { results := make([]string, 0) - for _, trait := range c.allTraits() { + for _, trait := range c.AllTraits() { trait := trait // pin c.processFields(structs.Fields(trait), func(name string) { results = append(results, string(trait.ID())+"."+name) diff --git a/pkg/trait/trait_test.go b/pkg/trait/trait_test.go index 6eb0261..e423830 100644 --- a/pkg/trait/trait_test.go +++ b/pkg/trait/trait_test.go @@ -497,7 +497,7 @@ func TestOnlySomeTraitsInfluenceBuild(t *testing.T) { c := NewTraitTestCatalog() buildTraits := []string{"builder", "quarkus"} - for _, trait := range c.allTraits() { + for _, trait := range c.AllTraits() { if trait.InfluencesKit() { assert.Contains(t, buildTraits, string(trait.ID())) } else { @@ -510,7 +510,7 @@ func TestOnlySomeTraitsArePlatform(t *testing.T) { c := NewTraitTestCatalog() platformTraits := []string{"builder", "camel", "jvm", "container", "dependencies", "deployer", "deployment", "environment", "error-handler", "kamelets", "openapi", "owner", "platform", "quarkus"} - for _, trait := range c.allTraits() { + for _, trait := range c.AllTraits() { if trait.IsPlatformTrait() { assert.Contains(t, platformTraits, string(trait.ID())) } else { @@ -523,7 +523,7 @@ func TestOnlySomeTraitsDoNotRequireIntegrationPlatform(t *testing.T) { c := NewTraitTestCatalog() doNotRequirePlatformTraits := []string{"deployer", "platform"} - for _, trait := range c.allTraits() { + for _, trait := range c.AllTraits() { if !trait.RequiresIntegrationPlatform() { assert.Contains(t, doNotRequirePlatformTraits, string(trait.ID())) } else {
