Impact on committed generated files on the framework CI

[gilles-peskine-arm]

Mbed-TLS/TF-PSA-Crypto#690 adds a generated test data file that's committed in TF-PSA-Crypto, from code that's in the framework repository (added in Mbed-TLS/mbedtls-framework#278). This follows the design proposed in Mbed-TLS/mbedtls-framework#272.

The script tests/scripts/check_committed_generated_files.py in TF-PSA-Crypto checks that the committed generated files identical to what the generation scripts would produce. It runs as part of all.sh?

Problem: what happens when we change the generation code? This happens in the framework. Typically the reason will be to add more tests, often non-regression tests for a bug that we're fixing. When we update the framework pointer in TF-PSA-Crypto, we'll have to run tests/scripts/check_committed_generated_files.py -u to update the files. That's fine. But what about the CI in the mbedtls-framework repository? It will run the TF-PSA-Crypto all.sh and find that the committed generated files are not up to date, which is a CI failure.

How do we update a framework script that changes a committed generated files? Let's explore potential solutions.

Don't check up-to-date-ness

Solves the CI problem, but bad because we risk forgetting to update the tests. When fixing a bug, we might accidentally not run its non-regression test.

Check up-to-date-ness only in nightly jobs

We can put the up-to-date check as a release-only component in all.sh. Then the framework pull request CI wouldn't run it, but neither would the TF-PSA-Crypto pull request CI. So if we forget to update the file, the TF-PSA-Crypto nightly job would tell us.

Not ideal, but maybe acceptable?

Exempt the up-to-date check in the framework CI

We could arrange for the up-to-date check to be skipped in the framework CI. In the medium term, we are planning to not run the full CI on all branches as the framework CI. We were planning this to reduce the CI load, but we could also do this in the short term just for this up-to-date check.

Downside: if the framework changes break TF-PSA-Crypto in a way that isn't solved by just committing the updated files, we won't know about it.

Update committed generated files before running the framework CI on a branch

We could run tests/scripts/check_committed_generated_files.py -u (if the script exists in the branch under test) before running the branch tests as part of the framework CI. This is morally the right thing, in the sense that we think that the framework CI represents what would happen if we updated the framework pointer on every branch.

Problem: if we're adding non-regression tests for a branch, we really want the bug fix to be simultaneous with adding the tests. With this solution, we'd have a CI failure if we add the non-regression tests first, so we'd have to merge the bug fix first. But it makes things more difficult and riskier if we can't run the non-regression tests in the pull request that has the bug fix.

Require changes to be versioned

We could keep the CI as is, and require TF-PSA-Crypto to say “I want version 3 of the this test”. In the framework, when we make a change, we keep the existing output as version 3, and offer the new desired output as version 4. In TF-PSA-Crypto, whenever we're ready, we move from version 3 to version 4.

Downside: it may be hard to keep both versions working, especially if we make a change to some auxiliary module (e.g. for automatic determination of compile-time dependencies).

Downside: there's a risk that we'll forget to enable the new tests, although less so than in solutions that require multiple pull requests to the same repository.

Keep all scripts that generate committed files in their own repo

Downside: more backporting work.

Note that it's not just about the calling script, but also potentially about any auxiliary module that's in the framework.

[mpg]

Check up-to-date-ness only in nightly jobs
Not ideal, but maybe acceptable?

Yes, I think that's probably the best solution in the short term, as we already have a mechanism in place to run some tests only in the nightly, so this has very low risk and uncertainty. That's the point I really like about this a a short-term solution: no pre-requisite that risks being much more work than anticipated.

Exempt the up-to-date check in the framework CI

I think that's my favourite solution in the long term. It keeps the consistency check on the side that has both sources of information being compared: the generated file, and a pointer to a particular framework commit.

And as you mention, it should come naturally when we switch the framework PR job to run only a subset of test on all consuming branches. But we really shouldn't make this subset thing a pre-requisite for your ML-DSA work IMO.

[gilles-peskine-arm]

Note that for the ML-DSA work, I don't need to resolve this: it's enough to have the script detect the presence of test functions, which is coming in in Mbed-TLS/mbedtls-framework#282. What can't really be detected automatically is other changes such as adding new test cases for existing function. That's likely to happen when we fix a bug, which might be a security bug. I don't like needing new infrastructure as part of a bug fix, which is why I want to treat this problem with some urgency. But it's not strictly needed right now.