From 0725b40ad7cc45d1a03f749505af2eafe3416059 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Thu, 7 Mar 2024 17:34:16 +0100 Subject: [PATCH] fix(traits): use Comparable matches Reverting #4512 which introduced a function diverging the match from the original design --- .../ROOT/pages/architecture/traits.adoc | 11 ++- e2e/common/misc/pipe_test.go | 6 +- e2e/common/traits/builder_test.go | 16 ++-- pkg/controller/integration/kits.go | 67 +------------- pkg/controller/integration/kits_test.go | 15 ---- pkg/controller/integration/monitor.go | 2 +- pkg/trait/builder.go | 31 ++++++- pkg/trait/builder_test.go | 51 +++++++++++ pkg/trait/camel.go | 10 ++- pkg/trait/camel_test.go | 22 +++++ pkg/trait/quarkus.go | 20 ++--- pkg/trait/quarkus_test.go | 28 ++++++ pkg/trait/registry.go | 5 -- pkg/trait/trait_types.go | 13 +-- pkg/trait/util.go | 89 ++++++++++++++---- pkg/trait/util_test.go | 90 +++++++++++++++++++ 16 files changed, 332 insertions(+), 144 deletions(-) diff --git a/docs/modules/ROOT/pages/architecture/traits.adoc b/docs/modules/ROOT/pages/architecture/traits.adoc index fc56b11928..dee146ccb3 100644 --- a/docs/modules/ROOT/pages/architecture/traits.adoc +++ b/docs/modules/ROOT/pages/architecture/traits.adoc @@ -49,6 +49,15 @@ type Trait interface { IsAllowedInProfile(v1.TraitProfile) bool Order() int } + +type Comparable interface { + Matches(trait Trait) bool +} + +type ComparableTrait interface { + Trait + Comparable +} ---- Each trait will implement this interface. The most important methods that will be invoked by the xref:architecture/operator.adoc[Operator] are `Configure()` and `Apply()`. Basically, the `Configure()` method will set those inputs aforementioned (each trait has its own). The method is in charge to verify also the correctness of those expected parameters, where it makes sense (i.e., a well expected `Kubernetes` resource name). The function can return a `TraitCondition` object containing any informative or warning condition to be attached to the resulting Integration (for instance, traits forcefully altered by the platform, or deprecation notices). @@ -59,6 +68,6 @@ The `Order()` method helps in resolving the order of execution of different trai The `InfluencesKit()`, `IsPlatformTrait()` and `RequiresIntegrationPlatform()` methods are easy to understand. They are used to determine if a trait has to influence an `IntegrationKit` build/initialization, if it's a platform trait (ie, needed by the platform itself) or are requiring the presence of an `IntegrationPlatform`. -The presence of `InfluencesBuild()` will let specify the level of granularity of a trait down to its properties for a rebuild. So, if you need, you can compare the traits properties coming from the `prev` (previous) Integration to decide if it is worth to rebuild an Integration or the trait can reuse the one already provided in `this` version. +For those traits that `InfluencesKit()` you may need to provide a `Matches(trait Trait)` func in order to specify those trait parameters that influences a build. This is required by the platform to decide if it is worth to rebuild an Integration or the trait can reuse the one already provided. Finally, through the `IsAllowedInProfile()` method we can override the default behavior (allow the trait for any profile). We must specify the profile we expect for this trait to be executed properly. diff --git a/e2e/common/misc/pipe_test.go b/e2e/common/misc/pipe_test.go index ef2f609362..e5e4db5298 100644 --- a/e2e/common/misc/pipe_test.go +++ b/e2e/common/misc/pipe_test.go @@ -63,7 +63,7 @@ func TestPipe(t *testing.T) { "--name", "throw-error-binding", ).Execute()).To(Succeed()) - g.Eventually(IntegrationPodPhase(t, ns, "throw-error-binding"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) + g.Eventually(IntegrationPodPhase(t, ns, "throw-error-binding"), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationLogs(t, ns, "throw-error-binding"), TestTimeoutShort).Should(ContainSubstring("[kameletErrorHandler] (Camel (camel-1) thread #1 - timer://tick)")) g.Eventually(IntegrationLogs(t, ns, "throw-error-binding"), TestTimeoutShort).ShouldNot(ContainSubstring("[integrationLogger] (Camel (camel-1) thread #1 - timer://tick)")) @@ -82,7 +82,7 @@ func TestPipe(t *testing.T) { "--name", "no-error-binding", ).Execute()).To(Succeed()) - g.Eventually(IntegrationPodPhase(t, ns, "no-error-binding"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) + g.Eventually(IntegrationPodPhase(t, ns, "no-error-binding"), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationLogs(t, ns, "no-error-binding"), TestTimeoutShort).ShouldNot(ContainSubstring("[kameletErrorHandler] (Camel (camel-1) thread #1 - timer://tick)")) g.Eventually(IntegrationLogs(t, ns, "no-error-binding"), TestTimeoutShort).Should(ContainSubstring("[integrationLogger] (Camel (camel-1) thread #1 - timer://tick)")) @@ -103,7 +103,7 @@ func TestPipe(t *testing.T) { "--name", "kb-with-traits", ).Execute()).To(Succeed()) - g.Eventually(IntegrationPodPhase(t, ns, "kb-with-traits"), TestTimeoutLong).Should(Equal(corev1.PodRunning)) + g.Eventually(IntegrationPodPhase(t, ns, "kb-with-traits"), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationLogs(t, ns, "kb-with-traits"), TestTimeoutShort).Should(ContainSubstring("hello from test")) g.Eventually(IntegrationLogs(t, ns, "kb-with-traits"), TestTimeoutShort).Should(ContainSubstring("integrationLogger")) }) diff --git a/e2e/common/traits/builder_test.go b/e2e/common/traits/builder_test.go index 42dc5b1b7b..0470b4bb0d 100644 --- a/e2e/common/traits/builder_test.go +++ b/e2e/common/traits/builder_test.go @@ -30,7 +30,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" . "github.com/apache/camel-k/v2/e2e/support" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" @@ -78,6 +77,8 @@ func TestBuilderTrait(t *testing.T) { name := RandomizedSuffixName("java-dependencies-strategy") g.Expect(KamelRunWithID(t, operatorID, ns, "files/Java.java", "--name", name, + // This is required in order to avoid reusing a Kit already existing (which is the default behavior) + "--build-property", "strategy=dependencies", "-t", "builder.order-strategy=dependencies").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -94,17 +95,8 @@ func TestBuilderTrait(t *testing.T) { g.Eventually(BuildConfig(t, integrationKitNamespace, integrationKitName)().LimitCPU, TestTimeoutShort).Should(Equal("")) g.Eventually(BuildConfig(t, integrationKitNamespace, integrationKitName)().RequestMemory, TestTimeoutShort).Should(Equal("")) g.Eventually(BuildConfig(t, integrationKitNamespace, integrationKitName)().LimitMemory, TestTimeoutShort).Should(Equal("")) - g.Eventually(BuilderPod(t, integrationKitNamespace, builderKitName), TestTimeoutShort).Should(BeNil()) - // check integration schema does not contains unwanted default trait value. - g.Eventually(UnstructuredIntegration(t, ns, name)).ShouldNot(BeNil()) - unstructuredIntegration := UnstructuredIntegration(t, ns, name)() - builderTrait, _, _ := unstructured.NestedMap(unstructuredIntegration.Object, "spec", "traits", "builder") - g.Expect(builderTrait).NotTo(BeNil()) - g.Expect(len(builderTrait)).To(Equal(1)) - g.Expect(builderTrait["orderStrategy"]).To(Equal("dependencies")) - g.Expect(Kamel(t, "delete", "--all", "-n", ns).Execute()).To(Succeed()) }) @@ -112,6 +104,8 @@ func TestBuilderTrait(t *testing.T) { name := RandomizedSuffixName("java-fifo-strategy") g.Expect(KamelRunWithID(t, operatorID, ns, "files/Java.java", "--name", name, + // This is required in order to avoid reusing a Kit already existing (which is the default behavior) + "--build-property", "strategy=fifo", "-t", "builder.order-strategy=fifo").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) @@ -138,6 +132,8 @@ func TestBuilderTrait(t *testing.T) { name := RandomizedSuffixName("java-resource-config") g.Expect(KamelRunWithID(t, operatorID, ns, "files/Java.java", "--name", name, + // This is required in order to avoid reusing a Kit already existing (which is the default behavior) + "--build-property", "resources=new-build", "-t", "builder.tasks-request-cpu=builder:500m", "-t", "builder.tasks-limit-cpu=builder:1000m", "-t", "builder.tasks-request-memory=builder:2Gi", diff --git a/pkg/controller/integration/kits.go b/pkg/controller/integration/kits.go index 20df5c682e..22adf0f039 100644 --- a/pkg/controller/integration/kits.go +++ b/pkg/controller/integration/kits.go @@ -19,8 +19,6 @@ package integration import ( "context" - "fmt" - "reflect" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" @@ -132,7 +130,7 @@ func integrationMatches(ctx context.Context, c client.Client, integration *v1.In return false, err } - if match, err := hasMatchingTraits(itc, ikc); !match || err != nil { + if match, err := trait.HasMatchingTraits(itc, ikc); !match || err != nil { ilog.Debug("Integration and integration-kit traits do not match", "integration", integration.Name, "integration-kit", kit.Name, "namespace", integration.Namespace) return false, err } @@ -195,7 +193,7 @@ func kitMatches(kit *v1.IntegrationKit, target *v1.IntegrationKit) (bool, error) return false, err } - if match, err := hasMatchingTraits(c1, c2); !match || err != nil { + if match, err := trait.HasMatchingTraits(c1, c2); !match || err != nil { return false, err } if !util.StringSliceContains(kit.Spec.Dependencies, target.Spec.Dependencies) { @@ -205,67 +203,6 @@ func kitMatches(kit *v1.IntegrationKit, target *v1.IntegrationKit) (bool, error) return true, nil } -func hasMatchingTraits(traitMap trait.Options, kitTraitMap trait.Options) (bool, error) { - catalog := trait.NewCatalog(nil) - - for _, t := range catalog.AllTraits() { - if t == nil { - continue - } - - id := string(t.ID()) - it, ok1 := traitMap.Get(id) - kt, ok2 := kitTraitMap.Get(id) - - if !ok1 && !ok2 { - continue - } - - if t.InfluencesKit() && t.InfluencesBuild(it, kt) { - if ct, ok := t.(trait.ComparableTrait); ok { - // if it's match trait use its matches method to determine the match - if match, err := matchesComparableTrait(ct, it, kt); !match || err != nil { - return false, err - } - } else { - if !matchesTrait(it, kt) { - return false, nil - } - } - } - } - - return true, nil -} - -func matchesComparableTrait(ct trait.ComparableTrait, it map[string]interface{}, kt map[string]interface{}) (bool, error) { - t1 := reflect.New(reflect.TypeOf(ct).Elem()).Interface() - if err := trait.ToTrait(it, &t1); err != nil { - return false, err - } - - t2 := reflect.New(reflect.TypeOf(ct).Elem()).Interface() - if err := trait.ToTrait(kt, &t2); err != nil { - return false, err - } - - ct2, ok := t2.(trait.ComparableTrait) - if !ok { - return false, fmt.Errorf("type assertion failed: %v", t2) - } - tt1, ok := t1.(trait.Trait) - if !ok { - return false, fmt.Errorf("type assertion failed: %v", t1) - } - - return ct2.Matches(tt1), nil -} - -func matchesTrait(it map[string]interface{}, kt map[string]interface{}) bool { - // perform exact match on the two trait maps - return reflect.DeepEqual(it, kt) -} - func hasMatchingSourcesForNative(it *v1.Integration, kit *v1.IntegrationKit) bool { if len(it.UserDefinedSources()) != len(kit.Spec.Sources) { return false diff --git a/pkg/controller/integration/kits_test.go b/pkg/controller/integration/kits_test.go index 685f5d0e01..1f946aa350 100644 --- a/pkg/controller/integration/kits_test.go +++ b/pkg/controller/integration/kits_test.go @@ -27,7 +27,6 @@ import ( traitv1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1/trait" "github.com/apache/camel-k/v2/pkg/trait" - "github.com/apache/camel-k/v2/pkg/util/log" "github.com/apache/camel-k/v2/pkg/util/test" "github.com/stretchr/testify/assert" @@ -94,10 +93,6 @@ func TestLookupKitForIntegration_DiscardKitsInError(t *testing.T) { require.NoError(t, err) - a := buildKitAction{} - a.InjectLogger(log.Log) - a.InjectClient(c) - kits, err := lookupKitsForIntegration(context.TODO(), c, &v1.Integration{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), @@ -221,10 +216,6 @@ func TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T) require.NoError(t, err) - a := buildKitAction{} - a.InjectLogger(log.Log) - a.InjectClient(c) - kits, err := lookupKitsForIntegration(context.TODO(), c, &v1.Integration{ TypeMeta: metav1.TypeMeta{ APIVersion: v1.SchemeGroupVersion.String(), @@ -288,9 +279,6 @@ func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t *testing.T) { }, } - a := buildKitAction{} - a.InjectLogger(log.Log) - ok, err := integrationAndKitHaveSameTraits(integration, kit) require.NoError(t, err) assert.False(t, ok) @@ -339,9 +327,6 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t *testing.T) { }, } - a := buildKitAction{} - a.InjectLogger(log.Log) - ok, err := integrationAndKitHaveSameTraits(integration, kit) require.NoError(t, err) assert.True(t, ok) diff --git a/pkg/controller/integration/monitor.go b/pkg/controller/integration/monitor.go index 4bd465bebd..2c526be948 100644 --- a/pkg/controller/integration/monitor.go +++ b/pkg/controller/integration/monitor.go @@ -237,7 +237,7 @@ func (action *monitorAction) checkDigestAndRebuild(ctx context.Context, integrat } if hash != integration.Status.Digest { - action.L.Info("Integration %s digest has changed: resetting its status. Will check if it needs to be rebuilt and restarted.", integration.Name) + action.L.Infof("Integration %s digest has changed: resetting its status. Will check if it needs to be rebuilt and restarted.", integration.Name) if isIntegrationKitResetRequired(integration, kit) { integration.SetIntegrationKit(nil) } diff --git a/pkg/trait/builder.go b/pkg/trait/builder.go index 6b93751262..8c6a8d790f 100644 --- a/pkg/trait/builder.go +++ b/pkg/trait/builder.go @@ -20,6 +20,7 @@ package trait import ( "fmt" "regexp" + "slices" "sort" "strconv" "strings" @@ -56,9 +57,33 @@ func (t *builderTrait) InfluencesKit() bool { return true } -// InfluencesBuild overrides base class method. -func (t *builderTrait) InfluencesBuild(this, prev map[string]interface{}) bool { - return true +func (t *builderTrait) Matches(trait Trait) bool { + otherTrait, ok := trait.(*builderTrait) + if !ok { + return false + } + if t.BaseImage != otherTrait.BaseImage || len(t.Properties) != len(otherTrait.Properties) || len(t.Tasks) != len(otherTrait.Tasks) { + return false + } + // More sofisticated check if len is the same. Sort and compare via slices equal func. + // Although the Matches func is used as a support for comparison, it makes sense + // to copy the properties and avoid possible inconsistencies caused by the sorting operation. + srtThisProps := make([]string, len(t.Properties)) + srtOtheProps := make([]string, len(otherTrait.Properties)) + copy(srtThisProps, t.Properties) + copy(srtOtheProps, otherTrait.Properties) + slices.Sort(srtThisProps) + slices.Sort(srtOtheProps) + if !slices.Equal(srtThisProps, srtOtheProps) { + return false + } + srtThisTasks := make([]string, len(t.Tasks)) + srtOtheTasks := make([]string, len(otherTrait.Tasks)) + copy(srtThisTasks, t.Tasks) + copy(srtOtheTasks, otherTrait.Tasks) + slices.Sort(srtThisTasks) + slices.Sort(srtOtheTasks) + return slices.Equal(srtThisTasks, srtOtheTasks) } func (t *builderTrait) Configure(e *Environment) (bool, *TraitCondition, error) { diff --git a/pkg/trait/builder_test.go b/pkg/trait/builder_test.go index 2448a28778..7b37567193 100644 --- a/pkg/trait/builder_test.go +++ b/pkg/trait/builder_test.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + traitv1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1/trait" "github.com/apache/camel-k/v2/pkg/util/camel" "github.com/apache/camel-k/v2/pkg/util/defaults" "github.com/apache/camel-k/v2/pkg/util/kubernetes" @@ -603,3 +604,53 @@ func tasksByName(tasks []v1.Task) []string { } return pipelineTasks } + +func TestBuilderMatches(t *testing.T) { + t1 := builderTrait{ + BasePlatformTrait: NewBasePlatformTrait("builder", 600), + BuilderTrait: traitv1.BuilderTrait{ + OrderStrategy: "dependencies", + }, + } + t2 := builderTrait{ + BasePlatformTrait: NewBasePlatformTrait("builder", 600), + BuilderTrait: traitv1.BuilderTrait{ + OrderStrategy: "dependencies", + }, + } + assert.True(t, t1.Matches(&t2)) + // This is a property that does not influence the build + t2.OrderStrategy = "fifo" + assert.True(t, t1.Matches(&t2)) + // Changing properties which influences build + t1.Properties = []string{"hello=world"} + assert.False(t, t1.Matches(&t2)) + t2.Properties = []string{"hello=world"} + assert.True(t, t1.Matches(&t2)) + t1.Properties = []string{"hello=world", "weare=theworld"} + assert.False(t, t1.Matches(&t2)) + // should detect swap + t2.Properties = []string{"weare=theworld", "hello=world"} + assert.True(t, t1.Matches(&t2)) +} + +func TestBuilderMatchesTasks(t *testing.T) { + t1 := builderTrait{ + BasePlatformTrait: NewBasePlatformTrait("builder", 600), + BuilderTrait: traitv1.BuilderTrait{}, + } + t2 := builderTrait{ + BasePlatformTrait: NewBasePlatformTrait("builder", 600), + BuilderTrait: traitv1.BuilderTrait{ + Tasks: []string{"task1;my-task;do-something"}, + }, + } + t3 := builderTrait{ + BasePlatformTrait: NewBasePlatformTrait("builder", 600), + BuilderTrait: traitv1.BuilderTrait{ + Tasks: []string{"task1;my-task;do-something-else"}, + }, + } + assert.False(t, t1.Matches(&t2)) + assert.False(t, t2.Matches(&t3)) +} diff --git a/pkg/trait/camel.go b/pkg/trait/camel.go index f56c2bd1f5..3d2a991d8d 100644 --- a/pkg/trait/camel.go +++ b/pkg/trait/camel.go @@ -53,9 +53,13 @@ func (t *camelTrait) InfluencesKit() bool { return true } -// InfluencesBuild only when the runtime has changed. -func (t *camelTrait) InfluencesBuild(this, prev map[string]interface{}) bool { - return this["runtimeVersion"] != prev["runtimeVersion"] +func (t *camelTrait) Matches(trait Trait) bool { + otherTrait, ok := trait.(*camelTrait) + if !ok { + return false + } + + return otherTrait.RuntimeVersion == t.RuntimeVersion } func (t *camelTrait) Configure(e *Environment) (bool, *TraitCondition, error) { diff --git a/pkg/trait/camel_test.go b/pkg/trait/camel_test.go index e60e7cf994..a3fc3688d6 100644 --- a/pkg/trait/camel_test.go +++ b/pkg/trait/camel_test.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + traitv1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1/trait" "github.com/apache/camel-k/v2/pkg/util/camel" "github.com/apache/camel-k/v2/pkg/util/kubernetes" "github.com/apache/camel-k/v2/pkg/util/test" @@ -208,3 +209,24 @@ func TestApplyCamelTraitWithSources(t *testing.T) { "content": "XML Source Code", }, sourceCm.Data) } + +func TestCamelMatches(t *testing.T) { + t1 := camelTrait{ + BasePlatformTrait: NewBasePlatformTrait("camel", 600), + CamelTrait: traitv1.CamelTrait{ + RuntimeVersion: "1.2.3", + }, + } + t2 := camelTrait{ + BasePlatformTrait: NewBasePlatformTrait("camel", 600), + CamelTrait: traitv1.CamelTrait{ + RuntimeVersion: "1.2.3", + }, + } + + assert.True(t, t1.Matches(&t2)) + t1.Properties = []string{"hello=world"} + assert.True(t, t1.Matches(&t2)) + t2.RuntimeVersion = "3.2.1" + assert.False(t, t1.Matches(&t2)) +} diff --git a/pkg/trait/quarkus.go b/pkg/trait/quarkus.go index bf7fdcc684..3803cb24fb 100644 --- a/pkg/trait/quarkus.go +++ b/pkg/trait/quarkus.go @@ -109,23 +109,14 @@ func (t *quarkusTrait) InfluencesKit() bool { return true } -// InfluencesBuild overrides base class method. -func (t *quarkusTrait) InfluencesBuild(this, prev map[string]interface{}) bool { - return true -} - -var _ ComparableTrait = &quarkusTrait{} - func (t *quarkusTrait) Matches(trait Trait) bool { qt, ok := trait.(*quarkusTrait) if !ok { return false } - if len(t.Modes) == 0 && len(qt.Modes) != 0 && !qt.containsMode(traitv1.JvmQuarkusMode) { return false } - for _, md := range t.Modes { if md == traitv1.JvmQuarkusMode && len(qt.Modes) == 0 { continue @@ -135,8 +126,17 @@ func (t *quarkusTrait) Matches(trait Trait) bool { } return false } + // We need to check if the native base image used is the same + thisNativeBaseImage := t.NativeBaseImage + if thisNativeBaseImage == "" { + thisNativeBaseImage = QuarkusNativeDefaultBaseImageName + } + otherNativeBaseImage := qt.NativeBaseImage + if otherNativeBaseImage == "" { + otherNativeBaseImage = QuarkusNativeDefaultBaseImageName + } - return true + return thisNativeBaseImage == otherNativeBaseImage } func (t *quarkusTrait) Configure(e *Environment) (bool, *TraitCondition, error) { diff --git a/pkg/trait/quarkus_test.go b/pkg/trait/quarkus_test.go index 752e57fc02..5d681a3097 100644 --- a/pkg/trait/quarkus_test.go +++ b/pkg/trait/quarkus_test.go @@ -225,3 +225,31 @@ func TestGetLanguageSettingsWithLoaders(t *testing.T) { assert.Equal(t, languageSettings{native: false, sourcesRequiredAtBuildTime: true}, getLanguageSettings(environment, v1.LanguageKotlin)) assert.Equal(t, languageSettings{native: true, sourcesRequiredAtBuildTime: false}, getLanguageSettings(environment, v1.LanguageJavaShell)) } + +func TestQuarkusMatches(t *testing.T) { + qt := quarkusTrait{ + BasePlatformTrait: NewBasePlatformTrait("quarkus", 600), + QuarkusTrait: traitv1.QuarkusTrait{ + Modes: []traitv1.QuarkusMode{traitv1.JvmQuarkusMode}, + }, + } + qt2 := quarkusTrait{ + BasePlatformTrait: NewBasePlatformTrait("quarkus", 600), + QuarkusTrait: traitv1.QuarkusTrait{ + Modes: []traitv1.QuarkusMode{traitv1.JvmQuarkusMode}, + NativeBaseImage: QuarkusNativeDefaultBaseImageName, + }, + } + + assert.True(t, qt.Matches(&qt2)) + qt2.Modes = append(qt2.Modes, traitv1.NativeQuarkusMode) + assert.True(t, qt.Matches(&qt2)) + qt2.Modes = []traitv1.QuarkusMode{traitv1.NativeQuarkusMode} + assert.False(t, qt.Matches(&qt2)) + qt2.Modes = nil + assert.True(t, qt.Matches(&qt2)) + qt2.Modes = []traitv1.QuarkusMode{} + assert.True(t, qt.Matches(&qt2)) + qt2.NativeBaseImage = "docker.io/my-new-native-base" + assert.False(t, qt.Matches(&qt2)) +} diff --git a/pkg/trait/registry.go b/pkg/trait/registry.go index 98d9f88e2e..906e6f7fa9 100644 --- a/pkg/trait/registry.go +++ b/pkg/trait/registry.go @@ -54,11 +54,6 @@ func (t *registryTrait) InfluencesKit() bool { return true } -// InfluencesBuild overrides base class method. -func (t *registryTrait) InfluencesBuild(this, prev map[string]interface{}) bool { - return true -} - func (t *registryTrait) Configure(e *Environment) (bool, *TraitCondition, error) { // disabled by default if e.IntegrationKit == nil || !pointer.BoolDeref(t.Enabled, false) { diff --git a/pkg/trait/trait_types.go b/pkg/trait/trait_types.go index 1d41da6320..85d152e47a 100644 --- a/pkg/trait/trait_types.go +++ b/pkg/trait/trait_types.go @@ -73,11 +73,6 @@ type Trait interface { // InfluencesKit determines if the trait has any influence on Integration Kits InfluencesKit() bool - // InfluencesBuild defines a low level of granularity for those traits which influences the build. - // The trait can specify if any particular trait configuration influences a build or not. - // Note: You must override this method if you override `InfluencesKit()` - InfluencesBuild(this, prev map[string]interface{}) bool - // IsPlatformTrait marks all fundamental traits that allow the platform to work IsPlatformTrait() bool @@ -91,10 +86,12 @@ type Trait interface { Order() int } +// Comparable is the interface exposing comparable funcs. type Comparable interface { Matches(trait Trait) bool } +// ComparableTrait is the interface used to compare two traits between them. type ComparableTrait interface { Trait Comparable @@ -154,12 +151,6 @@ func (trait *BaseTrait) InfluencesKit() bool { return false } -// InfluencesBuild defines a low level of granularity for those traits which influences the build. -// The trait can specify if any particular trait configuration influences a build or not. -func (trait *BaseTrait) InfluencesBuild(this, prev map[string]interface{}) bool { - return false -} - // IsPlatformTrait marks all fundamental traits that allow the platform to work. func (trait *BaseTrait) IsPlatformTrait() bool { return false diff --git a/pkg/trait/util.go b/pkg/trait/util.go index fe1190ea65..374d90e48b 100644 --- a/pkg/trait/util.go +++ b/pkg/trait/util.go @@ -533,7 +533,6 @@ func NewTraitsOptionsForKameletBinding(kb *v1alpha1.KameletBinding) (Options, er func FromAnnotations(meta *metav1.ObjectMeta) (Options, error) { options := make(Options) - for k, v := range meta.Annotations { if strings.HasPrefix(k, v1.TraitAnnotationPrefix) { configKey := strings.TrimPrefix(k, v1.TraitAnnotationPrefix) @@ -544,22 +543,7 @@ func FromAnnotations(meta *metav1.ObjectMeta) (Options, error) { if _, ok := options[id]; !ok { options[id] = make(map[string]interface{}) } - - propParts := util.ConfigTreePropertySplit(prop) - var current = options[id] - if len(propParts) > 1 { - c, err := util.NavigateConfigTree(current, propParts[0:len(propParts)-1]) - if err != nil { - return options, err - } - if cc, ok := c.(map[string]interface{}); ok { - current = cc - } else { - return options, errors.New(`invalid array specification: to set an array value use the ["v1", "v2"] format`) - } - } - current[prop] = v - + options[id][prop] = stringOrSlice(v) } else { return options, fmt.Errorf("wrong format for trait annotation %q: missing trait ID", k) } @@ -569,6 +553,24 @@ func FromAnnotations(meta *metav1.ObjectMeta) (Options, error) { return options, nil } +// stringOrSlice returns either a string or a slice with trimmed values when the input is +// represented as an array style (ie, [a,b,c]). +func stringOrSlice(val string) interface{} { + if val == "[]" { + // empty array + return []string{} + } + if strings.HasPrefix(val, "[") && strings.HasSuffix(val, "]") { + slice := strings.Split(val[1:len(val)-1], ",") + for i := range slice { + slice[i] = strings.Trim(slice[i], " ") + } + return slice + } else { + return val + } +} + // verify if the integration in the Environment contains an endpoint. func containsEndpoint(name string, e *Environment, c client.Client) (bool, error) { sources, err := kubernetes.ResolveIntegrationSources(e.Ctx, c, e.Integration, e.Resources) @@ -593,3 +595,56 @@ func containsEndpoint(name string, e *Environment, c client.Client) (bool, error } return hasKnativeEndpoint, nil } + +// HasMatchingTraits verifies if two traits options match. +func HasMatchingTraits(traitMap Options, kitTraitMap Options) (bool, error) { + catalog := 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 + } + id := string(t.ID()) + it, _ := traitMap.Get(id) + kt, _ := kitTraitMap.Get(id) + if ct, ok := t.(ComparableTrait); ok { + // if it's match trait use its matches method to determine the match + if match, err := matchesComparableTrait(ct, it, kt); !match || err != nil { + return false, err + } + } else { + if !matchesTrait(it, kt) { + return false, nil + } + } + } + + return true, nil +} + +func matchesComparableTrait(ct ComparableTrait, it map[string]interface{}, kt map[string]interface{}) (bool, error) { + t1 := reflect.New(reflect.TypeOf(ct).Elem()).Interface() + if err := ToTrait(it, &t1); err != nil { + return false, err + } + t2 := reflect.New(reflect.TypeOf(ct).Elem()).Interface() + if err := ToTrait(kt, &t2); err != nil { + return false, err + } + ct2, ok := t2.(ComparableTrait) + if !ok { + return false, fmt.Errorf("type assertion failed: %v", t2) + } + tt1, ok := t1.(Trait) + if !ok { + return false, fmt.Errorf("type assertion failed: %v", t1) + } + + return ct2.Matches(tt1), nil +} + +func matchesTrait(it map[string]interface{}, kt map[string]interface{}) bool { + // perform exact match on the two trait maps + return reflect.DeepEqual(it, kt) +} diff --git a/pkg/trait/util_test.go b/pkg/trait/util_test.go index 69767785b1..d89262bbf9 100644 --- a/pkg/trait/util_test.go +++ b/pkg/trait/util_test.go @@ -345,3 +345,93 @@ func TestSameTraits(t *testing.T) { assert.False(t, ok) }) } + +func TestHasMathchingTraitsEmpty(t *testing.T) { + opt1 := Options{ + "builder": {}, + "camel": { + "runtimeVersion": "1.2.3", + }, + "quarkus": {}, + } + opt2 := Options{ + "camel": { + "runtimeVersion": "1.2.3", + }, + } + opt3 := Options{ + "camel": { + "runtimeVersion": "1.2.3", + }, + } + opt4 := Options{ + "camel": { + "runtimeVersion": "3.2.1", + }, + } + b1, err := HasMatchingTraits(opt1, opt2) + assert.Nil(t, err) + assert.True(t, b1) + b2, err := HasMatchingTraits(opt1, opt4) + assert.Nil(t, err) + assert.False(t, b2) + b3, err := HasMatchingTraits(opt2, opt3) + assert.Nil(t, err) + assert.True(t, b3) +} + +func TestHasMathchingTraitsMissing(t *testing.T) { + opt1 := Options{} + opt2 := Options{ + "camel": { + "properties": []string{"a=1"}, + }, + } + b1, err := HasMatchingTraits(opt1, opt2) + assert.Nil(t, err) + assert.True(t, b1) +} + +func TestFromAnnotationsPlain(t *testing.T) { + meta := metav1.ObjectMeta{ + Annotations: map[string]string{ + "trait.camel.apache.org/trait.prop1": "hello1", + "trait.camel.apache.org/trait.prop2": "hello2", + }, + } + opt, err := FromAnnotations(&meta) + require.NoError(t, err) + tt, ok := opt.Get("trait") + assert.True(t, ok) + assert.Equal(t, "hello1", tt["prop1"]) + assert.Equal(t, "hello2", tt["prop2"]) +} + +func TestFromAnnotationsArray(t *testing.T) { + meta := metav1.ObjectMeta{ + Annotations: map[string]string{ + "trait.camel.apache.org/trait.prop1": "[hello,world]", + // The func should trim empty spaces as well + "trait.camel.apache.org/trait.prop2": "[\"hello=1\", \"world=2\"]", + }, + } + opt, err := FromAnnotations(&meta) + require.NoError(t, err) + tt, ok := opt.Get("trait") + assert.True(t, ok) + assert.Equal(t, []string{"hello", "world"}, tt["prop1"]) + assert.Equal(t, []string{"\"hello=1\"", "\"world=2\""}, tt["prop2"]) +} + +func TestFromAnnotationsArrayEmpty(t *testing.T) { + meta := metav1.ObjectMeta{ + Annotations: map[string]string{ + "trait.camel.apache.org/trait.prop": "[]", + }, + } + opt, err := FromAnnotations(&meta) + require.NoError(t, err) + tt, ok := opt.Get("trait") + assert.True(t, ok) + assert.Equal(t, []string{}, tt["prop"]) +}