-
Task
-
Resolution: Unresolved
-
Minor
-
None
-
None
-
None
The MCO repository receives a lot of PR traffic for items we don't control or have complete context of. An example of this is everything within the templates/ directory. Static analysis is useful for identifying potential problems in code. Shell scripts, in particular, are prone to unintended consequences and side-effects. While developers might use static analysis tools locally to write better code, it's also a useful quality gate to have CI check for and enforce.
By way of example, the Openshift Release repository runs https://github.com/koalaman/shellcheck on all step registry scripts as part of its CI process: https://github.com/openshift/release/blob/master/hack/validate-registry-commands.sh
While it would be great to adopt that script for our purposes, our template files present two special challenges:
- There are scripts / systemd units / etc. embedded within a YAML file, meaning we need to extract the contents to redirect them to our static analysis tools. Something like https://github.com/mikefarah/yq works nicely for this purpose.
- In addition to the above, some of these files also contain Golang template tags. These tags make them invalid YAML, meaning that yq cannot immediately process them. We could use something like https://docs.gomplate.ca/ to render them with fake data before running them through yq.
There are additional linters such as (https://github.com/priv-kweihmann/systemdlint or https://github.com/mackwic/systemd-linter) that could also be run on the systemd templates.
As a proof of concept (requires ripgrep, yq, gomplate, and shellcheck):
- $ rg -g '*.yaml' --files-without-match -F '{{' ./templates | xargs rg --files-with-matches -F '#!/bin/bash' | xargs yq eval '.contents.inline' | shellcheck -
- $ echo '{"Network": {"MTUMigration": true' | gomplate -c '.=stdin:?type=application/json' -f ./templates/common/_base/files/mtu-migration.yaml | yq eval '.contents.inline' - | shellcheck -}}
Done When:
- We've modified our Dockerfile to retrieve the necessary tools for this as part of the builder stage. This won't affect our final container image since we're using multi-stage Dockerfiles.
- A script is written to make use of the aforementioned tools to accomplish the above goals.
- A new Makefile target is added to the MCO's Makefile to call the aforementioned scripts.
- The MCO job definitions (https://github.com/openshift/release/blob/master/ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-master.yaml) are updated with an additional optional test to invoke the new Makefile target. CI impact will be minimal as this will run concurrently with our unit tests / lint checks and will take approximately the same amount of time or less.
- After communication to the teams who maintain templates and adequate time has passed (time period TBD), we can change this to being a required test.
- Alternatively, if the templates mechanism is rendered obsolete by CoreOS layering, this issue would most likely be rendered moot.
- depends on
-
MCO-468 MCO should use a build_root image in OpenShift CI
- To Do
- links to