fix: implement compose_target_name

[MatteoVoges]

Proposed changes

• fix functionality for compose-node-name
  • rename compose-node-name to compose-target-name, because it should be kapitan-lingo
  • mark compose-node-name deprecated
  • add test for compose-target-name for reclass
• update examples to be compatible with compose-target-name (no breaking changes, just improvements)
• add new scoping for cached.args

[MatteoVoges]

Will add tests for compose_node_name so that it won't happen again, that we break it!

[MatteoVoges]

Currently this test fails:

    def test_compile_not_matching_targets(self):
        with (
            self.assertLogs(logger="kapitan.targets", level="ERROR") as cm,
            contextlib.redirect_stdout(io.StringIO()),
        ):
            # as of now, we cannot capture stdout with contextlib.redirect_stdout
            # since we only do logger.error(e) in targets.py before exiting
            with self.assertRaises(SystemExit) as ca:
                unmatched_filename = "inventory/targets/minikube-es-fake.yml"
                correct_filename = "inventory/targets/minikube-es.yml"
                os.rename(src=correct_filename, dst=unmatched_filename)
                sys.argv = ["kapitan", "compile"]

                try:
                    main()
                finally:
                    # correct the filename again, even if assertion fails
                    if os.path.exists(unmatched_filename):
                        os.rename(src=unmatched_filename, dst=correct_filename)
        error_message_substr = "is missing the corresponding yml file"
        self.assertTrue(" ".join(cm.output).find(error_message_substr) != -1)

And this makes sense, because the test tests actually unintended behavior.

1. If the file name and target name does not match, I don't want a SystemExit and
2. I don't want to search the logs for is missing the file.

[MatteoVoges]

And this makes sense, because the test tests actually unintended behavior.
1. If the file name and target name does not match, I don't want a SystemExit and
2. I don't want to search the logs for is missing the file.

The error happens in the validate_matching_target_name function. Because I modified the examples to use the name ${_reclass_:name:full}, which is the filename, the test does not succeed anymore.

Now the question is, if we handle a mismatch of filename and parameters.kapitan.vars.target?
And I surely will modify the test.

[ademariag]

Now the question is, if we handle a mismatch of filename and parameters.kapitan.vars.target? And I surely will modify the test.

Context, we added this behaviour before starting using ${_reclass_:name:short}, and it was to catch the case in which someone would copy the target file without changing the target parameter properly. Nowadays we should discourage to set the target manually, making it automatically get the value from the filename.

Reasoning...

I usually have this setup:

  target: ${_reclass_:name:full}
  target_name: ${_reclass_:name:short}
  target_path: ${_reclass_:name:path}
  
  ...
  
  kapitan:
    vars:
      target: ${target}

IMHO compose_target_name=True should be interpreted as "automatically infer the target name from the directory/path", so a mismatch between parameters.kapitan.vars.target and the "composed" target name should fail

for inventory/targets/test1/test2.yml

with compose_target_name=True:

• ${_reclass_:name:short}=test2
• ${_reclass_:name:full}=test1.test2

path/filename should match test1/test2 (just filename check is not enough because it could be in the directory test3/)

I think the test should fail if this is not true, and warn that you should not override the target manually

However with compose_target_name=False:
${_reclass_:name:short}=test2
${_reclass_:name:full}=test2

we should allow for someone to change the target in cases like:

test1/test2.yml
test3/test2.yml

as otherwise they would both be test2 and we want to allow for this manual setup.

I believe long term compose_target_name=False should be the default, perhaps in the upcoming release?

[ademariag]

partially implemented by #1164 which was an urgent fix. Please rebase from that.

[ademariag]

#1173