[camel-k] branch main updated: chore: Improve Camel dependency validation

[pc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

pcongiusti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git


The following commit(s) were added to refs/heads/main by this push:
     new c744ec1ff chore: Improve Camel dependency validation
c744ec1ff is described below

commit c744ec1ff3af83ae89970f6d12488da7eaa05a98
Author: Christoph Deppisch <cd...@redhat.com>
AuthorDate: Wed May 24 11:39:16 2023 +0200

    chore: Improve Camel dependency validation
    
    - Reuse logic in validate function just logging a warning and those validate function raising errors
    - Remove unused function ValidateDependencies
    - Add some unit tests
    - Add some documentation on Camel dependency resolution via catalog
---
 .../ROOT/pages/configuration/dependencies.adoc     |   7 ++
 pkg/cmd/run_support.go                             |   2 +-
 pkg/util/camel/camel_dependencies.go               |  63 +++++-----
 pkg/util/camel/camel_dependencies_test.go          | 128 +++++++++++++++++++++
 4 files changed, 169 insertions(+), 31 deletions(-)

diff --git a/docs/modules/ROOT/pages/configuration/dependencies.adoc b/docs/modules/ROOT/pages/configuration/dependencies.adoc
index 396220347..62d319437 100644
--- a/docs/modules/ROOT/pages/configuration/dependencies.adoc
+++ b/docs/modules/ROOT/pages/configuration/dependencies.adoc
@@ -39,6 +39,13 @@ kamel run -d camel:http Integration.java
 ```
 In this case, the dependency will be added with the correct version. Note that the standard notation for specifying a Camel dependency is `camel:xxx`, while `kamel` also accepts `camel-xxx` for usability.
 
+While resolving Camel dependencies (`camel:xxx` or `camel-xxx`) the Camel K operator tries to find the dependency in the xref:architecture/cr/camel-catalog.adoc[Camel catalog].
+In case the dependency is not listed in the catalog for some reason you will be provided with am error.
+Please make sure to use Camel dependencies listed in the catalog as these components are eligible to being used in Camel K (e.g. due to proper version resolving and runtime optimization).
+Using Camel dependencies not listed in the catalog may lead to unexpected behavior and is not supported.
+In case you do have a custom Camel component that you want to use as part of an Integration you can add this as an external Maven dependency using the respective Maven coordinates of your project.
+Please do not use one of the reserved the Camel groupIds (`org.apache.camel`) in that case.
+
 *External dependencies* can be added using the `-d` flag, the `mvn` prefix, and the maven coordinates:
 ```
 kamel run -d mvn:com.google.guava:guava:26.0-jre Integration.java
diff --git a/pkg/cmd/run_support.go b/pkg/cmd/run_support.go
index dd1e21a3d..048834243 100644
--- a/pkg/cmd/run_support.go
+++ b/pkg/cmd/run_support.go
@@ -42,7 +42,7 @@ import (
 
 func addDependency(cmd *cobra.Command, it *v1.Integration, dependency string, catalog *camel.RuntimeCatalog) {
 	normalized := camel.NormalizeDependency(dependency)
-	camel.ValidateDependency(catalog, normalized, cmd)
+	camel.ValidateDependency(catalog, normalized, cmd.ErrOrStderr())
 	it.Spec.AddDependency(normalized)
 }
 
diff --git a/pkg/util/camel/camel_dependencies.go b/pkg/util/camel/camel_dependencies.go
index e0e48a30f..684b4403c 100644
--- a/pkg/util/camel/camel_dependencies.go
+++ b/pkg/util/camel/camel_dependencies.go
@@ -49,53 +49,56 @@ func NormalizeDependency(dependency string) string {
 	return newDep
 }
 
-type Output interface {
-	OutOrStdout() io.Writer
-	ErrOrStderr() io.Writer
-}
-
-// ValidateDependencies validates dependencies against Camel catalog.
-// It only shows warning and does not throw error in case the Catalog is just not complete
+// ValidateDependency validates a dependency against Camel catalog.
+// It only shows warning and does not throw error in case the Catalog is just not complete,
 // and we don't want to let it stop the process.
-func ValidateDependencies(catalog *RuntimeCatalog, dependencies []string, out Output) {
-	for _, d := range dependencies {
-		ValidateDependency(catalog, d, out)
+func ValidateDependency(catalog *RuntimeCatalog, dependency string, out io.Writer) {
+	if err := ValidateDependencyE(catalog, dependency); err != nil {
+		fmt.Fprintf(out, "Warning: %s\n", err.Error())
 	}
-}
 
-// ValidateDependency validates a dependency against Camel catalog.
-// It only shows warning and does not throw error in case the Catalog is just not complete
-// and we don't want to let it stop the process.
-func ValidateDependency(catalog *RuntimeCatalog, dependency string, out Output) {
 	switch {
-	case strings.HasPrefix(dependency, "camel:"):
-		artifact := strings.TrimPrefix(dependency, "camel:")
-		if ok := catalog.IsValidArtifact(artifact); !ok {
-			fmt.Fprintf(out.ErrOrStderr(), "Warning: dependency %s not found in Camel catalog\n", dependency)
-		}
 	case strings.HasPrefix(dependency, "mvn:org.apache.camel:"):
 		component := strings.Split(dependency, ":")[2]
-		fmt.Fprintf(out.ErrOrStderr(), "Warning: do not use %s. Use %s instead\n",
+		fmt.Fprintf(out, "Warning: do not use %s. Use %s instead\n",
 			dependency, NormalizeDependency(component))
 	case strings.HasPrefix(dependency, "mvn:org.apache.camel.quarkus:"):
 		component := strings.Split(dependency, ":")[2]
-		fmt.Fprintf(out.ErrOrStderr(), "Warning: do not use %s. Use %s instead\n",
+		fmt.Fprintf(out, "Warning: do not use %s. Use %s instead\n",
 			dependency, NormalizeDependency(component))
 	}
+}
 
+// ValidateDependencyE validates a dependency against Camel catalog and throws error
+// in case it does not exist in the catalog.
+func ValidateDependencyE(catalog *RuntimeCatalog, dependency string) error {
+	var artifact string
+	switch {
+	case strings.HasPrefix(dependency, "camel:"):
+		artifact = strings.TrimPrefix(dependency, "camel:")
+	case strings.HasPrefix(dependency, "camel-quarkus:"):
+		artifact = strings.TrimPrefix(dependency, "camel-quarkus:")
+	case strings.HasPrefix(dependency, "camel-"):
+		artifact = dependency
+	}
+
+	if artifact == "" {
+		return nil
+	}
+
+	if ok := catalog.IsValidArtifact(artifact); !ok {
+		return fmt.Errorf("dependency %s not found in Camel catalog", dependency)
+	}
+
+	return nil
 }
 
 // ValidateDependenciesE validates dependencies against Camel catalog and throws error
-// if it doesn't exist in the catalog.
+// in case it does not exist in the catalog.
 func ValidateDependenciesE(catalog *RuntimeCatalog, dependencies []string) error {
 	for _, dependency := range dependencies {
-		if !strings.HasPrefix(dependency, "camel:") {
-			continue
-		}
-
-		artifact := strings.TrimPrefix(dependency, "camel:")
-		if ok := catalog.IsValidArtifact(artifact); !ok {
-			return fmt.Errorf("dependency %s not found in Camel catalog", dependency)
+		if err := ValidateDependencyE(catalog, dependency); err != nil {
+			return err
 		}
 	}
 
diff --git a/pkg/util/camel/camel_dependencies_test.go b/pkg/util/camel/camel_dependencies_test.go
index 8bb78f3da..2e817546e 100644
--- a/pkg/util/camel/camel_dependencies_test.go
+++ b/pkg/util/camel/camel_dependencies_test.go
@@ -18,8 +18,11 @@ limitations under the License.
 package camel
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
+	"github.com/apache/camel-k/v2/pkg/util/maven"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -30,4 +33,129 @@ func TestNormalizeDependency(t *testing.T) {
 	assert.Equal(t, "camel:file", NormalizeDependency("camel-quarkus:file"))
 	assert.Equal(t, "camel-k:knative", NormalizeDependency("camel-k-knative"))
 	assert.Equal(t, "camel-k:knative", NormalizeDependency("camel-k:knative"))
+	assert.Equal(t, "mvn:org.apache.camel:camel-file", NormalizeDependency("mvn:org.apache.camel:camel-file"))
+	assert.Equal(t, "mvn:org.apache.camel.quarkus:camel-quarkus-file", NormalizeDependency("mvn:org.apache.camel.quarkus:camel-quarkus-file"))
+	assert.Equal(t, "mvn:org.apache.camel:camel-k-knative", NormalizeDependency("mvn:org.apache.camel:camel-k-knative"))
+}
+
+func TestValidateDependency(t *testing.T) {
+	catalog, err := DefaultCatalog()
+	assert.Nil(t, err)
+
+	output := strings.Builder{}
+	ValidateDependency(catalog, "", &output)
+	assert.Equal(t, "", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "camel:file", &output)
+	assert.Equal(t, "", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "camel-quarkus-file", &output)
+	assert.Equal(t, "", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "camel-quarkus:file", &output)
+	assert.Equal(t, "", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "camel:unknown", &output)
+	assert.Equal(t, "Warning: dependency camel:unknown not found in Camel catalog\n", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "mvn:org.apache.camel:camel-foo", &output)
+	assert.Equal(t, "Warning: do not use mvn:org.apache.camel:camel-foo. Use camel:foo instead\n", output.String())
+
+	output.Reset()
+	ValidateDependency(catalog, "mvn:org.apache.camel.quarkus:camel-quarkus-foo", &output)
+	assert.Equal(t, "Warning: do not use mvn:org.apache.camel.quarkus:camel-quarkus-foo. Use camel:foo instead\n", output.String())
+}
+
+func TestManageIntegrationDependencies(t *testing.T) {
+	catalog, err := DefaultCatalog()
+	assert.Nil(t, err)
+
+	tests := []struct {
+		name         string
+		dependencies []string
+		coordinates  string
+	}{
+		{
+			name: "basic_camel",
+			dependencies: []string{
+				"camel:direct",
+				"camel:log",
+				"camel:core",
+			},
+			coordinates: "org.apache.camel.quarkus:camel-quarkus-direct," +
+				"org.apache.camel.quarkus:camel-quarkus-log," +
+				"org.apache.camel.quarkus:camel-quarkus-core",
+		},
+		{
+			name: "camel_quarkus",
+			dependencies: []string{
+				"camel:direct",
+				"camel-quarkus:log",
+				"camel:camel-quarkus-core",
+			},
+			coordinates: "org.apache.camel.quarkus:camel-quarkus-direct," +
+				"org.apache.camel.quarkus:camel-quarkus-log," +
+				"org.apache.camel.quarkus:camel-quarkus-core",
+		},
+		{
+			name: "camel_k",
+			dependencies: []string{
+				"camel:direct",
+				"camel-k:webhook",
+			},
+			coordinates: "org.apache.camel.quarkus:camel-quarkus-direct," +
+				"org.apache.camel.k:camel-k-webhook",
+		},
+		{
+			name: "not_in_catalog",
+			dependencies: []string{
+				"camel:direct",
+				"camel:resiliance4j",
+			},
+			coordinates: "org.apache.camel.quarkus:camel-quarkus-direct," +
+				"org.apache.camel.quarkus:camel-quarkus-resiliance4j",
+		},
+		{
+			name: "mvn",
+			dependencies: []string{
+				"mvn:org.foo:bar",
+				"mvn:org.apache.camel:camel-resiliance4j",
+			},
+			coordinates: "org.foo:bar," +
+				"org.apache.camel:camel-resiliance4j",
+		},
+		{
+			name: "jitpack",
+			dependencies: []string{
+				"github:apache/camel-sample/1.0",
+			},
+			coordinates: "com.github.apache:camel-sample",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			project := maven.Project{}
+
+			err = ManageIntegrationDependencies(&project, test.dependencies, catalog)
+			assert.Nil(t, err)
+
+			coordinates := strings.Builder{}
+			for i, d := range project.Dependencies {
+				if i == 0 {
+					_, err = fmt.Fprintf(&coordinates, "%s:%s", d.GroupID, d.ArtifactID)
+					assert.Nil(t, err)
+				} else {
+					_, err = fmt.Fprintf(&coordinates, ",%s:%s", d.GroupID, d.ArtifactID)
+					assert.Nil(t, err)
+				}
+			}
+			assert.Equal(t, test.coordinates, coordinates.String(), coordinates)
+		})
+	}
 }