Skip to content

Run Crypto abi-check in TF-PSA-Crypto PRs #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bensze01 wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Run Crypto abi-check in TF-PSA-Crypto PRs #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
67cb2b3
Run Crypto abi-check in TF-PSA-Crypto PRs
bensze01 Dec 12, 2025
1d08642
Allow TARGET_REPO to be parametrized
bensze01 Dec 12, 2025
24c454d
Replace uses of RUN_TF_PSA_CRYPTO_ALL_SH with TARGET_REPO
bensze01 Dec 16, 2025
cd02e62
Call list_internal_identifiers.py directly
bensze01 Dec 17, 2025
b739688
Fix TARGET_REPO logic for filtering tests
bensze01 Dec 17, 2025
d3d4a4f
Use exact repo names in TARGET_REPO
bensze01 Dec 16, 2025
bfa3990
Document TEST_REPO in Jenkinsfiles
bensze01 Dec 17, 2025
f271e51
Fix typo in Jenkinsfiles
bensze01 Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion vars/analysis.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fi
}
}

String script_in_docker = info.repo == 'tls' ? "tests/scripts/analyze_outcomes.py '$outcomes_csv'" : ''
String script_in_docker = info.repo == 'mbedtls' ? "tests/scripts/analyze_outcomes.py '$outcomes_csv'" : ''

Closure post_execution = {
sh "[ -f '$outcomes_csv' ] && xz -0 -T0 '$outcomes_csv'"
Expand Down
16 changes: 8 additions & 8 deletions vars/checkout_repo.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ String get_submodule_commit(String working_dir = '.', String submodule) {
}

private void checkout_framework_repo(BranchInfo info) {
if (env.TARGET_REPO == 'framework' && env.CHECKOUT_METHOD == 'scm') {
if (env.TARGET_REPO == 'mbedtls-framework' && env.CHECKOUT_METHOD == 'scm') {
checkout_report_errors(scm)
} else {
def branch = env.FRAMEWORK_BRANCH ?: info.framework_override
Expand All @@ -105,7 +105,7 @@ private void checkout_framework_repo(BranchInfo info) {
}

private void checkout_tf_psa_crypto_repo(BranchInfo info) {
if (env.TARGET_REPO == 'tf-psa-crypto' && env.CHECKOUT_METHOD == 'scm') {
if (env.TARGET_REPO == 'TF-PSA-Crypto' && env.CHECKOUT_METHOD == 'scm') {
checkout_report_errors(scm)
if (!info.framework_override) {
if (!isUnix()) {
Expand All @@ -116,13 +116,13 @@ private void checkout_tf_psa_crypto_repo(BranchInfo info) {
}
} else {
String branch
if (info.repo == 'tf-psa-crypto') {
if (info.repo == 'TF-PSA-Crypto') {
branch = info.branch
} else {
branch = env.TF_PSA_CRYPTO_BRANCH
}
if (env.TF_PSA_CRYPTO_REPO && branch) {
if (info.repo != 'tf-psa-crypto') {
if (info.repo != 'TF-PSA-Crypto') {
echo "Applying tf-psa-crypto override ($branch)"
}
try_checkout_from_repos([env.TF_PSA_CRYPTO_REPO, env.TF_PSA_CRYPTO_FALLBACK_REPO], branch)
Expand All @@ -141,12 +141,12 @@ private void checkout_tf_psa_crypto_repo(BranchInfo info) {
}

private Map<String, String> checkout_tls_repo(BranchInfo info) {
if (info.repo != 'tls') {
if (info.repo != 'mbedtls') {
throw new IllegalArgumentException("checkout_tls_repo() called with BranchInfo for repo '$info.repo'")
}

def scm_config
if (env.TARGET_REPO == 'tls' && env.CHECKOUT_METHOD == 'scm') {
if (env.TARGET_REPO == 'mbedtls' && env.CHECKOUT_METHOD == 'scm') {
scm_config = scm
} else {
scm_config = parametrized_repo(env.MBED_TLS_REPO, info.branch)
Expand Down Expand Up @@ -178,10 +178,10 @@ void checkout_repo(BranchInfo info) {
lock(resource: "stash-lock/${env.BUILD_TAG}-${stashName}") {
if (!info.stash) {
switch (info.repo) {
case 'tls':
case 'mbedtls':
checkout_tls_repo(info)
break
case 'tf-psa-crypto':
case 'TF-PSA-Crypto':
checkout_tf_psa_crypto_repo(info)
break
default:
Expand Down
2 changes: 1 addition & 1 deletion vars/common.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ List<BranchInfo> get_branch_information(Collection<String> tls_branches, Collect
List<BranchInfo> infos = []
Map<String, Object> list_components_jobs = [:]

Map<String, Collection<String>> repos = ['tls': tls_branches, 'tf-psa-crypto': tf_psa_crypto_branches]
Map<String, Collection<String>> repos = ['mbedtls': tls_branches, 'TF-PSA-Crypto': tf_psa_crypto_branches]
// Filter out repos with no branches
repos = repos.findAll({repo, branches -> branches})

Expand Down
4 changes: 2 additions & 2 deletions vars/environ.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ def set_common_pr_production_environment() {
parse_scm_repo()
}

def set_tls_release_environment() {
def set_tls_release_environment(String target_repo) {
set_common_environment()
env.JOB_TYPE = 'release'
env.TARGET_REPO = 'tls'
env.TARGET_REPO = target_repo
env.CHECKOUT_METHOD = 'parametrized'
}

Expand Down
4 changes: 2 additions & 2 deletions vars/gen_jobs.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ git submodule foreach --recursive '
}

String script_in_docker = """
tests/scripts/list-identifiers.sh --internal
tests/scripts/list_internal_identifiers.py
# Workaround for abi_check.py failing to properly escape slashes in ref names
scripts/abi_check.py -o \$(git rev-parse 'origin/${env.CHANGE_TARGET}^{commit}') -n HEAD -s identifiers --brief
"""
Expand Down Expand Up @@ -686,7 +686,7 @@ def gen_release_jobs(BranchInfo info, boolean run_examples=true) {
})
}

if (info.repo == 'tls') {
if (info.repo == 'mbedtls') {
if (env.RUN_BASIC_BUILD_TEST == "true") {
jobs << gen_code_coverage_job(info, 'ubuntu-16.04-amd64');
}
Expand Down
15 changes: 13 additions & 2 deletions vars/mbedtls-pr-test-Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,18 @@
* This script takes the following parameters:
*
* Repos and branches
* - TARGET_REPO
* Tells the parametrized job to behave like a PR targeting the specified repo.
* At the moment, it affects two things in PR jobs:
* - TF-PSA-Crypto all.sh components are skipped if set to 'mbedtls'
* - API-ABI-Check selects which repo to use based on the TARGET_REPO setting.
* (selecting 'mbedtls-framework' disables the test)
* Possible values:
* - mbedtls
* - TF-PSA-Crypto
* - mbedtls-framework
* - CHANGE_TARGET
* a commit-ish (commit/branch/tag/etc.) in TARGET_REPO that API-ABI-Check compares against
* - MBED_TLS_REPO
* - MBED_TLS_BRANCH
* - FRAMEWORK_REPO
Expand All @@ -37,7 +48,6 @@
* - RUN_BASIC_BUILD_TEST
* - RUN_FREEBSD
* - RUN_WINDOWS_TEST
* - RUN_TF_PSA_CRYPTO_ALL_SH
*
* Other parameters
* - TEST_BRANCH
Expand All @@ -49,4 +59,5 @@

/* main job */
library identifier: 'mbedtls-test@master', retriever: legacySCM(scm)
mbedtls.run_pr_job('tls', false, env.MBED_TLS_BRANCH, env.RUN_TF_PSA_CRYPTO_ALL_SH == 'true' ? env.TF_PSA_CRYPTO_BRANCH : '')
String target_repo = env.TARGET_REPO ?: 'mbedtls'
mbedtls.run_pr_job(target_repo, false, env.MBED_TLS_BRANCH, target_repo == 'mbedtls' ? '' : env.TF_PSA_CRYPTO_BRANCH)
12 changes: 10 additions & 2 deletions vars/mbedtls-release-Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@
* This script takes the following parameters:
*
* Repos and branches
* - TARGET_REPO
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking it up in https://review.trustedfirmware.org/plugins/gitiles/ci/mbedtls/mbed-tls-job-configs/+/refs/heads/openci-migration/mbed-tls-restricted-pr-test-parametrized.yaml the possible values of TARGET_REPO are tls, tf-psa-crypto and framework. Can we make them consistently the actual repo names, or consistently short versions? Either mbedtls, TF-PSA-Crypto, mbedtls-framework or tls, crypto, framework.

And how do restricted repos fit into this?

Copy link
Contributor Author

@bensze01 bensze01 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make them consistently the actual repo names, or consistently short versions? Either mbedtls, TF-PSA-Crypto, mbedtls-framework or tls, crypto, framework.

I'd like to move us to using enums (perhaps in a separate PR), but unfortunately - is not valid in groovy identifiers.

Just to orient myself towards the eventual goal - would you prefer the style RepoType.crypto or RepoType.TF_PSA_CRYPTO?

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not something we'll have to type on a daily basis, so I'd prefer to just use the repository name.

I don't really mind if it's a string. We don't really need the whole Groovy code to know about all the possible repositories.

Copy link
Contributor

@minosgalanakis minosgalanakis Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make them consistently the actual repo names, or consistently short versions? Either mbedtls, TF-PSA-Crypto, mbedtls-framework or tls, crypto, framework.

I'd like to move us to using enums (perhaps in a separate PR), but unfortunately - is not valid in groovy identifiers.

Just to orient myself towards the eventual goal - would you prefer the style RepoType.crypto or RepoType.TF_PSA_CRYPTO?

I would advise against enums at this stage with groovy. I agree with Gilles to keep in String format.

Copy link
Contributor Author

@bensze01 bensze01 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And how do restricted repos fit into this?

TEST_BRANCH specifies the "type" of the repo, ie. the value of which variable we use out of MBED_TLS_REPO, TF_PSA_CRYPTO_REPO and FRAMEWORK_REPO.

I've detailed the exact effects it has at the moment in the Jenkinsfiles.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is new and currently only available in the mbed-tls-restricted-pr-test-parametrized job, from the openci-migration branch of the job configs which is still the one that the new CI uses.

Copy link
Contributor Author

@bensze01 bensze01 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to push a change to openci-migration anyways to adapt to the new values of TARGET_REPO - I'll just add it to the other parametrized jobs as well instead of documenting this as a limitation.

Copy link
Contributor Author

@bensze01 bensze01 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: https://review.trustedfirmware.org/c/ci/mbedtls/mbed-tls-job-configs/+/46425

* Tells the parametrized job to behave like a PR targeting the specified repo.
* At the moment, it affects one thing in release jobs:
* - TF-PSA-Crypto all.sh components are skipped if set to 'mbedtls'
* Possible values:
* - mbedtls
* - TF-PSA-Crypto
* - mbedtls-framework
* - MBED_TLS_REPO
* - MBED_TLS_BRANCH
* - FRAMEWORK_REPO
Expand All @@ -35,7 +43,6 @@
* - RUN_BASIC_BUILD_TEST
* - RUN_FREEBSD
* - RUN_WINDOWS_TEST
* - RUN_TF_PSA_CRYPTO_ALL_SH
*
* Other parameters
* - PUSH_COVERITY
Expand All @@ -49,4 +56,5 @@

/* main job */
library identifier: 'mbedtls-test@master', retriever: legacySCM(scm)
mbedtls.run_release_job(env.MBED_TLS_BRANCH, env.RUN_TF_PSA_CRYPTO_ALL_SH == 'true' ? env.TF_PSA_CRYPTO_BRANCH : '')
String target_repo = env.TARGET_REPO ?: 'mbedtls'
mbedtls.run_release_job(target_repo, env.MBED_TLS_BRANCH, target_repo == 'mbedtls' ? '' : env.TF_PSA_CRYPTO_BRANCH)
14 changes: 7 additions & 7 deletions vars/mbedtls.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ void run_tls_tests(Collection<BranchInfo> infos) {
infos.each { info ->
jobs << gen_jobs.gen_release_jobs(info, false)

if (env.RUN_ABI_CHECK == "true" && info.repo == 'tls') {
if (env.RUN_ABI_CHECK == 'true' && info.repo == env.TARGET_REPO) {
jobs << gen_jobs.gen_abi_api_checking_job(info, 'ubuntu-18.04-amd64')
}
}
Expand Down Expand Up @@ -153,29 +153,29 @@ void run_pr_job(String target_repo, boolean is_production, Collection<String> tl
/* main job */
void run_job() {
// CHANGE_BRANCH is not set in "branch" jobs, eg. in the merge queue
run_pr_job('tls', true, env.CHANGE_BRANCH ?: env.BRANCH_NAME, '')
run_pr_job('mbedtls', true, env.CHANGE_BRANCH ?: env.BRANCH_NAME, '')
}

void run_framework_pr_job() {
environ.parse_scm_repo()
run_pr_job(
'framework', true,
'mbedtls-framework', true,
env.IS_RESTRICTED ? ['development-restricted', 'mbedtls-3.6-restricted'] : ['development', 'mbedtls-3.6'],
env.IS_RESTRICTED ? ['development-restricted'] : ['development']
)
}

void run_release_job(String tls_branches, String tf_psa_crypto_branches) {
void run_release_job(String target_repo, String tls_branches, String tf_psa_crypto_branches) {
def (tls_split, tf_psa_crypto_split) =
[tls_branches, tf_psa_crypto_branches].collect({branches -> branches.split(',').findAll()})
Copy link
Contributor

@minosgalanakis minosgalanakis Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker but would it make sense to filter out whitespace?

def (tls_split, tf_psa_crypto_split) = [tls_branches, tf_psa_crypto_branches].collect { s -> (s ?: '').split(/\s*,\s*/).findAll { it } }

run_release_job(tls_split, tf_psa_crypto_split)
run_release_job(target_repo, tls_split, tf_psa_crypto_split)
}

void run_release_job(Collection<String> tls_branches, Collection<String> tf_psa_crypto_branches) {
void run_release_job(String target_repo, Collection<String> tls_branches, Collection<String> tf_psa_crypto_branches) {
analysis.main_record_timestamps('run_release_job') {
List<BranchInfo> infos = []
try {
environ.set_tls_release_environment()
environ.set_tls_release_environment(target_repo)
common.init_docker_images()

stage('branch-info') {
Expand Down
2 changes: 1 addition & 1 deletion vars/tf_psa_crypto.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
void run_pr_job() {
environ.parse_scm_repo()
mbedtls.run_pr_job('tf-psa-crypto', true, env.IS_RESTRICTED ? 'development-restricted' : 'development', env.CHANGE_BRANCH ?: env.BRANCH_NAME)
mbedtls.run_pr_job('TF-PSA-Crypto', true, env.IS_RESTRICTED ? 'development-restricted' : 'development', env.CHANGE_BRANCH ?: env.BRANCH_NAME)
}