feat: add ability to read as builder

[metacosm]

No description provided.

[Sgitario]

Spite of you added this interface to avoid adding the io.dekorate.utils.Serialization dependency in the spi module, in practical, this method is useless unless you provide one implementation of this interface which can be hard to understand at first.

Instead, we should provide an utility maybe in the deployment module to read the HelmChartBuildItem instead of adding this method in here and avoid complicating the spi module.

[metacosm]

There are 2 implementations of that interface in this PR 😉

[Sgitario]

yes, what I disliked it more :)
Let's wait for @iocanel's comment before moving forward.
But these methods should be moved away from the builditem class.

[Sgitario]

@metacosm I see the intent of these changes of re-using some logic in both the processor and the build item. But instead we should provide an utility to read the builditem and parse the content of the folder, instead of adding this logic in the build item. This is for several reasons:

• Keep the spi module as light as possible.
• Use the io.dekorate.utils.Serialization util instead of providing an interface will guarantee that the builder will be safer and easier to use.

To be honest, I didn't add the HelmChartBuildItem class originally, and I see before these changes, there are too many logic to read/write. I need to clarify if this is indeed used somewhere.

[Sgitario]

@iocanel are the read/write methods from the HelmChartBuildItem class used somewhere in Quarkus? If so, can you share some links?

[metacosm]

I personally it makes sense to have the read/write methods in the SPI because they require internal knowledge of the extension that would need to be duplicated outside of it and would be brittle. This way, people don't have to know how to locate or load the appropriate files to populate the Chart model. One could argue, though, that might be better located on the model module than on the build item. That said, there currently isn't any model entity that encapsulates what the build item does.

[iocanel]

@metacosm @Sgitario: Sorry for the delay guys, I was between conferences, easter holidays and PTO. I'll have a look ASAP.

[iocanel]

So, the main motivation behind the HelmChartBuildItem is that I would to consume it from quarkus-backstage.

Why ?

Because we need to include generated Helm Charts inside generated Backstage templates.
However, getting the build item is not enough as we also need to read the generated files. This was impossible as files were written using GeneratedFileSystemResourceBuildItem which is consumed very late in the process. So, we had to have a write method. I don't recall why the read method was added but from what I see it's directly used in quarkus-backstage atm.

Another area where we are considering using HelmChartBuildItem is in quarkus-tekton where we'd like to either populate charts that include generated tekton pipelines or modify the existing ones.

[metacosm]

Yes, in our case (quarkus-operator-sdk), we'd like to enrich the default Helm charts as well, in a way similar to what we do with what gets generated by the kubernetes extension.

[Sgitario]

Yes, in our case (quarkus-operator-sdk), we'd like to enrich the default Helm charts as well, in a way similar to what we do with what gets generated by the kubernetes extension.

Still adding this kind of logic in a SPI module is a bad pattern for multiple reasons. The write/read methods should be moved away the build iteam and probably be added in a new module (?) for this specific use case, plus properly adding tests/documentation to avoid breaking changes.

[metacosm]

The point, though, is that things have been implemented that way. Maybe a better way could be considered for an hypothetical v2 version of the extension but we also have to be pragmatic: the extension has to meet its users where they are or people won't use it.

[Sgitario]

The point, though, is that things have been implemented that way. Maybe a better way could be considered for an hypothetical v2 version of the extension but we also have to be pragmatic: the extension has to meet its users where they are or people won't use it.

I disagree with you that if things are like this now, we need to keep iterating over this since we'll be adding more and more tech debt.