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 <pasquale.congiu...@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)