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 <[email protected]> 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),
