-
Notifications
You must be signed in to change notification settings - Fork 11
Description
Summary
The reusable workflows integration.yaml and the proposed e2e.yaml (#313) share ~90% of their step logic (services, PHP/Composer setup, Magento project creation, fixups, MySQL 5.7 downgrade, etc.). This duplication makes bugs harder to fix and increases maintenance burden as more workflow types are added.
We should extract the shared setup into a composite action (e.g. setup-magento-test-env) and have both workflows call it, fixing several long-standing issues along the way.
Issues to address during extraction
- Shell injection in
run:steps. Multiple steps interpolate${{ }}expressions directly into shell commands (e.g.composer create-project --repository-url="${{ inputs.magento_repository }}",echo ${{ steps.supported-version.outputs.matrix }}). These should useenv:blocks and reference shell variables instead, per GitHub's security hardening guide. - Unsigned MySQL 5.7 binary download. The MySQL 5.7 client downgrade step downloads
.debpackages viawget --quietwith no SHA256 checksum verification. Should pin expected hashes. - Internal actions pinned to
@main.mage-os/github-actions/get-magento-version@mainshould be pinned to a release tag or SHA.
Possible bugs
- Hardcoded artifact upload path. Both workflows use
path: /home/runner/work/infrastructure/magento2/dev/tests/…which is specific to a single repo and will be wrong for any external consumer. Should be derived frominputs.magento_directory. $GITHUB_WORKSPACEas input default. Workflow input defaults are literal strings, not shell-expanded.$GITHUB_WORKSPACEworks inrun:steps but would break if used in non-shell contexts (e.g.if:expressions or actionwith:blocks).- Missing
db-namesed replacement in e2e.yaml. The MySQL service createsmagento_e2e_testsbut the sed block doesn't update the database name ininstall-config-mysql.php.dist.
Documentation
- README examples reference
actions/checkout@v2— should be@v4. e2e-README.mddocumentstest_commanddefault as"../../../vendor/bin/phpunit"but the workflow defaults to"".
Proposed approach
-
Create a new composite action (e.g.
setup-magento-test-env/action.yml) that handles:- PHP + Composer setup
- Composer cache
- Magento project creation
- Version-specific fixups (Monolog, Dotmailer, Composer plugins, prestissimo)
- Package require + install
- Config sed replacements (parameterized for integration vs e2e paths)
- MySQL 5.7 client downgrade (with checksum)
-
Refactor
integration.yamlande2e.yamlto call the composite action, adding only their test-type-specific steps (phpunit vs Playwright, working directories, etc.). -
Fix all security and bug issues listed above in the shared action.
This also aligns with the direction discussed in #140 (reducing public workflow surface area).
Related
- ci(e2e): add Playwright-based E2E workflow for Mage-OS #313 (E2E workflow PR)
- [RFC] Removal of publicly-facing workflows #140 (RFC: Removal of publicly-facing workflows)