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 3a96804b1bfed56a8c1955005a7c346e5d322a57 Author: Pasquale Congiusti <pasquale.congiu...@gmail.com> AuthorDate: Thu Jun 13 17:44:37 2024 +0200 fix(traits): annotations refactoring The Pipe transform the annotations into .integration.spec.traits instead of transferring to Integration annotations. This should bring consistency as the Integration would manage the copy to IntegrationKit according its internal logic which is already available. Closes #5620 --- pkg/cmd/bind.go | 4 +- pkg/cmd/kit_create.go | 2 +- pkg/cmd/run.go | 4 +- pkg/cmd/run_test.go | 2 +- pkg/controller/pipe/integration.go | 64 +++++++++++++++++++++++++++++ pkg/controller/pipe/integration_test.go | 71 +++++++++++++++++++++++++++++++++ pkg/{cmd => trait}/trait_support.go | 23 +++++++---- 7 files changed, 156 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/bind.go b/pkg/cmd/bind.go index d8b66be13..2f629adcb 100644 --- a/pkg/cmd/bind.go +++ b/pkg/cmd/bind.go @@ -175,7 +175,7 @@ func (o *bindCmdOptions) validate(cmd *cobra.Command, args []string) error { } catalog := trait.NewCatalog(client) - return validateTraits(catalog, extractTraitNames(o.Traits)) + return trait.ValidateTraits(catalog, extractTraitNames(o.Traits)) } func (o *bindCmdOptions) run(cmd *cobra.Command, args []string) error { @@ -236,7 +236,7 @@ func (o *bindCmdOptions) run(cmd *cobra.Command, args []string) error { binding.Spec.Integration = &v1.IntegrationSpec{} } catalog := trait.NewCatalog(client) - if err := configureTraits(o.Traits, &binding.Spec.Integration.Traits, catalog); err != nil { + if err := trait.ConfigureTraits(o.Traits, &binding.Spec.Integration.Traits, catalog); err != nil { return err } } diff --git a/pkg/cmd/kit_create.go b/pkg/cmd/kit_create.go index f488c7ee4..fa03c0718 100644 --- a/pkg/cmd/kit_create.go +++ b/pkg/cmd/kit_create.go @@ -153,7 +153,7 @@ func (command *kitCreateCommandOptions) run(cmd *cobra.Command, args []string) e if err := command.parseAndConvertToTrait(command.Secrets, "mount.config"); err != nil { return err } - if err := configureTraits(command.Traits, &kit.Spec.Traits, catalog); err != nil { + if err := trait.ConfigureTraits(command.Traits, &kit.Spec.Traits, catalog); err != nil { return err } existed := false diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 4eb1d5d7b..0afbb1d19 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -297,7 +297,7 @@ func (o *runCmdOptions) validate(cmd *cobra.Command) error { } catalog := trait.NewCatalog(client) - return validateTraits(catalog, extractTraitNames(o.Traits)) + return trait.ValidateTraits(catalog, extractTraitNames(o.Traits)) } func filterBuildPropertyFiles(maybePropertyFiles []string) []string { @@ -561,7 +561,7 @@ func (o *runCmdOptions) createOrUpdateIntegration(cmd *cobra.Command, c client.C if len(o.Traits) > 0 { catalog := trait.NewCatalog(c) - if err := configureTraits(o.Traits, &integration.Spec.Traits, catalog); err != nil { + if err := trait.ConfigureTraits(o.Traits, &integration.Spec.Traits, catalog); err != nil { return nil, err } } diff --git a/pkg/cmd/run_test.go b/pkg/cmd/run_test.go index 8ba5399b8..3d3a19a5f 100644 --- a/pkg/cmd/run_test.go +++ b/pkg/cmd/run_test.go @@ -449,7 +449,7 @@ func TestConfigureTraits(t *testing.T) { catalog := trait.NewCatalog(client) traits := v1.Traits{} - err = configureTraits(runCmdOptions.Traits, &traits, catalog) + err = trait.ConfigureTraits(runCmdOptions.Traits, &traits, catalog) require.NoError(t, err) traitMap, err := trait.ToTraitMap(traits) diff --git a/pkg/controller/pipe/integration.go b/pkg/controller/pipe/integration.go index b281f7b95..8e8cc25c4 100644 --- a/pkg/controller/pipe/integration.go +++ b/pkg/controller/pipe/integration.go @@ -22,11 +22,13 @@ import ( "encoding/json" "fmt" "sort" + "strings" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + "github.com/apache/camel-k/v2/pkg/trait" "github.com/apache/camel-k/v2/pkg/client" "github.com/apache/camel-k/v2/pkg/platform" @@ -49,6 +51,10 @@ func CreateIntegrationFor(ctx context.Context, c client.Client, binding *v1.Pipe annotations := util.CopyMap(binding.Annotations) // avoid propagating the icon to the integration as it's heavyweight and not needed delete(annotations, v1.AnnotationIcon) + traits, err := extractAndDeleteTraits(c, annotations) + if err != nil { + return nil, fmt.Errorf("could not marshal trait annotations %w", err) + } it := v1.Integration{ ObjectMeta: metav1.ObjectMeta{ @@ -82,6 +88,14 @@ func CreateIntegrationFor(ctx context.Context, c client.Client, binding *v1.Pipe it.Spec = *binding.Spec.Integration.DeepCopy() } + if &it.Spec != nil && traits != nil { + it.Spec = v1.IntegrationSpec{} + } + + if traits != nil { + it.Spec.Traits = *traits + } + // Set replicas (or override podspecable value) if present if binding.Spec.Replicas != nil { replicas := *binding.Spec.Replicas @@ -210,6 +224,56 @@ func CreateIntegrationFor(ctx context.Context, c client.Client, binding *v1.Pipe return &it, nil } +// extractAndDeleteTraits will extract the annotation traits into v1.Traits struct, removing from the value from the input map. +func extractAndDeleteTraits(c client.Client, annotations map[string]string) (*v1.Traits, error) { + // structure that will be marshalled into a v1.Traits as it was a kamel run command + catalog := trait.NewCatalog(c) + traitsPlainParams := []string{} + for k, v := range annotations { + if strings.HasPrefix(k, v1.TraitAnnotationPrefix) { + key := strings.ReplaceAll(k, v1.TraitAnnotationPrefix, "") + traitId := strings.Split(key, ".")[0] + if err := trait.ValidateTrait(catalog, traitId); err != nil { + return nil, err + } + traitArrayParams := extractAsArray(v) + for _, param := range traitArrayParams { + traitsPlainParams = append(traitsPlainParams, fmt.Sprintf("%s=%s", key, param)) + } + delete(annotations, k) + } + } + if len(traitsPlainParams) == 0 { + return nil, nil + } + var traits v1.Traits + if err := trait.ConfigureTraits(traitsPlainParams, &traits, catalog); err != nil { + return nil, err + } + + return &traits, nil +} + +// extractTraitValue can detect if the value is an array representation as ["prop1=1", "prop2=2"] and +// return an array with the values or with the single value passed as a parameter. +func extractAsArray(value string) []string { + if strings.HasPrefix(value, "[") && strings.HasSuffix(value, "]") { + arrayValue := []string{} + data := value[1 : len(value)-1] + vals := strings.Split(data, ",") + for _, v := range vals { + prop := strings.Trim(v, " ") + if strings.HasPrefix(prop, `"`) && strings.HasSuffix(prop, `"`) { + prop = prop[1 : len(prop)-1] + } + arrayValue = append(arrayValue, prop) + } + return arrayValue + } + + return []string{value} +} + func configureBinding(integration *v1.Integration, bindings ...*bindings.Binding) error { for _, b := range bindings { if b == nil { diff --git a/pkg/controller/pipe/integration_test.go b/pkg/controller/pipe/integration_test.go index 30af04706..9a6c69229 100644 --- a/pkg/controller/pipe/integration_test.go +++ b/pkg/controller/pipe/integration_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" ) func TestCreateIntegrationForPipe(t *testing.T) { @@ -187,3 +188,73 @@ func expectedNominalRouteWithDataType(name string) string { id: binding ` } + +func TestExtractTraitAnnotations(t *testing.T) { + client, err := test.NewFakeClient() + require.NoError(t, err) + annotations := map[string]string{ + "my-personal-annotation": "hello", + v1.TraitAnnotationPrefix + "service.enabled": "true", + v1.TraitAnnotationPrefix + "container.image-pull-policy": "Never", + v1.TraitAnnotationPrefix + "camel.runtime-version": "1.2.3", + v1.TraitAnnotationPrefix + "camel.properties": `["prop1=1", "prop2=2"]`, + v1.TraitAnnotationPrefix + "environment.vars": `["env1=1"]`, + } + traits, err := extractAndDeleteTraits(client, annotations) + require.NoError(t, err) + assert.Equal(t, pointer.Bool(true), traits.Service.Enabled) + assert.Equal(t, corev1.PullNever, traits.Container.ImagePullPolicy) + assert.Equal(t, "1.2.3", traits.Camel.RuntimeVersion) + assert.Equal(t, []string{"prop1=1", "prop2=2"}, traits.Camel.Properties) + assert.Equal(t, []string{"env1=1"}, traits.Environment.Vars) + assert.Len(t, annotations, 1) + assert.Empty(t, annotations[v1.TraitAnnotationPrefix+"service.enabled"]) + assert.Empty(t, annotations[v1.TraitAnnotationPrefix+"container.image-pull-policy"]) + assert.Empty(t, annotations[v1.TraitAnnotationPrefix+"camel.runtime-version"]) + assert.Empty(t, annotations[v1.TraitAnnotationPrefix+"camel.properties"]) + assert.Empty(t, annotations[v1.TraitAnnotationPrefix+"environment.vars"]) + assert.Equal(t, "hello", annotations["my-personal-annotation"]) +} + +func TestExtractTraitAnnotationsError(t *testing.T) { + client, err := test.NewFakeClient() + require.NoError(t, err) + annotations := map[string]string{ + "my-personal-annotation": "hello", + v1.TraitAnnotationPrefix + "servicefake.bogus": "true", + } + traits, err := extractAndDeleteTraits(client, annotations) + require.Error(t, err) + assert.Equal(t, "trait servicefake does not exist in catalog", err.Error()) + assert.Nil(t, traits) + assert.Len(t, annotations, 2) +} + +func TestExtractTraitAnnotationsEmpty(t *testing.T) { + client, err := test.NewFakeClient() + require.NoError(t, err) + annotations := map[string]string{ + "my-personal-annotation": "hello", + } + traits, err := extractAndDeleteTraits(client, annotations) + require.NoError(t, err) + assert.Nil(t, traits) + assert.Len(t, annotations, 1) +} + +func TestCreateIntegrationTraitsForPipeWithTraitAnnotations(t *testing.T) { + client, err := test.NewFakeClient() + require.NoError(t, err) + + pipe := nominalPipe("my-pipe") + pipe.Annotations[v1.TraitAnnotationPrefix+"service.enabled"] = "true" + + it, err := CreateIntegrationFor(context.TODO(), client, &pipe) + require.NoError(t, err) + assert.Equal(t, "my-pipe", it.Name) + assert.Equal(t, "default", it.Namespace) + assert.Equal(t, map[string]string{ + "my-annotation": "my-annotation-val", + }, it.Annotations) + assert.Equal(t, pointer.Bool(true), it.Spec.Traits.Service.Enabled) +} diff --git a/pkg/cmd/trait_support.go b/pkg/trait/trait_support.go similarity index 92% rename from pkg/cmd/trait_support.go rename to pkg/trait/trait_support.go index 5ae94a1a4..eca4ba63f 100644 --- a/pkg/cmd/trait_support.go +++ b/pkg/trait/trait_support.go @@ -15,7 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd +package trait import ( "encoding/json" @@ -26,7 +26,6 @@ import ( "strings" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" - "github.com/apache/camel-k/v2/pkg/trait" "github.com/apache/camel-k/v2/pkg/util" "github.com/mitchellh/mapstructure" ) @@ -38,18 +37,26 @@ var knownAddons = []string{"keda", "master", "strimzi", "3scale", "tracing"} var traitConfigRegexp = regexp.MustCompile(`^([a-z0-9-]+)((?:\.[a-z0-9-]+)(?:\[[0-9]+\]|\..+)*)=(.*)$`) -func validateTraits(catalog *trait.Catalog, traits []string) error { +func ValidateTrait(catalog *Catalog, trait string) error { + tr := catalog.GetTrait(trait) + if tr == nil { + return fmt.Errorf("trait %s does not exist in catalog", trait) + } + + return nil +} + +func ValidateTraits(catalog *Catalog, traits []string) error { for _, t := range traits { - tr := catalog.GetTrait(t) - if tr == nil { - return fmt.Errorf("trait %s does not exist in catalog", t) + if err := ValidateTrait(catalog, t); err != nil { + return err } } return nil } -func configureTraits(options []string, traits interface{}, catalog trait.Finder) error { +func ConfigureTraits(options []string, traits interface{}, catalog Finder) error { config, err := optionsToMap(options) if err != nil { return err @@ -144,7 +151,7 @@ func optionsToMap(options []string) (optionMap, error) { return optionMap, nil } -func configureAddons(config optionMap, traits interface{}, catalog trait.Finder) error { +func configureAddons(config optionMap, traits interface{}, catalog Finder) error { // Addon traits require raw message mapping addons := make(map[string]v1.AddonTrait) for id, props := range config {