fix(traits): use Comparable matches

Reverting apache#4512 which introduced a function diverging the match from the original design

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.

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

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(),
@@ -234,10 +229,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(),
@@ -316,9 +307,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)
@@ -387,9 +375,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)

pkg/trait/builder.go

@@ -20,6 +20,7 @@ package trait
 import (
 	"fmt"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -56,9 +57,18 @@ 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) {
+		return false
+	}
+	// More sofisticated check if len is the same. Sort and compare via slices equal func.
+	slices.Sort(t.Properties)
+	slices.Sort(otherTrait.Properties)
+	return slices.Equal(t.Properties, otherTrait.Properties)
 }
 
 func (t *builderTrait) Configure(e *Environment) (bool, *TraitCondition, error) {

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,60 @@ 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 TestBuilderTraitDoesntInfluencesBuild(t *testing.T) {
+	opt1 := Options{
+		"builder": map[string]interface{}{
+			"orderStrategy": "dependencies",
+		},
+	}
+	opt2 := Options{
+		"builder": map[string]interface{}{
+			"builder": map[string]interface{}{
+				"orderStrategy": "dependencies",
+			},
+		},
+	}
+	opt3 := Options{
+		"builder": map[string]interface{}{
+			"builder": map[string]interface{}{
+				"orderStrategy": "fifo",
+			},
+		},
+	}
+	b1, err := HasMatchingTraits(opt1, opt2)
+	assert.Nil(t, err)
+	assert.True(t, b1, "Traits should match")
+	b2, err := HasMatchingTraits(opt1, opt3)
+	assert.Nil(t, err)
+	assert.True(t, b2, "Traits should match")
+}

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) {

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))
+}

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) {

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))
+}