feat(template): Check that Helm defined templates carry the chart name

[lfrancke]

Adds a script called by prek which walks our charts and checks whether all defines/calls use a namespaced name

I tested it locally:

❯ python3 scripts/check_namespaced_defines.py deploy/helm/trino-operator/templates/*
  deploy/helm/trino-operator/templates/_helpers.tpl: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.appname
    operator.chart
    operator.fullname
    operator.image
    operator.labels
    operator.name
    operator.selectorLabels
    operator.serviceAccountName
    operator.testLabels
  deploy/helm/trino-operator/templates/_maintenance.tpl: defined templates and the calls to them must be prefixed with 'trino-operator.'
    maintenance.envVars
  deploy/helm/trino-operator/templates/_telemetry.tpl: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.appname
    telemetry.envVars
  deploy/helm/trino-operator/templates/clusterrole-operator.yaml: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.fullname
    operator.labels
    operator.name
  deploy/helm/trino-operator/templates/clusterrole-product.yaml: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.labels
    operator.name
  deploy/helm/trino-operator/templates/deployment.yaml: defined templates and the calls to them must be prefixed with 'trino-operator.'
    maintenance.envVars
    operator.appname
    operator.fullname
    operator.image
    operator.labels
    operator.selectorLabels
    operator.serviceAccountName
    telemetry.envVars
  deploy/helm/trino-operator/templates/service.yaml: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.fullname
    operator.labels
    operator.selectorLabels
  deploy/helm/trino-operator/templates/serviceaccount.yaml: defined templates and the calls to them must be prefixed with 'trino-operator.'
    operator.fullname
    operator.labels
    operator.serviceAccountName

This was a checkout before the rollout of #645
It succeeds on main now.

stackabletech/issues#882

[Techassi]

What does read_text do here? I think we should explain it, because at least I don't understand it just from context. It might be obvious for people used to Python, but it is not obvious for me (and potentially a bunch of other people).

[Techassi]

If it does what I assume it does - reading the file as text - this is even more surprising and should be documented. Usually, filesystem io goes through a file first: File::open(path).read() (pseudo code).

[lfrancke]

https://docs.python.org/3/library/pathlib.html#pathlib.Path.read_text

This also exists in Java and in Rust, no? Shortcuts to just read the full file?
Honest question: What do you find surprising here? I read the code and thought "yup, makes sense"
So on this I'll push back and say: This is the normal way I learned.

[Techassi]

Mhm yeah. I find it very unintuitive personally, especially coming from Rust. In Rust we have this:

// The long form
let mut contents = String::new();
let file = std::fs::File::open("my-path")?;
let n = file.read(&mut contents)?;

// The short form
let contents = std::fs::read_to_string("my-path")?;

Both forms make it very obvious - in my opinion - what the code does.

When I saw the Python code, my mind didn't connect the dots at all before reading it multiple times and seeing how it is used by unprefixed(). I find the jump from a plain path to the contents of (a potential) file at that path very odd.

[Techassi]

It is generally nice that there are some tests, but how exactly are they executed?

[lfrancke]

That's documented right at the beginning of the file. They are not executed automatically.

[lfrancke]

python3 -m unittest check_namespaced_defines.py

[lfrancke]

But.... it might be relatively easy to add them to CI, I'll see about that.

[lfrancke]

Should be possible. Question is if it should go into the prek file and then tested indirectly via github actions or directly into github actions and leave the test out of prek. Opinions?

[Techassi]

That's documented right at the beginning of the file. They are not executed automatically.

Ah I see. I kinda skipped that block of text. Could we maybe leave a simple comment here either stating the command or referring back to the top of the file?

Question is if it should go into the prek file and then tested indirectly via github actions or directly into github actions and leave the test out of prek. Opinions?

I would say add it to prek. For people who have it enabled locally to run on git commit, it will immediately be tested and validates that any potential change still passes. Additionally, we can more easily scope based on file paths.

Only having it in CI as a separate workflow delays the time one sees if the tests still pass - potentially leading to a follow-up fix commit.