Skip to content

test(appstore): cover the release Nextcloud setup and validate the unsigned package - #9063

Open
maia-andre wants to merge 4 commits into
LibreSign:mainfrom
maia-andre:test/appstore-packaging-regression-8845
Open

maia-andre wants to merge 4 commits into
LibreSign:mainfrom
maia-andre:test/appstore-packaging-regression-8845

Conversation

@maia-andre

Copy link
Copy Markdown
Contributor

Resolves: #8845

📝 Summary

Adds a small, tested boundary around the release packaging, following the direction in #8845 (comment).

  • Makefile. The Nextcloud initialization block moved unchanged from appstore into the internal target _appstore-nextcloud-setup, which appstore calls at the same point. The Nextcloud location is now nextcloud_directory (default unchanged, ../nextcloud). The target is not listed in make help. The signing block, the staging and the tarball stay in appstore.
  • verify-appstore-package also checks the archive itself: it must be readable, contain only libresign/, contain every required path and none of the forbidden ones. The lists mirror package.required_paths / package.forbidden_paths in .nextcloud-release.yml. This also applies to nightly and official releases, which already run this target.
  • Bats tests in tests/ci/ run the real targets with occ, curl and openssl stubbed, in a temporary directory. No network, no Nextcloud, no real key, nothing written in the repository.
  • CI. release-metadata.yml runs the Bats tests and now also triggers on tests/ci/** and on its own file. The Playwright job, which already installs composer and npm dependencies and the submodule, builds the real unsigned package with make appstore verify-appstore-package right after the frontend build.

make appstore, make verify-appstore-package, build/artifacts/libresign.tar.gz, the signing order, the nightly template, its patch and the publishing action are unchanged.

Composition: Makefile +43/−20 (most of it the moved block), tests +212 (3 files), workflows +23/−1, docs +16. Suggested reading order: Makefile, tests/ci/helpers.bash, the two .bats files, workflows.

🧪 How to test

bats tests/ci
GITHUB_ACTIONS= make appstore verify-appstore-package   # real unsigned package

Validation evidence

  • Focused regression/acceptance test: bats tests/ci, 8/8 locally (Bats 1.11.1, GNU Make 4.4.1), also with GITHUB_ACTIONS=true and MAKEFLAGS exported and from another working directory.
    1. key present + GITHUB_ACTIONS=true → maintenance:install, --version, app:enable --force libresign, data dir and app link created
    2. key present, GITHUB_ACTIONS unset → skipped, no command run
    3. no key, GITHUB_ACTIONS=true → skipped
    4. occ maintenance:install fails → non-zero exit, app:enable never runs
    5. real make appstore → setup runs before libresign:install and integrity:sign-app
      6–8. unsigned package accepted; a package with src/ or without templates/ rejected
  • The original regression is detected. With [ "${GITHUB_ACTIONS:-}" = "true" ] restored temporarily, tests 1, 4 and 5 fail; with the current expression they pass. Removing the _appstore-nextcloud-setup call from appstore makes test 5 fail. Neither change is committed.
  • Real unsigned package built locally from the repository: passes verify-appstore-package; its top-level layout is identical to the published libresign-v15.0.8.tar.gz.
  • shellcheck -s bash on tests/ci, reuse lint.
  • CI reviewed. The Playwright packaging step only runs on CI.

⚙️ API / Back-end changes

  • Tests added (Bats)
  • Documentation updated (tests/README.md)

Notes

  • Fixture path mismatch, not changed here. appstore packages tests/php/fixtures/small_valid.pdf (also in the published v15.0.8 archive), but DevelopController reads tests/php/fixtures/pdfs/small_valid.pdf and .nextcloud-release.yml lists that path as required. I left that file out of the required list in verify-appstore-package so releases keep passing. Happy to open a separate issue.
  • Cost of the packaging check. Because it lives in the Playwright job, a PR that changes only the Makefile or .nextcloud-release.yml now runs the e2e job. It is the only existing job with all packaging inputs installed; the alternative would be a new dependency setup, which the issue asked to avoid.
  • Not verified: actionlint (could not run locally).

✅ Checklist

  • I have read and followed the contribution guide.
  • The diff does not contain unrelated refactors or generated changes.
  • Commits follow Conventional Commits where applicable.
  • All commits include the required DCO sign-off.

🤖 AI (if applicable)

  • The content of this PR was partially or fully generated using AI

The Nextcloud initialization that signs a release lived inside the
appstore recipe, so nothing could exercise it without running the whole
package and signing flow. That is how the ${GITHUB_ACTIONS:-} expansion
skipped the setup silently.

Move the block unchanged into _appstore-nextcloud-setup, called by
appstore at the same point, and read the Nextcloud location from
nextcloud_directory (default unchanged: ../nextcloud). The target is
internal and stays out of make help.

The Bats tests run the real target with occ, curl and openssl stubbed:
setup on GitHub Actions with the key, skip without GITHUB_ACTIONS or
without the key, stop when occ fails, and appstore reaching the setup
before signing.

Refs: LibreSign#8845
Signed-off-by: André Maia <andrefnkmm@gmail.com>
verify-appstore-package only looked at the staging directory. It now
also reads the tarball and fails when it cannot be read, when an entry
sits outside the app directory, when a required path is missing or when
a development path is shipped. The lists mirror package.required_paths
and package.forbidden_paths in .nextcloud-release.yml.

tests/php/fixtures/pdfs/small_valid.pdf is left out of the required
list: appstore packages it as tests/php/fixtures/small_valid.pdf, as in
the published v15.0.8 archive, so requiring the configured path would
fail every release.

Refs: LibreSign#8845
Signed-off-by: André Maia <andrefnkmm@gmail.com>
Release metadata runs the Bats tests in tests/ci, and also runs when
only those tests or the workflow change.

The Playwright job already installs the composer and npm dependencies
and the 3rdparty submodule, so it now builds the real unsigned package
with make appstore verify-appstore-package right after the frontend
build. The image has no make, so it is installed with the other system
packages, and the job also runs when the Makefile or
.nextcloud-release.yml change.

Refs: LibreSign#8845
Signed-off-by: André Maia <andrefnkmm@gmail.com>
Refs: LibreSign#8845
Signed-off-by: André Maia <andrefnkmm@gmail.com>

@YvesCesar YvesCesar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks again @maia-andre !

I commented on a review. Could you take a look at it?

Comment thread tests/ci/helpers.bash
# Makefile into it, so packaging runs without touching the repository. Prints
# the app directory.
make_app_fixture() {
local app="$BATS_TEST_TMPDIR/work/$APP_NAME"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests fail when the repository or the temp directory is reached through a symlink. make stores the physical path in $(CURDIR). REPO_ROOT (line 9) uses pwd, which returns the logical path, and make_app_fixture (line 71) builds its path from BATS_TEST_TMPDIR without resolving it. So the readlink asserts compare two different paths:

  • repository reached through a symlink: test 1 fails (readlink ... = "$REPO_ROOT")
  • TMPDIR behind a symlink, which is the default on macOS (/var -> /private/var): test 5 fails (readlink ... = "$app")

Suggestion: resolve the physical path in both places.

REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../.." && pwd -P)"

local app
app="$(cd "$BATS_TEST_TMPDIR" && pwd -P)/work/$APP_NAME"

With this change, all 8 tests pass in both cases.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: define a proper regression strategy for Makefile release packaging

2 participants