From 3a96804b1bfed56a8c1955005a7c346e5d322a57 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Thu, 13 Jun 2024 17:44:37 +0200 Subject: [PATCH] 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(-) rename pkg/{cmd => trait}/trait_support.go (92%) diff --git a/pkg/cmd/bind.go b/pkg/cmd/bind.go index d8b66be139..2f629adcb5 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 f488c7ee46..fa03c0718e 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 4eb1d5d7b9..0afbb1d19e 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 8ba5399b8f..3d3a19a5f4 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 b281f7b95a..8e8cc25c49 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 30af047067..9a6c692297 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 5ae94a1a49..eca4ba63f9 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 {