config: CNF installation (2.1) Transformer for old version of config

[svteb]

Description

The transformer transforms the old config according to the structure proposed in #2129.

Notes

1. To allow for some input validation, the v1 config was reimplemented as class in config_v1.cr.
2. Certain arguments are found in cnf-testsuite.yml files spread across the samples and examples that are unused anywhere in the code, these are still parsed but ignored to avoid too much manual intervention in later stages.
3. A new task has been added to allow for transformation, users can execute it by calling this command:

./cnf-testsuite transform_config OLD_FILE_PATH NEW_FILE_PATH

4. The ConfigTransformer class found in config_transformer.cr automatically detects the version of the old config and transforms it to the latest version (currently only v1 -> v2), but allows for future extendibility.
5. Upon determining the version of the old config, the main transform function is called which does the actual transformation in code (through use of Hashes and YAML::Any).
6. The transform function uses the appropriate transformation rules (V1ToV2Transformation class in v1_to_v2_transformation.cr file)
7. To make future extension of transformation rules easier, the two functions hash_to_yaml_any and remove_nil_values have been added (Transformation class in transformation.cr file), these convert the underlying hashes and arrays to the YAML::Any type and remove any nil branches so they don't clutter the final output.
8. After the transformation is done the result can be printed to either String or dumped to a file through the serialize_to_string and serialize_to_file functions.
9. Most edge cases have been resolved through the use of exceptions and their catching, giving users a readable output in case of any errors.

Issues:

Refs: #2130

How has this been tested:

• [ ] Covered by existing integration testing
• [ ] Added integration testing to cover
• [x] Verified all A/C passes
  • [ ] develop
  • [ ] master
  • [ ] tag/other branch
• [ ] Test environment
  • [ ] Shared Packet K8s cluster
  • [ ] New Packet K8s cluster
  • [x] Kind cluster
• [ ] Have not tested

Types of changes:

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Documentation update

Checklist:

Documentation

• [x] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] No updates required.

Code Review

• [ ] Does the test handle fatal exceptions, ie. rescue block

Issue

• [ ] Tasks in issue are checked off

[martin-mat]

Can the architecture be made more future-proof towards possible future config versions?

• Identify source version of the confing on the input
• Have possibility to tell which config version one needs to transform to (perhaps with hardcoded config version actually used by the testsuite as a constant somewhere)
• Make an easy way for future introduction of config v3.0 and newer, with possibility to transform from 1.0 and 2.0

[martin-mat]

Additionally, how about error handling? Like non-supported config keys on input? Maybe other error conditions?

[svteb]

Additionally, how about error handling? Like non-supported config keys on input? Maybe other error conditions?

I believe that this is out of scope of this ticket. These could all be considered validation.

To my knowledge the v1 config does not have a validator that we could reuse to verify the input data, but considering that all the sample/example cnfs are currently (hopefully) deployable I do not find it necessary to verify the input data. Afterall they will all be replaced in step 3 of #2120 epic.

As for the output validation, that also probably does not fall in scope of this ticket as that should be included in #2129 which is the main parser for v2. The current flow of the program for v2 config should be something like this:

1. ./cnf-testsuite cnf_setup config.yml
2. Execute parser from #2129 ~> Checks if the config is v1 or v2.
3. If v1, execute the transformer code which will return a transformed config back to the parser from #2129.
4. Parser then validates the input, etc.

As for error handling, the current format of "key" => @old_config["key"]?.as_s assures that if some key is not present its value will be stored as nil (which is inconsequential as the v2 class will also have nil values). Non-supported config keys are not recognized by the transformer because it looks only for the valid keys. In case of nested keys, I first check for existence of their parent. There should not be any conditions under which an error is given.

Input validation: I simply don't find this necessary, do we really want to spend time on validating the v1 config if we intend to replace it in the next couple of weeks and the testsuite worked without it for some time now?

[svteb]

Can the architecture be made more future-proof towards possible future config versions?

• Identify source version of the confing on the input
• Have possibility to tell which config version one needs to transform to (perhaps with hardcoded config version actually used by the testsuite as a constant somewhere)
• Make an easy way for future introduction of config v3.0 and newer, with possibility to transform from 1.0 and 2.0

This is a valid point, I will attempt to generalize the process/add more private functions.

[svteb]

Short summary of what was attempted during implementation and the solution:

I made multiple attempts at creating a generic solution that would allow for future extendibility, these have unfortunately hit some really hard roadblocks. The outline for a generic solution encompassed these properties:

1. Output of the transformer is easily parsed by the appropriate config class it is being converted to.
2. Future config formats (v2,3,...) would not require any changes to the main ConfigTransformer class, they would merely require a new appropriate rule set made up of transformations to be added.
3. The transformation rule sets would be easy to write (old key chain -> new key chain).
4. The transformer would resolve any reasonably expected/correct YAML configs.

The approaches: Rule sets The advanced approach would allow for rules like these to be defined:

[:container_names, :rolling_version_upgrade] -> [:common_parameters, :container_names, :rolling_version_upgrade]
[:helm_repo_name] -> [:deployments, :helm_charts, :helm_repo_name]
...

These would be easy to write, specifying where some prior key path should lead in the new config. The problems that come with this approach are numerous:

1. No easy way to resolve keys that repeat. In the above example the container_names can have many rolling_version_upgrade keys inside of it as part of a YAML sequence. This could be resolved by adding an additional value to the rule, something like key_repeats = true. But that would bloat the rules quite a bit and could be hard to understand for future users.
2. Repeat keys must be transformed in the correct order, that is a sequence of container_names must first finish and only then can another sequence of container_names be started. This could only be resolved by detecting the type of container_names as an array and the correctly attempting to parse it which would require some type of recursive/stack approach. This was determined to be quite unreadable.
3. Extracting data from a class like this with a simple key chain requires quite a bit of hacking, such as generating getter functions for all parameters using a macro, additionally we run into return types like Class or Array(Class) which crystals typing system is not fond of.

A simplified approach is to hard-code what variable should be assigned where, i.e. common_params["white_list_container_names"] = @old_config.white_list_container_names.to_s. This does not look as pretty but the jump from specific solution to a generic one is too vast.

Transformations

1. Making use of YAML::Builder methods to construct a String. This is supposedly the correct approach but it is exceedingly hard to implement as all possible types need to be resolved, this includes the types like Class/Array(Class). The key chains need to be order lexicographically because there is no possibility to move around the keys in String once its been constructed. Additionally there is a lot of prechecking necessary to determine which key chain should be constructed (to avoid empty keys), this gets harder with repeat keys/sequences. Also the YAML::Builder is pretty poorly documented and the solution that would come out of it would be very long, creating a brand new "reverse YAML parser" that is worthy of its own library module should not be within the scope of this ticket.
2. Class to class transformation. This could be a relatively short solution but it would necessitate that the target class has initializers for passed values, this would make the target class quite bloated (as currently the expected initialize method is through a YAML file). Having a future requirement like this for the users is likely not what we want.
3. Creating a temporary Hash/YAML::Any structure on the go while adding data from the old config. While this is not a pretty solution it allows for some semblence of a generic approach. The ConfigTransformer itself would not need to be rewritten in case of future extension, there would only need to be a pretty hefty transformation class/function that is passed to it. While not the prettiest, it is easily more readable and allows for quick fixes. It also does not impose any requirements on other parts of the codebase.

As things stand the third option is the only one that does not impose on other parts of the codebase and is relatively readable/short. While it does not grant an easy way for future users to transform to a new config, the construction of new transformation rule set should not take too long.

[svteb]

@martin-mat @kosstennbl @barmull ready for review.

[kosstennbl]

Re-created from different branch as #2147