Compare commits

..
Author SHA1 Message Date
github-actions[bot] fe516562da Update CHANGELOG.md 2026-11-10 18:36:46 +00:00
Rögl-Brunner Michel e1dd96614d pr-script-test: approve migrations only when they change nothing else
Anyone with write access can push to a push-app-to-main branch, and the
bot's approval counts toward the main ruleset's required review. The
approval now only covers what a migration contains: if the PR changes
anything besides ct/*.sh and install/*.sh (workflows, misc/, ...), it is
tested as usual but not approved, and the result comment says why.
2026-11-10 16:34:29 +01:00
Rögl-Brunner Michel cfada6ed6a pr-script-test: run on DevScripts migrations by itself, approve passes
PRs that the push-app-to-main app opens on a branch of this repository
(scripts moving over from DevScripts, reviewed and tested there,
writable only by writers) are now built on pull_request_target for
every push, pinned to the pushed commit; an event for an older push is
ignored. When every script passes, the bot approves the tested commit,
which counts toward the main ruleset's required review; a new push
drops it as stale, and a later run that does not fully pass withdraws
it. Untestable (CPU) and setup errors are never approved. All other
PRs still need /test and are never approved.
2026-11-10 16:32:40 +01:00
Rögl-Brunner Michel cc27caf470 pr-script-test: say why nothing was built
A /test on a PR that only changes vm/ (#18578, TrueNAS) found nothing
to build and stopped without a word. The gate now always answers:
"Nothing to build" with the reason for each file (vm/ not covered,
tools/ not built, install/ script without a ct/ script of the same
name, more than 10 scripts), and when some scripts are built the rest
are listed as "Not built" in the progress and the result comment.
File names from the PR are put in code spans with backticks and line
breaks removed.
2026-11-10 16:24:59 +01:00
community-scripts-pr-app[bot]andgithub-actions[bot] dc9d75a5ad Update CHANGELOG.md (#18597)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-11-10 15:22:34 +00:00
Michel Roegl-Brunner 79481f3a74 pr-script-test: pre-answer prompt variables built in a loop (#18596)
OpenCloud reads var_opencloud_${server}_fqdn for each server in
servers=("opencloud" "collabora" "wopi"), so the scan for literal
var_ names found nothing and the build stopped at the first prompt
(#18586). Templates of the form var_...${x}... are now expanded with
the words of `for x in ...`, or of the array that loop walks; nothing
is executed. Hostname-like answers are derived from the variable name
(ci-opencloud-collabora.example.test), so several cannot collide.
2026-11-10 16:22:06 +01:00
2 changed files with 111 additions and 13 deletions

No files matched your search

+107 -11
View File
@@ -5,11 +5,15 @@ name: PR script test
# date.
#
# A self-hosted runner that executes pull request code as root is only
# acceptable if a human decides what runs on it. Nothing here starts on a push:
# someone with write access comments `/test` (optionally followed by a full SHA
# to pin, and the ct/ paths to build). Only a commit that existed when the
# comment was written gets built: a push after it is never covered, not even
# one that lands in the seconds before this job runs.
# acceptable if a human decides what runs on it. So it starts on a push only
# for trusted PRs: those the push-app-to-main app opens on a branch of this
# repository when a script moves over from DevScripts (already reviewed and
# tested there; only writers can push to such a branch). Those are built
# automatically and approved when every script passes. Everything else waits
# for someone with write access to comment `/test` (optionally followed by a
# full SHA to pin, and the ct/ paths to build). Only a commit that existed when
# the comment was written gets built: a push after it is never covered, not
# even one that lands in the seconds before this job runs.
#
# gate ubuntu-latest who asked, which scripts, which commit
# test proxmox-test build + checks, one matrix job per script/OS
@@ -24,6 +28,13 @@ name: PR script test
on:
issue_comment:
types: [created]
# Trusted PRs only, see above. pull_request_target runs this file from main,
# never the PR's copy of it.
pull_request_target:
types: [opened, reopened, synchronize, ready_for_review]
paths:
- "ct/**"
- "install/**"
workflow_dispatch:
inputs:
pr:
@@ -45,7 +56,10 @@ jobs:
gate:
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.issue.pull_request && startsWith(github.event.comment.body, '/test'))
(github.event_name == 'issue_comment' && github.event.issue.pull_request && startsWith(github.event.comment.body, '/test')) ||
(github.event_name == 'pull_request_target' && !github.event.pull_request.draft &&
github.event.pull_request.user.login == 'push-app-to-main[bot]' &&
github.event.pull_request.head.repo.full_name == github.repository)
runs-on: ubuntu-latest
permissions:
actions: read # when the run was dispatched, for a /test without SHA
@@ -60,6 +74,8 @@ jobs:
sha: ${{ steps.plan.outputs.sha }}
check_id: ${{ steps.plan.outputs.check_id }}
actor: ${{ steps.plan.outputs.actor }}
not_built: ${{ steps.plan.outputs.not_built }}
trusted: ${{ steps.plan.outputs.trusted }}
steps:
- id: plan
uses: actions/github-script@v8
@@ -69,8 +85,20 @@ jobs:
const MAX = 10;
core.setOutput('matrix', '[]');
// Trusted: a PR the push-app-to-main app opened on a branch of this
// repository, i.e. a script moving over from DevScripts.
const TRUSTED_AUTHOR = 'push-app-to-main[bot]';
const isTrusted = p => !!p && p.user && p.user.login === TRUSTED_AUTHOR && p.user.type === 'Bot' &&
!!p.head.repo && p.head.repo.full_name === `${owner}/${repo}`;
let prNumber, requested = '';
if (context.eventName === 'workflow_dispatch') {
if (context.eventName === 'pull_request_target') {
const p = context.payload.pull_request;
if (!isTrusted(p)) return;
prNumber = p.number;
// Exactly the pushed commit; a later push starts its own run.
requested = p.head.sha;
} else if (context.eventName === 'workflow_dispatch') {
prNumber = Number(context.payload.inputs.pr);
requested = `${context.payload.inputs.sha || ''} ${context.payload.inputs.scripts || ''}`;
} else {
@@ -93,6 +121,11 @@ jobs:
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
if (pr.state !== 'open' || !pr.head.repo) { core.notice('PR is closed or its fork is gone'); return; }
const head = pr.head.repo;
if (context.eventName === 'pull_request_target' && pr.head.sha !== requested) {
core.notice('Pushed to again since; the newer push has its own run');
return;
}
core.setOutput('trusted', isTrusted(pr) ? 'true' : 'false');
const MARK = '<!-- pr-script-test -->';
const sticky = async body => {
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: prNumber });
@@ -150,6 +183,10 @@ jobs:
core.setFailed(`Only ct/<name>.sh paths can be requested, got: ${requested}`);
return;
}
// Everything the PR changes that this test cannot build is named in
// the comment, so a /test never ends in silence.
const notBuilt = [];
const code = s => '`' + String(s).replace(/[`\r\n]/g, '') + '`';
if (entries.length === 0) {
const files = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: prNumber });
const set = new Set();
@@ -158,6 +195,8 @@ jobs:
let m;
if ((m = f.filename.match(/^install\/([A-Za-z0-9._-]+)-install\.sh$/))) set.add(`ct/${m[1]}.sh`);
else if (/^ct\/[A-Za-z0-9._-]+\.sh$/.test(f.filename)) set.add(f.filename);
else if (/^vm\/.*\.sh$/.test(f.filename)) notBuilt.push(`${code(f.filename)}: \`vm/\` scripts are not covered`);
else if (/^(tools|turnkey)\/.*\.sh$/.test(f.filename)) notBuilt.push(`${code(f.filename)}: only \`ct/\` scripts are built`);
}
entries = [...set].sort();
}
@@ -168,14 +207,25 @@ jobs:
try {
const res = await github.rest.repos.getContent({ owner: head.owner.login, repo: head.name, path: script, ref: pr.head.sha });
text = Buffer.from(res.data.content || '', 'base64').toString('utf8');
} catch (e) { continue; } // install script without a ct/ entry point
} catch (e) {
notBuilt.push(`\`${script}\` does not exist in this PR (an \`install/\` script is built through the \`ct/\` script of the same name)`);
continue;
}
// Scripts that ask for the OS get built once per OS.
const oses = /msg_menu\s+"Choose the container OS"/.test(text) ? ['debian', 'alpine'] : [''];
const name = script.slice(3, -3);
for (const os of oses) include.push({ script, os, name: os ? `${name}-${os}` : name });
}
if (entries.length > MAX) core.warning(`Only the first ${MAX} of ${entries.length} scripts are built`);
if (include.length === 0) { core.notice('No ct/ script to build'); return; }
for (const script of entries.slice(MAX)) notBuilt.push(`\`${script}\`: at most ${MAX} scripts per run, name it with \`/test ${script}\``);
if (include.length === 0) {
await sticky([
MARK, '### Proxmox script test', '',
'ℹ️ Nothing to build: this test builds `ct/` scripts (and changed `install/` scripts through their `ct/` script).', '',
...(notBuilt.length ? notBuilt.map(n => `- ${n}`) : ['- this PR changes no `ct/` or `install/` script']),
].join('\n'));
core.notice('No ct/ script to build');
return;
}
const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`;
const { data: check } = await github.rest.checks.create({
@@ -189,6 +239,7 @@ jobs:
core.setOutput('repo', head.full_name);
core.setOutput('sha', pr.head.sha);
core.setOutput('check_id', String(check.id));
core.setOutput('not_built', JSON.stringify(notBuilt));
// Who asked: the /test commenter, or whoever dispatched the run.
core.setOutput('actor', context.actor);
@@ -196,6 +247,7 @@ jobs:
MARK, '### Proxmox script test', '',
`⏳ Building ${include.length} at ${pr.head.sha.slice(0, 7)} — [watch it live](${runUrl}) (open the \`test\` job)`, '',
...include.map(i => `- \`${i.script}\`${i.os ? ` (${i.os})` : ''}`),
...(notBuilt.length ? ['', '**Not built:**', ...notBuilt.map(n => `- ${n}`)] : []),
].join('\n'));
test:
@@ -262,6 +314,8 @@ jobs:
CHECK_ID: ${{ needs.gate.outputs.check_id }}
TEST_RESULT: ${{ needs.test.result }}
ACTOR: ${{ needs.gate.outputs.actor }}
NOT_BUILT: ${{ needs.gate.outputs.not_built }}
TRUSTED: ${{ needs.gate.outputs.trusted }}
with:
script: |
const fs = require('fs');
@@ -415,6 +469,7 @@ jobs:
`${headline} at \`${process.env.SHA.slice(0, 7)}\``, '',
table, '',
...shots, '',
...(() => { try { const n = JSON.parse(process.env.NOT_BUILT || '[]'); return n.length ? ['**Not built:**', ...n.map(x => `- ${x}`), ''] : []; } catch (e) { return []; } })(),
...logs, '',
`Full logs: [workflow run](${runUrl}) (artifacts \`result-*\`). Comment \`/test\` to build again.`,
].join('\n');
@@ -452,14 +507,55 @@ jobs:
const REVIEW_MARK = '<!-- pr-script-test-review -->';
const failedRows = cancelled ? [] : rows.filter(r => r.status === 'fail');
const sha7 = process.env.SHA.slice(0, 7);
// Trusted PRs (scripts moving over from DevScripts) are approved when
// every script passes -- not on untestable or setup trouble, which
// means "not tested", not "good". That approval counts toward the
// ruleset's required review; a push dismisses it as stale, and a
// later run that does not fully pass withdraws it.
// Anyone with write access can push to a migration branch, and the
// approval counts toward the required review. So it only covers
// what a migration contains: ct/ and install/ scripts. A PR that
// also changes anything else (workflows, misc/, ...) is tested but
// never approved.
let approve = process.env.TRUSTED === 'true' && allPass;
let noApproveWhy = '';
if (approve) {
const changed = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr });
const other = changed.map(f => f.filename).filter(n => !/^(ct|install)\/[A-Za-z0-9._-]+\.sh$/.test(n));
if (other.length) {
approve = false;
noApproveWhy = `Not approved automatically: besides \`ct/\` and \`install/\` scripts this PR changes ${other.slice(0, 5).map(n => '`' + n.replace(/[`\r\n]/g, '') + '`').join(', ')}${other.length > 5 ? ', …' : ''}. That needs a human review.`;
}
}
if (noApproveWhy) {
await github.rest.issues.updateComment({ owner, repo, comment_id: posted.id, body: `${body}\n\n> ${noApproveWhy}`.slice(0, 65000) });
}
try {
const reviews = await github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number: pr });
for (const v of reviews.filter(v => v.user && v.user.type === 'Bot' && v.state === 'CHANGES_REQUESTED' && (v.body || '').includes(REVIEW_MARK))) {
const ours = state => reviews.filter(v => v.user && v.user.type === 'Bot' && v.state === state && (v.body || '').includes(REVIEW_MARK));
for (const v of ours('CHANGES_REQUESTED')) {
await github.rest.pulls.dismissReview({
owner, repo, pull_number: pr, review_id: v.id,
message: failedRows.length ? 'Replaced by a newer Proxmox script test.' : `Proxmox script test passed at ${sha7}.`,
});
}
if (!approve && !cancelled) {
for (const v of ours('APPROVED')) {
await github.rest.pulls.dismissReview({
owner, repo, pull_number: pr, review_id: v.id,
message: `Withdrawn: the Proxmox script test at ${sha7} did not fully pass.`,
});
}
}
if (approve && !ours('APPROVED').some(v => v.commit_id === process.env.SHA)) {
await github.rest.pulls.createReview({
owner, repo, pull_number: pr, commit_id: process.env.SHA, event: 'APPROVE',
body: [
REVIEW_MARK, `**Proxmox script test passed** at \`${sha7}\`: ${rows.map(r => `\`${r.script}\``).join(', ')}.`, '',
`Script moved over from DevScripts, built and checked on a real Proxmox host. [Test result](${posted.html_url}).`,
].join('\n'),
});
}
if (failedRows.length) {
await github.rest.pulls.createReview({
owner, repo, pull_number: pr, commit_id: process.env.SHA, event: 'REQUEST_CHANGES',
+4 -2
View File
@@ -594,11 +594,13 @@ Exercise vigilance regarding copycat or coat-tailing sites that seek to exploit
### 💾 Core
- fix: flush the image before importing a VM disk [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([core#223](https://github.com/community-scripts/core/pull/223))
- fix: keep a script's config file when a package installs it [@MickLesk](https://github.com/MickLesk) ([core#226](https://github.com/community-scripts/core/pull/226))
- fix: flush the image before importing a VM disk [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([core#223](https://github.com/community-scripts/core/pull/223))
### 📂 Github
- Build PR scripts on a Proxmox test host via /test [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([#18594](https://github.com/community-scripts/ProxmoxVE/pull/18594))
- pr-script-test: pre-answer prompt variables built in a loop [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([#18596](https://github.com/community-scripts/ProxmoxVE/pull/18596))
- Build PR scripts on a Proxmox test host via /test [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([#18594](https://github.com/community-scripts/ProxmoxVE/pull/18594))
## 2026-11-09