[discussion] feat: configuration trait

[squakez]

This draft PR serves as an helper to discuss the changes required to develop feature #2320.

1. Right now a property file or other resources attached to an Integration, are materialized via deployment, cron or knative_service traits. If we introduce the configuration trait we can let this take care of this aspect, although I am not entirely sure if this will have some drawback I am not able to see at the moment (see the changes proposed). It basically means moving the logic that is taking care of it (e.computeConfigmaps()) from the 3 trait into the other one. We are introducing some trait dependency (to configuration trait), but I don't see this a big concern.

2. If we're happy with point 1, then we'll need to start moving the logic that is happening now in the e.computeConfigmaps() directly in the configuration trait. As an example, in this PR you will see I've done this with the Properties parameter which is going to take care of the user properties.

3. What was done here with Properties will have to be extended with all the other user configuration such as config, resource, ... It seems doable with no big concerns either.

4. ConfigurationSpec is widely used in Integration, IntegrationKit and only in one case in IntegrationPlatform. If we proceed with this design change we may be in the situation to extend the trait to the kit as well. Or we can keep it in the kit and the platform and just remove from the integration spec only.

5. The backward compatibility is something we will have to take case as well. I think we can find some way to let both approach (integration.spec.configspec and configuration trait) coexist, deprecating the first and eventually remove it.

It does not look an easy development as there are several dependencies to the ConfigurationSpec. In any case, with the approach proposed we will be able to have all in a unique place, although it means we will depends on a trait. With the changes proposed here I manage to make an integration work seamlessly.

@astefanutti, @lburgazzoli you were involved in the past in this discussion, I'll gladly receive your feedback to confirm if the approach is fine and it's worth to keep ahead with this.

CC @nicolaferraro @oscerd @doru1004 @jamesnetherton @johnpoth @tadayosi @orpiske @davsclaus, happy if you can have a look as well.

Release Note

NONE

[lburgazzoli]

1. Right now a property file or other resources attached to an `Integration`, are materialized via `deployment`, `cron` or `knative_service` traits. If we introduce the `configuration` trait we can let this take care of this aspect, although I am not entirely sure if this will have some drawback I am not able to see at the moment (see the changes proposed). It basically means moving the logic that is taking care of it (`e.computeConfigmaps()`) from the 3 trait into the other one. We are introducing some trait dependency (to `configuration` trait), but I don't see this a big concern.

2. If we're happy with point 1, then we'll need to start moving the logic that is happening now in the `e.computeConfigmaps()` directly in the configuration trait. As an example, in this PR you will see I've done this with the `Properties` parameter which is going to take care of the user properties.

3. What was done here with `Properties` will have to be extended with all the other user configuration such as `config`, `resource`, ... It seems doable with no big concerns either.

I think that implementing those points is a good opportunity to cleanup the code and make it more consistent so +1 with me

4. `ConfigurationSpec` is widely used in `Integration`, `IntegrationKit` and only in one case in `IntegrationPlatform`. If we proceed with this design change we may be in the situation to extend the trait to the kit as well. Or we can keep it in the kit and the platform and just remove from the integration spec only.

This point is not super clear to me, in particular the reason to remove the trait from the integration.

5. The backward compatibility is something we will have to take case as well. I think we can find some way to let both approach (`integration.spec.configspec` and `configuration` trait) coexist, deprecating the first and eventually remove it.

I think that trait.Configure for the configuration trait, should by default attempt to converting the .spec.config to its own structure so we can support the two mode for some time but we would swich to the trait internally

[squakez]

Thanks Luca, some clarification inline:

1. Right now a property file or other resources attached to an `Integration`, are materialized via `deployment`, `cron` or `knative_service` traits. If we introduce the `configuration` trait we can let this take care of this aspect, although I am not entirely sure if this will have some drawback I am not able to see at the moment (see the changes proposed). It basically means moving the logic that is taking care of it (`e.computeConfigmaps()`) from the 3 trait into the other one. We are introducing some trait dependency (to `configuration` trait), but I don't see this a big concern.

2. If we're happy with point 1, then we'll need to start moving the logic that is happening now in the `e.computeConfigmaps()` directly in the configuration trait. As an example, in this PR you will see I've done this with the `Properties` parameter which is going to take care of the user properties.

3. What was done here with `Properties` will have to be extended with all the other user configuration such as `config`, `resource`, ... It seems doable with no big concerns either.

I think that implementing those points is a good opportunity to cleanup the code and make it more consistent so +1 with me

4. `ConfigurationSpec` is widely used in `Integration`, `IntegrationKit` and only in one case in `IntegrationPlatform`. If we proceed with this design change we may be in the situation to extend the trait to the kit as well. Or we can keep it in the kit and the platform and just remove from the integration spec only.

This point is not super clear to me, in particular the reason to remove the trait from the integration.

I did not explain properly. I was meaning that, as soon as we go deep in the development, we may be in the opportunity to apply the same mechanics used in the new configuration trait also for the ConfigurationSpec now hold by the IntegrationKit. In any case it's too early to say. I will come back on this point if it needs further discussion.

5. The backward compatibility is something we will have to take case as well. I think we can find some way to let both approach (`integration.spec.configspec` and `configuration` trait) coexist, deprecating the first and eventually remove it.

I think that trait.Configure for the configuration trait, should by default attempt to converting the .spec.config to its own structure so we can support the two mode for some time but we would swich to the trait internally

It's a good approach. I was thinking about maintaining temporarily the repeated logic (ie, e.computeConfigmaps() + the new logic), though I don't like repeated code. I will apply your suggestion as it looks cleaner.

[orpiske]

I think those are good points and I don't really have anything to add to them right now. I do have a comment about the following:

5. The backward compatibility is something we will have to take case as well. I think we can find some way to let both approach (`integration.spec.configspec` and `configuration` trait) coexist, deprecating the first and eventually remove it.

I think that trait.Configure for the configuration trait, should by default attempt to converting the .spec.config to its own structure so we can support the two mode for some time but we would swich to the trait internally

+1. I don't really worry with the specifics of the technical implementation at this point, but I think we should aim for a solution that allows seamless migration from one to another from the user perspective.

[squakez]

Superseded by #2635