You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by pc...@apache.org on 2021/08/23 12:58:05 UTC

[camel-k] branch main updated (034ada1 -> ec908ea)

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

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


    from 034ada1  doc: contribute links
     new 6139abb  fix(cli): property priority
     new 80b8d97  feat(e2e): properties priority test
     new ec908ea  doc(cli): properties priority

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../pages/configuration/build-time-properties.adoc |  5 ++
 .../pages/configuration/runtime-properties.adoc    |  5 ++
 e2e/common/config/config_test.go                   | 16 ++++++
 pkg/cmd/run.go                                     | 57 ++++++++------------
 pkg/cmd/run_help.go                                | 36 +++++++++++++
 pkg/cmd/run_help_test.go                           | 63 ++++++++++++++++++++++
 pkg/cmd/run_test.go                                | 35 ------------
 7 files changed, 146 insertions(+), 71 deletions(-)

[camel-k] 01/03: fix(cli): property priority

Posted by 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

commit 6139abb0551c73626ca25c7faa7782714218a287
Author: Pasquale Congiusti <pa...@gmail.com>
AuthorDate: Thu Aug 12 15:38:41 2021 +0200

    fix(cli): property priority
    
    Introduced a merge function which is in charge to merge all properties, giving priority to single declaration property options
    
    Closes #2538
---
 pkg/cmd/run.go           | 57 ++++++++++++++++---------------------------
 pkg/cmd/run_help.go      | 36 +++++++++++++++++++++++++++
 pkg/cmd/run_help_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 pkg/cmd/run_test.go      | 35 ---------------------------
 4 files changed, 120 insertions(+), 71 deletions(-)

diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go
index 5095362..8cb1ebf 100644
--- a/pkg/cmd/run.go
+++ b/pkg/cmd/run.go
@@ -575,34 +575,33 @@ func (o *runCmdOptions) createOrUpdateIntegration(cmd *cobra.Command, c client.C
 		// Deprecated: making it compatible with newer mechanism
 		o.Properties = append(o.Properties, "file:"+item)
 	}
-	for _, item := range o.Properties {
-		props, err := extractProperties(item)
-		if err != nil {
-			return nil, err
-		}
-		if err := addIntegrationProperties(props, &integration.Spec); err != nil {
-			return nil, err
-		}
+
+	props, err := mergePropertiesWithPrecedence(o.Properties)
+	if err != nil {
+		return nil, err
+	}
+	if err := addIntegrationProperties(props, &integration.Spec); err != nil {
+		return nil, err
 	}
+
 	// convert each build configuration to a builder trait property
-	for _, item := range o.BuildProperties {
-		props, err := extractProperties(item)
-		if err != nil {
-			return nil, err
-		}
-		for _, k := range props.Keys() {
-			v, ok := props.Get(k)
-			if ok {
-				entry, err := property.EncodePropertyFileEntry(k, v)
-				if err != nil {
-					return nil, err
-				}
-				o.Traits = append(o.Traits, fmt.Sprintf("builder.properties=%s", entry))
-			} else {
+	buildProps, err := mergePropertiesWithPrecedence(o.BuildProperties)
+	if err != nil {
+		return nil, err
+	}
+	for _, k := range buildProps.Keys() {
+		v, ok := buildProps.Get(k)
+		if ok {
+			entry, err := property.EncodePropertyFileEntry(k, v)
+			if err != nil {
 				return nil, err
 			}
+			o.Traits = append(o.Traits, fmt.Sprintf("builder.properties=%s", entry))
+		} else {
+			return nil, err
 		}
 	}
+
 	for _, item := range o.Configs {
 		if config, parseErr := ParseConfigOption(item); parseErr == nil {
 			if applyConfigOptionErr := ApplyConfigOption(config, &integration.Spec, c, namespace, o.Compression); applyConfigOptionErr != nil {
@@ -687,20 +686,6 @@ func addResource(resourceLocation string, integrationSpec *v1.IntegrationSpec, e
 	return nil
 }
 
-// The function parse the value and if it is a file (file:/path/), it will parse as property file
-// otherwise return a single property built from the item passed as `key=value`
-func extractProperties(value string) (*properties.Properties, error) {
-	if !strings.HasPrefix(value, "file:") {
-		return keyValueProps(value)
-	}
-	// we already validated the existence of files during validate()
-	return loadPropertyFile(strings.Replace(value, "file:", "", 1))
-}
-
-func keyValueProps(value string) (*properties.Properties, error) {
-	return properties.Load([]byte(value), properties.UTF8)
-}
-
 func (o *runCmdOptions) GetIntegrationName(sources []string) string {
 	name := ""
 	if o.IntegrationName != "" {
diff --git a/pkg/cmd/run_help.go b/pkg/cmd/run_help.go
index 55ea888..0bd45fe 100644
--- a/pkg/cmd/run_help.go
+++ b/pkg/cmd/run_help.go
@@ -27,6 +27,7 @@ import (
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 	"github.com/apache/camel-k/pkg/client"
 	"github.com/apache/camel-k/pkg/util/kubernetes"
+	"github.com/magiconair/properties"
 )
 
 var invalidPaths = []string{"/etc/camel", "/deployments/dependencies"}
@@ -277,3 +278,38 @@ func filterFileLocation(maybeFileLocations []string) []string {
 	}
 	return filteredOptions
 }
+
+func mergePropertiesWithPrecedence(items []string) (*properties.Properties, error) {
+	loPrecedenceProps := properties.NewProperties()
+	hiPrecedenceProps := properties.NewProperties()
+	for _, item := range items {
+		prop, err := extractProperties(item)
+		if err != nil {
+			return nil, err
+		}
+		// We consider file props to have a lower priority versus single properties
+		if strings.HasPrefix(item, "file:") {
+			loPrecedenceProps.Merge(prop)
+		} else {
+			hiPrecedenceProps.Merge(prop)
+		}
+	}
+	// Any property contained in both collections will be merged
+	// giving precedence to the ones in hiPrecedenceProps
+	loPrecedenceProps.Merge(hiPrecedenceProps)
+	return loPrecedenceProps, nil
+}
+
+// The function parse the value and if it is a file (file:/path/), it will parse as property file
+// otherwise return a single property built from the item passed as `key=value`
+func extractProperties(value string) (*properties.Properties, error) {
+	if !strings.HasPrefix(value, "file:") {
+		return keyValueProps(value)
+	}
+	// we already validated the existence of files during validate()
+	return loadPropertyFile(strings.Replace(value, "file:", "", 1))
+}
+
+func keyValueProps(value string) (*properties.Properties, error) {
+	return properties.Load([]byte(value), properties.UTF8)
+}
diff --git a/pkg/cmd/run_help_test.go b/pkg/cmd/run_help_test.go
index a7d92bb..02f852d 100644
--- a/pkg/cmd/run_help_test.go
+++ b/pkg/cmd/run_help_test.go
@@ -18,6 +18,8 @@ limitations under the License.
 package cmd
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -173,3 +175,64 @@ func TestValidateFileLocation(t *testing.T) {
 	assert.NotNil(t, err)
 	assert.Equal(t, "you cannot mount a file under /deployments/dependencies path", err.Error())
 }
+
+func TestExtractProperties_SingleKeyValue(t *testing.T) {
+	correctValues := []string{"key=val", "key = val", "key= val", " key   =  val"}
+	for _, val := range correctValues {
+		prop, err := extractProperties(val)
+		assert.Nil(t, err)
+		value, ok := prop.Get("key")
+		assert.True(t, ok)
+		assert.Equal(t, "val", value)
+	}
+}
+
+func TestExtractProperties_FromFile(t *testing.T) {
+	var tmpFile1 *os.File
+	var err error
+	if tmpFile1, err = ioutil.TempFile("", "camel-k-*.properties"); err != nil {
+		t.Error(err)
+	}
+
+	assert.Nil(t, tmpFile1.Close())
+	assert.Nil(t, ioutil.WriteFile(tmpFile1.Name(), []byte(`
+	key=value
+	#key2=value2
+	my.key=value
+	`), 0644))
+
+	props, err := extractProperties("file:" + tmpFile1.Name())
+	assert.Nil(t, err)
+	assert.Equal(t, 2, props.Len())
+	for _, prop := range props.Keys() {
+		value, ok := props.Get(prop)
+		assert.True(t, ok)
+		assert.Equal(t, "value", value)
+	}
+}
+
+func TestExtractPropertiesFromFileAndSingleValue(t *testing.T) {
+	var tmpFile1 *os.File
+	var err error
+	if tmpFile1, err = ioutil.TempFile("", "camel-k-*.properties"); err != nil {
+		t.Error(err)
+	}
+
+	assert.Nil(t, tmpFile1.Close())
+	assert.Nil(t, ioutil.WriteFile(tmpFile1.Name(), []byte(`
+	key=value
+	#key2=value2
+	my.key=value
+	`), 0644))
+
+	properties := []string{"key=override", "file:" + tmpFile1.Name(), "my.key = override"}
+	props, err := mergePropertiesWithPrecedence(properties)
+	assert.Nil(t, err)
+	assert.Equal(t, 2, props.Len())
+	val, ok := props.Get("key")
+	assert.True(t, ok)
+	assert.Equal(t, "override", val)
+	val, ok = props.Get("my.key")
+	assert.True(t, ok)
+	assert.Equal(t, "override", val)
+}
diff --git a/pkg/cmd/run_test.go b/pkg/cmd/run_test.go
index 00ca7fb..6da2bf0 100644
--- a/pkg/cmd/run_test.go
+++ b/pkg/cmd/run_test.go
@@ -565,41 +565,6 @@ func TestResolveJsonPodTemplate(t *testing.T) {
 	assert.Equal(t, 2, len(integrationSpec.PodTemplate.Spec.Containers))
 }
 
-func TestExtractProperties_SingleKeyValue(t *testing.T) {
-	correctValues := []string{"key=val", "key = val", "key= val", " key   =  val"}
-	for _, val := range correctValues {
-		prop, err := extractProperties(val)
-		assert.Nil(t, err)
-		value, ok := prop.Get("key")
-		assert.True(t, ok)
-		assert.Equal(t, "val", value)
-	}
-}
-
-func TestExtractProperties_FromFile(t *testing.T) {
-	var tmpFile1 *os.File
-	var err error
-	if tmpFile1, err = ioutil.TempFile("", "camel-k-*.properties"); err != nil {
-		t.Error(err)
-	}
-
-	assert.Nil(t, tmpFile1.Close())
-	assert.Nil(t, ioutil.WriteFile(tmpFile1.Name(), []byte(`
-	key=value
-	#key2=value2
-	my.key=value
-	`), 0644))
-
-	props, err := extractProperties("file:" + tmpFile1.Name())
-	assert.Nil(t, err)
-	assert.Equal(t, 2, props.Len())
-	for _, prop := range props.Keys() {
-		value, ok := props.Get(prop)
-		assert.True(t, ok)
-		assert.Equal(t, "value", value)
-	}
-}
-
 func TestFilterBuildPropertyFiles(t *testing.T) {
 	inputValues := []string{"file:/tmp/test", "key=val"}
 	outputValues := filterBuildPropertyFiles(inputValues)

[camel-k] 02/03: feat(e2e): properties priority test

Posted by 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

commit 80b8d97e6f4031f51672ede60246a87f92787599
Author: Pasquale Congiusti <pa...@gmail.com>
AuthorDate: Thu Aug 12 15:39:31 2021 +0200

    feat(e2e): properties priority test
---
 e2e/common/config/config_test.go | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/e2e/common/config/config_test.go b/e2e/common/config/config_test.go
index 0a93e78..2aa9536 100644
--- a/e2e/common/config/config_test.go
+++ b/e2e/common/config/config_test.go
@@ -57,6 +57,14 @@ func TestRunConfigExamples(t *testing.T) {
 			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
 		})
 
+		t.Run("Property precedence", func(t *testing.T) {
+			Expect(Kamel("run", "-n", ns, "./files/property-file-route.groovy", "-p", "my.key.2=universe", "-p", "file:./files/my.properties").Execute()).To(Succeed())
+			Eventually(IntegrationPodPhase(ns, "property-file-route"), TestTimeoutMedium).Should(Equal(v1.PodRunning))
+			Eventually(IntegrationCondition(ns, "property-file-route", camelv1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(v1.ConditionTrue))
+			Eventually(IntegrationLogs(ns, "property-file-route"), TestTimeoutShort).Should(ContainSubstring("hello universe"))
+			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
+		})
+
 		// Configmap
 
 		// Store a configmap on the cluster
@@ -225,5 +233,13 @@ func TestRunConfigExamples(t *testing.T) {
 			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
 		})
 
+		t.Run("Build time property file with precedence", func(t *testing.T) {
+			Expect(Kamel("run", "-n", ns, "./files/build-property-file-route.groovy", "--build-property", "quarkus.application.name=my-overridden-application", "--build-property", "file:./files/quarkus.properties").Execute()).To(Succeed())
+			Eventually(IntegrationPodPhase(ns, "build-property-file-route"), TestTimeoutMedium).Should(Equal(v1.PodRunning))
+			Eventually(IntegrationCondition(ns, "build-property-file-route", camelv1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(v1.ConditionTrue))
+			Eventually(IntegrationLogs(ns, "build-property-file-route"), TestTimeoutShort).Should(ContainSubstring("my-overridden-application"))
+			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
+		})
+
 	})
 }

[camel-k] 03/03: doc(cli): properties priority

Posted by 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

commit ec908ea607e081a1f59c186dc9ff667e3fb71810
Author: Pasquale Congiusti <pa...@gmail.com>
AuthorDate: Thu Aug 12 15:39:52 2021 +0200

    doc(cli): properties priority
---
 docs/modules/ROOT/pages/configuration/build-time-properties.adoc | 5 +++++
 docs/modules/ROOT/pages/configuration/runtime-properties.adoc    | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/docs/modules/ROOT/pages/configuration/build-time-properties.adoc b/docs/modules/ROOT/pages/configuration/build-time-properties.adoc
index 4898ab0..b1ab7b1 100644
--- a/docs/modules/ROOT/pages/configuration/build-time-properties.adoc
+++ b/docs/modules/ROOT/pages/configuration/build-time-properties.adoc
@@ -50,6 +50,11 @@ kamel run --build-property=file:quarkus.properties build-property-file-route.gro
 
 The property file is parsed and its properties configured on the `Integration`. As soon as the application starts, you will see the log with the expected configuration.
 
+[[build-time-props-file-precedence]]
+== Property collision priority
+
+If you have a property repeated more than once, the general rule is that the last one declared in your `kamel run` statement will be taken in consideration. If the same property is found both in a single option declaration and inside a file, then, the single option will have higher priority and will be used.
+
 [[build-time-runtime-conf]]
 == Run time properties
 
diff --git a/docs/modules/ROOT/pages/configuration/runtime-properties.adoc b/docs/modules/ROOT/pages/configuration/runtime-properties.adoc
index 8ef5c9f..4fc3b94 100644
--- a/docs/modules/ROOT/pages/configuration/runtime-properties.adoc
+++ b/docs/modules/ROOT/pages/configuration/runtime-properties.adoc
@@ -51,6 +51,11 @@ You'll need to provide a `property` _file_ flag when launching the application:
 
 The property file is parsed and its properties configured on the `Integration`. As soon as the application starts, you will see the log with the expected configuration.
 
+[[runtime-props-file-precedence]]
+== Property collision priority
+
+If you have a property repeated more than once, the general rule is that the last one declared in your `kamel run` statement will be taken in consideration. If the same property is found both in a single option declaration and inside a file, then, the single option will have higher priority and will be used.
+
 [[runtime-build-time-conf]]
 == Build time properties