Refactor handling of inventory backend command line flag

[simu]

Proposed Changes

This PR switches the inventory backend command line flag to the form --inventory-backend=<backend name>. Additionally, we restrict the allowed values for the flag to the known backends.

To ensure the allowed values for the command line flag, and the known implementations are in sync, we introduce a dict holding the mapping from flag values to Inventory subclasses.

Finally, we ensure that the selected backend is cached as a string in cached.args["inventory-backend"]. Note that this is not how cached.args is used otherwise, but since it's unlikely that there ever will be a real command inventory-backend this should be fine.

Docs and Tests

• Tests added
• Updated documentation

[simu]

I was looking at updating the docs, and found that https://kapitan.dev/pages/commands/kapitan_dotfile/#inventory notes that you can set inventory.inventory-path to adjust the path to the inventory. However, this setting only applies for kapitan inventory but not for kapitan compile (which the docs might suggest).

Given that documentation, would it make sense to move the --inventory-path flag to the new inventory_backend parser, and actually use .kapitan config key inventory for all flags for that backend, instead of having inventory-backend as a new toplevel key in the config file?

[MatteoVoges]

Given that documentation, would it make sense to move the --inventory-path flag to the new inventory_backend parser, and actually use .kapitan config key inventory for all flags for that backend, instead of having inventory-backend as a new toplevel key in the config file?

Yeah, it would make sense if we want to add every single argument that affects some general kapitan functionality to every command. Another approach (and what I've done for nexenio-dev) would be to group arguments by functionality and not commands. This would mean. that we don't have to check if we in compile-args or inventory-args and we could organize the command-parsers to include the neccessary functionality-argument-groups.

This might be a more complex (maybe breaking) change and needs to be discussed with @ademariag and @ramaro. What do you all think?

For now, I think a top level inventory_backend key is okay.

[simu]

This might be a more complex (maybe breaking) change

Thinking about it again, it would be a breaking change for sure, if we start reading argument values for existing flags from .kapitan from a key which doesn't match the command that's executed. I'll document the current behavior for this PR and will add a note to the kapitan inventory section in the docs to make it more clear that users may need to set the same config in multiple sections.

[ademariag]

Perhaps we could have a global section?
In general the inventory backend or path is not something that makes sense to have different between commands. There might be also other options that we might want to move from the subcommand to a kapitan level setting, for instance refs path, libs etc.

I think global could be a good way to start cleaning things up, while still retaining the old behaviour

[simu]

Perhaps we could have a global section? In general the inventory backend or path is not something that makes sense to have different between commands. There might be also other options that we might want to move from the subcommand to a kapitan level setting, for instance refs path, libs etc.

I think global could be a good way to start cleaning things up, while still retaining the old behaviour

Having a global section makes sense to provide a migration path. However, I think this is best done as a separate PR since it will require a change to from_dot_kapitan() to handle cases where a shared flag is set in both the global and the command-specific section.

[simu]

I've created a PR which implements a possible way to introduce a global section in the config file, see #1131. Please note that I've rebased this PR onto the new PR, so we can cleanly make use of the new section in this PR.