Module encapsulation Take 2

[greenatatlassian]

In a previous PR, #783, I had a first attempt at providing better encapsulation for CEKit modules.

The current implementation, as I see it, has a major flaw. It is not enough to reference a module name in e.g. an image descriptor and expect that module to be added to the image. If the module depends on a docker image to do some of it's work (e.g. to build an artefact), when adding that module to an image descriptor, one has to know that module also requires a "builder image" be defined in the top level image descriptor and what it should contain. It would make much more sense for that module to include within its definition the required builder image, and for CEKit to handle ensuring that builder image is available in the image build.

Again, I am very keen to hear the input of the maintainers/other users on this direction, and whether it makes sense.

This does not pass all tests, and is not quite complete, but we are actively using it for some proof of design work. Many of the failing tests are related to internal RedHat functionality—which I will not investigate.

Work that would be required to get this "release ready" is to improve the handling of the ModuleRegistry(s). Changes here mean that CEKit could silently fail if two different module definitions share the same name. Also repository definitions for modules within modules should perhaps leverage the parent module's repository (see second commit that "fixes" the test, a change that should probably not be required).

[rnc]

Thank you for these proposals. It is certainly a set of interesting ideas. I also concur that adding typing really helps make it easier to develop ; in fact the typing changes could probably be factored out and applied in either separate PRs / commits. Similarly for things like the ModuleRegistry extraction into its own file. This would make the diffing and understanding of the changes easier

In terms if the ideas behind the changes I think there could be a lot of merit in them. From looking through the current codebase, I don't think modules actually support FROM even though the documentation implies it which is an issue.

I don't quite understand why you have added a new build_images tag to the module descriptor when the descriptor should already support from. Is build_image meant to be an isolated representation of something that can 'build a module' ? i.e. not only a potential from, but whatever else is needed to compose this module? What was the reason for not instead using from?

[greenatatlassian]

@rnc

I don't quite understand why you have added a new build_images tag to the module descriptor when the descriptor should already support from. Is build_image meant to be an isolated representation of something that can 'build a module' ? i.e. not only a potential from, but whatever else is needed to compose this module? What was the reason for not instead using from?

Basically build_images contains multi-stage build images that must be put in the docker file before the final image build.

The current CEKit syntax is that image.yaml can contain either an image, or a YAML list of images—the latter being support for multi-stage build. This multi-stage build syntax (which I don't like) is not supported for module descriptors, so module descriptors need some other way of defining "builder/intermediate" images that they require. build_image may not be the best name here (names are hard), but I think it is much clearer (and simpler) to enable multi-stage builds by adding a key for them with the value being the list of image descriptors. (The list syntax currently used in image descriptors feels like a hack to me.)

[greenatatlassian]

In terms if the ideas behind the changes I think there could be a lot of merit in them. From looking through the current codebase, I don't think modules actually support FROM even though the documentation implies it which is an issue.

I would have to experiment, but I think a module can have a FROM in that it will pass schema validation, but its value will be ignored.