You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by ts...@apache.org on 2022/07/08 06:12:09 UTC
[camel-k] 14/14: fix(cli): make promote aware of legacy and new trait configurations
This is an automated email from the ASF dual-hosted git repository.
tsato pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit 35ed0ee29c86caf824f17224d8a06751baeda07b
Author: Tadayoshi Sato <sa...@gmail.com>
AuthorDate: Tue Jul 5 18:10:28 2022 +0900
fix(cli): make promote aware of legacy and new trait configurations
---
pkg/cmd/describe.go | 6 +-
pkg/cmd/promote.go | 111 +++++++++++++++++++++----------------
pkg/cmd/run_test.go | 4 +-
pkg/controller/integration/kits.go | 8 +--
pkg/trait/trait_configure.go | 29 ++++------
pkg/trait/trait_configure_test.go | 2 +-
pkg/trait/util.go | 54 ++++++++++++++++--
pkg/trait/util_test.go | 99 ++++++++++++++++++++++++++++++++-
8 files changed, 229 insertions(+), 84 deletions(-)
diff --git a/pkg/cmd/describe.go b/pkg/cmd/describe.go
index 44e119f8e..5252f753a 100644
--- a/pkg/cmd/describe.go
+++ b/pkg/cmd/describe.go
@@ -51,15 +51,15 @@ func describeObjectMeta(w *indentedwriter.Writer, om metav1.ObjectMeta) {
}
func describeTraits(w *indentedwriter.Writer, traits interface{}) error {
- traitsMap, err := trait.ToMap(traits)
+ traitMap, err := trait.ToTraitMap(traits)
if err != nil {
return err
}
- if len(traitsMap) > 0 {
+ if len(traitMap) > 0 {
w.Writef(0, "Traits:\n")
- for id, trait := range traitsMap {
+ for id, trait := range traitMap {
w.Writef(1, "%s:\n", strings.Title(id))
// TODO: print the whole TraitSpec as Yaml
for k, v := range trait {
diff --git a/pkg/cmd/promote.go b/pkg/cmd/promote.go
index db9159475..4eb216c89 100644
--- a/pkg/cmd/promote.go
+++ b/pkg/cmd/promote.go
@@ -19,7 +19,6 @@ package cmd
import (
"context"
- "encoding/json"
"fmt"
"regexp"
"strings"
@@ -30,6 +29,7 @@ import (
traitv1 "github.com/apache/camel-k/pkg/apis/camel/v1/trait"
"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
"github.com/apache/camel-k/pkg/client"
+ "github.com/apache/camel-k/pkg/trait"
"github.com/apache/camel-k/pkg/util/camel"
"github.com/apache/camel-k/pkg/util/kamelets"
"github.com/apache/camel-k/pkg/util/kubernetes"
@@ -181,81 +181,83 @@ func (o *promoteCmdOptions) getIntegration(c client.Client, name string) (*v1.In
}
func (o *promoteCmdOptions) validateDestResources(c client.Client, it *v1.Integration) error {
- var traits map[string][]string
var configmaps []string
var secrets []string
var pvcs []string
var kamelets []string
// Mount trait
- mounts := it.Spec.Traits.Mount
- if err := json.Unmarshal(mounts.Configuration.RawMessage, &traits); err != nil {
+ mount, err := toPropertyMap(it.Spec.Traits.Mount)
+ if err != nil {
return err
}
- for t, v := range traits {
+ for t, v := range mount {
switch t {
case "configs":
- for _, cn := range v {
- if conf, parseErr := resource.ParseConfig(cn); parseErr == nil {
- if conf.StorageType() == resource.StorageTypeConfigmap {
- configmaps = append(configmaps, conf.Name())
- } else if conf.StorageType() == resource.StorageTypeSecret {
- secrets = append(secrets, conf.Name())
+ if list, ok := v.([]string); ok {
+ for _, cn := range list {
+ if conf, parseErr := resource.ParseConfig(cn); parseErr == nil {
+ if conf.StorageType() == resource.StorageTypeConfigmap {
+ configmaps = append(configmaps, conf.Name())
+ } else if conf.StorageType() == resource.StorageTypeSecret {
+ secrets = append(secrets, conf.Name())
+ }
+ } else {
+ return parseErr
}
- } else {
- return parseErr
}
}
case "resources":
- for _, cn := range v {
- if conf, parseErr := resource.ParseResource(cn); parseErr == nil {
- if conf.StorageType() == resource.StorageTypeConfigmap {
- configmaps = append(configmaps, conf.Name())
- } else if conf.StorageType() == resource.StorageTypeSecret {
- secrets = append(secrets, conf.Name())
+ if list, ok := v.([]string); ok {
+ for _, cn := range list {
+ if conf, parseErr := resource.ParseResource(cn); parseErr == nil {
+ if conf.StorageType() == resource.StorageTypeConfigmap {
+ configmaps = append(configmaps, conf.Name())
+ } else if conf.StorageType() == resource.StorageTypeSecret {
+ secrets = append(secrets, conf.Name())
+ }
+ } else {
+ return parseErr
}
- } else {
- return parseErr
}
}
case "volumes":
- for _, cn := range v {
- if conf, parseErr := resource.ParseVolume(cn); parseErr == nil {
- if conf.StorageType() == resource.StorageTypePVC {
- pvcs = append(pvcs, conf.Name())
+ if list, ok := v.([]string); ok {
+ for _, cn := range list {
+ if conf, parseErr := resource.ParseVolume(cn); parseErr == nil {
+ if conf.StorageType() == resource.StorageTypePVC {
+ pvcs = append(pvcs, conf.Name())
+ }
+ } else {
+ return parseErr
}
- } else {
- return parseErr
}
}
}
}
- // Openapi trait
- openapis := it.Spec.Traits.OpenAPI
- traits = map[string][]string{}
- if len(openapis.Configuration.RawMessage) > 0 {
- if err := json.Unmarshal(openapis.Configuration.RawMessage, &traits); err != nil {
- return err
- }
+ // OpenAPI trait
+ openapi, err := toPropertyMap(it.Spec.Traits.OpenAPI)
+ if err != nil {
+ return err
}
- for k, v := range traits {
- for _, cn := range v {
- if k == "configmaps" {
- configmaps = append(configmaps, cn)
- }
+ for k, v := range openapi {
+ if k != "configmaps" {
+ continue
}
- }
-
- // Kamelet trait
- kameletTrait := it.Spec.Traits.Kamelets
- var kameletListTrait map[string]string
- if len(kameletTrait.Configuration.RawMessage) > 0 {
- if err := json.Unmarshal(kameletTrait.Configuration.RawMessage, &kameletListTrait); err != nil {
- return err
+ if list, ok := v.([]string); ok {
+ configmaps = append(configmaps, list...)
+ break
}
+ }
- kamelets = strings.Split(kameletListTrait["list"], ",")
+ // Kamelets trait
+ kamelet, err := toPropertyMap(it.Spec.Traits.Kamelets)
+ if err != nil {
+ return err
+ }
+ if list, ok := kamelet["list"].(string); ok {
+ kamelets = strings.Split(list, ",")
}
sourceKamelets, err := o.listKamelets(c, it)
if err != nil {
@@ -297,6 +299,19 @@ func (o *promoteCmdOptions) validateDestResources(c client.Client, it *v1.Integr
return nil
}
+func toPropertyMap(src interface{}) (map[string]interface{}, error) {
+ propMap, err := trait.ToPropertyMap(src)
+ if err != nil {
+ return nil, err
+ }
+ // Migrate legacy configuration properties before promoting
+ if err := trait.MigrateLegacyConfiguration(propMap); err != nil {
+ return nil, err
+ }
+
+ return propMap, nil
+}
+
func (o *promoteCmdOptions) listKamelets(c client.Client, it *v1.Integration) ([]string, error) {
catalog, err := camel.DefaultCatalog()
if err != nil {
diff --git a/pkg/cmd/run_test.go b/pkg/cmd/run_test.go
index d826e2338..4e1fc28a2 100644
--- a/pkg/cmd/run_test.go
+++ b/pkg/cmd/run_test.go
@@ -373,9 +373,9 @@ func TestConfigureTraits(t *testing.T) {
err = configureTraits(runCmdOptions.Traits, &traits, catalog)
assert.Nil(t, err)
- traitsMap, err := trait.ToMap(traits)
+ traitMap, err := trait.ToTraitMap(traits)
assert.Nil(t, err)
- assert.Len(t, traitsMap, 5)
+ assert.Len(t, traitMap, 5)
assertTraitConfiguration(t, traits.Affinity, &traitv1.AffinityTrait{PodAffinity: pointer.Bool(false)})
assertTraitConfiguration(t, traits.Container, &traitv1.ContainerTrait{DeprecatedProbesEnabled: pointer.Bool(false)})
assertTraitConfiguration(t, traits.Environment, &traitv1.EnvironmentTrait{ContainerMeta: pointer.Bool(false)})
diff --git a/pkg/controller/integration/kits.go b/pkg/controller/integration/kits.go
index 2f029b037..06aa5fcf6 100644
--- a/pkg/controller/integration/kits.go
+++ b/pkg/controller/integration/kits.go
@@ -161,11 +161,11 @@ func kitMatches(kit1 *v1.IntegrationKit, kit2 *v1.IntegrationKit) (bool, error)
}
func hasMatchingTraits(traits interface{}, kitTraits interface{}) (bool, error) {
- traitsMap, err := trait.ToMap(traits)
+ traitMap, err := trait.ToTraitMap(traits)
if err != nil {
return false, err
}
- kitTraitsMap, err := trait.ToMap(kitTraits)
+ kitTraitMap, err := trait.ToTraitMap(kitTraits)
if err != nil {
return false, err
}
@@ -177,8 +177,8 @@ func hasMatchingTraits(traits interface{}, kitTraits interface{}) (bool, error)
continue
}
id := string(t.ID())
- it, ok1 := findTrait(traitsMap, id)
- kt, ok2 := findTrait(kitTraitsMap, id)
+ it, ok1 := findTrait(traitMap, id)
+ kt, ok2 := findTrait(kitTraitMap, id)
if !ok1 && !ok2 {
continue
diff --git a/pkg/trait/trait_configure.go b/pkg/trait/trait_configure.go
index 94ae5578f..658449b54 100644
--- a/pkg/trait/trait_configure.go
+++ b/pkg/trait/trait_configure.go
@@ -61,13 +61,15 @@ func (c *Catalog) Configure(env *Environment) error {
}
func (c *Catalog) configureTraits(traits interface{}) error {
- traitsMap, err := ToMap(traits)
+ traitMap, err := ToTraitMap(traits)
if err != nil {
return err
}
- for id, trait := range traitsMap {
+ for id, trait := range traitMap {
if id == "addons" {
+ // Handle addons later so that the configurations on the new API
+ // take precedence over the legacy addon configurations
continue
}
if err := c.configureTrait(id, trait); err != nil {
@@ -75,7 +77,7 @@ func (c *Catalog) configureTraits(traits interface{}) error {
}
}
// Addons
- for id, trait := range traitsMap["addons"] {
+ for id, trait := range traitMap["addons"] {
if addons, ok := trait.(map[string]interface{}); ok {
if err := c.configureTrait(id, addons); err != nil {
return err
@@ -88,7 +90,7 @@ func (c *Catalog) configureTraits(traits interface{}) error {
func (c *Catalog) configureTrait(id string, trait map[string]interface{}) error {
if catTrait := c.GetTrait(id); catTrait != nil {
- if err := decodeTrait(trait, catTrait, true); err != nil {
+ if err := decodeTrait(trait, catTrait); err != nil {
return err
}
}
@@ -96,21 +98,10 @@ func (c *Catalog) configureTrait(id string, trait map[string]interface{}) error
return nil
}
-func decodeTrait(in map[string]interface{}, target Trait, root bool) error {
- // decode legacy configuration first if it exists
- if root && in["configuration"] != nil {
- if config, ok := in["configuration"].(map[string]interface{}); ok {
- // for traits that had the same property name "configuration",
- // it needs to be renamed to "config" to avoid naming conflicts
- // (e.g. Knative trait).
- if config["configuration"] != nil {
- config["config"] = config["configuration"]
- }
-
- if err := decodeTrait(config, target, false); err != nil {
- return err
- }
- }
+func decodeTrait(in map[string]interface{}, target Trait) error {
+ // Migrate legacy configuration properties before applying to catalog
+ if err := MigrateLegacyConfiguration(in); err != nil {
+ return err
}
data, err := json.Marshal(&in)
diff --git a/pkg/trait/trait_configure_test.go b/pkg/trait/trait_configure_test.go
index 2c106fa39..ad4d7020b 100644
--- a/pkg/trait/trait_configure_test.go
+++ b/pkg/trait/trait_configure_test.go
@@ -223,7 +223,7 @@ func TestTraitDecode(t *testing.T) {
target, ok := newContainerTrait().(*containerTrait)
require.True(t, ok)
- err := decodeTrait(trait, target, true)
+ err := decodeTrait(trait, target)
require.NoError(t, err)
assert.Equal(t, false, pointer.BoolDeref(target.Enabled, true))
diff --git a/pkg/trait/util.go b/pkg/trait/util.go
index ef7b9ec31..c1310ec6e 100644
--- a/pkg/trait/util.go
+++ b/pkg/trait/util.go
@@ -236,8 +236,8 @@ func AssertTraitsType(traits interface{}) error {
return nil
}
-// ToMap accepts either v1.Traits or v1.IntegrationKitTraits and converts it to a map.
-func ToMap(traits interface{}) (map[string]map[string]interface{}, error) {
+// ToTraitMap accepts either v1.Traits or v1.IntegrationKitTraits and converts it to a map of traits.
+func ToTraitMap(traits interface{}) (map[string]map[string]interface{}, error) {
if err := AssertTraitsType(traits); err != nil {
return nil, err
}
@@ -246,12 +246,56 @@ func ToMap(traits interface{}) (map[string]map[string]interface{}, error) {
if err != nil {
return nil, err
}
- traitsMap := make(map[string]map[string]interface{})
- if err = json.Unmarshal(data, &traitsMap); err != nil {
+ traitMap := make(map[string]map[string]interface{})
+ if err = json.Unmarshal(data, &traitMap); err != nil {
return nil, err
}
- return traitsMap, nil
+ return traitMap, nil
+}
+
+// ToPropertyMap accepts a trait and converts it to a map of trait properties.
+func ToPropertyMap(trait interface{}) (map[string]interface{}, error) {
+ data, err := json.Marshal(trait)
+ if err != nil {
+ return nil, err
+ }
+ propMap := make(map[string]interface{})
+ if err = json.Unmarshal(data, &propMap); err != nil {
+ return nil, err
+ }
+
+ return propMap, nil
+}
+
+// MigrateLegacyConfiguration moves up the legacy configuration in a trait to the new top-level properties.
+// Values of the new properties always take precedence over the ones from the legacy configuration
+// with the same property names.
+func MigrateLegacyConfiguration(trait map[string]interface{}) error {
+ if trait["configuration"] == nil {
+ return nil
+ }
+
+ if config, ok := trait["configuration"].(map[string]interface{}); ok {
+ // For traits that had the same property name "configuration",
+ // the property needs to be renamed to "config" to avoid naming conflicts
+ // (e.g. Knative trait).
+ if config["configuration"] != nil {
+ config["config"] = config["configuration"]
+ delete(config, "configuration")
+ }
+
+ for k, v := range config {
+ if trait[k] == nil {
+ trait[k] = v
+ }
+ }
+ delete(trait, "configuration")
+ } else {
+ return errors.Errorf(`unexpected type for "configuration" field: %v`, reflect.TypeOf(trait["configuration"]))
+ }
+
+ return nil
}
// ToTrait unmarshals a map configuration to a target trait.
diff --git a/pkg/trait/util_test.go b/pkg/trait/util_test.go
index 3a56bf029..c44a3e26b 100644
--- a/pkg/trait/util_test.go
+++ b/pkg/trait/util_test.go
@@ -28,11 +28,15 @@ import (
"github.com/stretchr/testify/assert"
)
-func TestToMap(t *testing.T) {
+func TestToTraitMap(t *testing.T) {
traits := v1.Traits{
Container: &traitv1.ContainerTrait{
Trait: traitv1.Trait{
Enabled: pointer.Bool(true),
+ Configuration: configurationFromMap(t, map[string]interface{}{
+ "name": "test-container",
+ "port": 8082,
+ }),
},
Auto: pointer.Bool(false),
Expose: pointer.Bool(true),
@@ -61,6 +65,10 @@ func TestToMap(t *testing.T) {
"portName": "http-8081",
"servicePort": float64(81),
"servicePortName": "http-81",
+ "configuration": map[string]interface{}{
+ "name": "test-container",
+ "port": float64(8082),
+ },
},
"service": {
"enabled": true,
@@ -72,12 +80,91 @@ func TestToMap(t *testing.T) {
},
}
- traitMap, err := ToMap(traits)
+ traitMap, err := ToTraitMap(traits)
assert.NoError(t, err)
assert.Equal(t, expected, traitMap)
}
+func TestToPropertyMap(t *testing.T) {
+ trait := traitv1.ContainerTrait{
+ Trait: traitv1.Trait{
+ Enabled: pointer.Bool(true),
+ Configuration: configurationFromMap(t, map[string]interface{}{
+ "name": "test-container",
+ "port": 8082,
+ }),
+ },
+ Auto: pointer.Bool(false),
+ Expose: pointer.Bool(true),
+ Port: 8081,
+ PortName: "http-8081",
+ ServicePort: 81,
+ ServicePortName: "http-81",
+ }
+ expected := map[string]interface{}{
+ "enabled": true,
+ "auto": false,
+ "expose": true,
+ "port": float64(8081),
+ "portName": "http-8081",
+ "servicePort": float64(81),
+ "servicePortName": "http-81",
+ "configuration": map[string]interface{}{
+ "name": "test-container",
+ "port": float64(8082),
+ },
+ }
+
+ propMap, err := ToPropertyMap(trait)
+
+ assert.NoError(t, err)
+ assert.Equal(t, expected, propMap)
+}
+
+func TestMigrateLegacyConfiguration(t *testing.T) {
+ trait := map[string]interface{}{
+ "enabled": true,
+ "auto": false,
+ "port": float64(8081),
+ "portName": "http-8081",
+ "servicePortName": "http-81",
+ "configuration": map[string]interface{}{
+ "auto": true,
+ "expose": true,
+ "name": "test-container",
+ "port": float64(8082),
+ "servicePort": float64(81),
+ },
+ }
+ expected := map[string]interface{}{
+ "enabled": true,
+ "auto": false,
+ "port": float64(8081),
+ "portName": "http-8081",
+ "servicePortName": "http-81",
+ "expose": true,
+ "name": "test-container",
+ "servicePort": float64(81),
+ }
+
+ err := MigrateLegacyConfiguration(trait)
+
+ assert.NoError(t, err)
+ assert.Equal(t, expected, trait)
+}
+
+func TestMigrateLegacyConfiguration_invalidConfiguration(t *testing.T) {
+ trait := map[string]interface{}{
+ "enabled": true,
+ "configuration": "It should not be a string!",
+ }
+
+ err := MigrateLegacyConfiguration(trait)
+
+ assert.Error(t, err)
+}
+
func TestToTrait(t *testing.T) {
config := map[string]interface{}{
"enabled": true,
@@ -87,10 +174,18 @@ func TestToTrait(t *testing.T) {
"portName": "http-8081",
"servicePort": 81,
"servicePortName": "http-81",
+ "configuration": map[string]interface{}{
+ "name": "test-container",
+ "port": float64(8082),
+ },
}
expected := traitv1.ContainerTrait{
Trait: traitv1.Trait{
Enabled: pointer.Bool(true),
+ Configuration: configurationFromMap(t, map[string]interface{}{
+ "name": "test-container",
+ "port": 8082,
+ }),
},
Auto: pointer.Bool(false),
Expose: pointer.Bool(true),