Skip to content
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.

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

Already on GitHub? Sign in to your account

CI: switch to GitHub Actions #226

Merged
merged 6 commits into from Jan 12, 2022
Merged

CI: switch to GitHub Actions #226

merged 6 commits into from Jan 12, 2022

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jan 6, 2022

Context

  • CI/Quality control

Summary

This PR can be summarized in the following changelog entry:

  • CI/QA is now run via GitHub Actions

Relevant technical choices:

馃憠馃徎 This PR will be easiest to review by reviewing each commit individually.

馃憠馃徎 This PR (ab)uses the feature/ branch prefix on purpose to test the deploy workflow. Once this PR has been reviewed and merged, the feature branch for this PR should be removed from the dist repo.

.gitattributes: update

This updates the .gitattributes files to:

  • ... be more in line with the same file in other repos format-wise.
  • ... contain an up-to-date list of files to export-ignore.

CI: switch to GitHub Actions - step 1: code style

This commit:

  • Adds a GH Actions workflow for the CI code style check and to validate the composer.json file.
  • Removes those actions from the .travis.yml configuration.

Notes:

  1. Builds will run on all pushes and on pull requests, except pushes to master.
  2. Builds can also be manually triggered.
    Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  3. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  4. Composer dependency downloads will be cached for faster builds using a predefined GH action specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the composer.json and a potential composer.lock hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
  5. The CS check will display the results in the actions script output log, as well as display any violations found inline in the GitHub code view using the cs2pr tool.

Differences with the Travis implementation:

  • This check will now run on ALL pushes and pulls (except pushes to master).
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
  • The composer validate command will now only be run against the PHP version used in the cs script (PHP 7.4), not against multiple PHP versions.

CI: switch to GitHub Actions - step 2: add security check

As this repo does not have a committed composer.lock file, we can just require the roave/security-advisories repository to handle the security check.

The CS, Lint and Test workflows all run a composer install, which will fail if with that package in place if it would be attempted to install an insecure dependency.

CI: switch to GitHub Actions - step 3: linting

This commit:

  • Adds a GH Actions workflow for the PHP lint CI check.
  • Removes all references to that action from the .travis.yml configuration, as well as does some additional clean up of parts of the Travis script which have now become unused.

Notes:

  1. Builds will run on:
    • Select pushes using branch filtering similar to before.
    • All pull requests.
    • When manually triggered.
      Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  3. The default ini settings used by the setup-php action are based on the php.ini-production configuration.
    This means that error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT, display_errors is set to Off and zend.assertions is set to -1 (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with zend.assertions=-1, error_reporting=-1 and display_errors=On to ensure all PHP notices are shown.
    Note: the defaults will be changed in the next major of setup-php to be based on the php.ini-develop configuration, but that may still be a while off.
    Refs:
  4. While the setup-php action offers the ability to install the PHAR file for Parallel Lint, I've elected not to use that option as it would mean that we would not be able to use the composer lint script in the workflow, which would mean that the CLI arguments would have to be duplicated between the composer.json file and the lint.yml file.
    IMO, that would make this a typical point of failure where updates would be done in one, but not the other.
    If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited.
  5. Composer dependency downloads will be cached for faster builds using a predefined GH action specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the composer.json and a potential composer.lock hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
  6. The Linting check will display the results inline in the GitHub code view using the cs2pr tool.

Differences with the Travis implementation:

  • There is a minor difference in the branch filtering being done for push events.
    Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though.
  • The tag-based branch filter has not been added as that filter was only intended for use with the deploy stage.
  • Linting will now also be executed against PHP 8.1 and 8.2 (nightly).
    Linting runs against PHP 8.2 will be "allowed to fail" for the time being.
    Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red x next to a PR.
    This is a known issue in GHA: Please support something like "allow-failure" for a given job聽actions/runner#2347
    There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.

CI: switch to GitHub Actions - step 4: PHP Unit tests

This commit:

  • Adds a GH Actions workflow for the PHPUnit CI check.
  • Removes all references to that check from the .travis.yml configuration.

Notes:

  1. Builds will run on:
    • Select pushes using branch filtering similar to before.
    • All pull requests.
    • When manually triggered.
      Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  3. The default ini settings used by the setup-php action are based on the php.ini-production configuration.
    This means that error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT, display_errors is set to Off and zend.assertions is set to -1 (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with zend.assertions=-1, error_reporting=-1anddisplay_errors=Onto ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major ofsetup-phpto be based on thephp.ini-develop` configuration, but that may still be a while off.
    Refs:
  4. While the setup-php action offers the ability to install the PHAR file for PHPUnit, I've elected not to use that option as we need to do a composer install anyway to get the other dependencies, like WP Test Utils.
  5. Composer dependency downloads will be cached for faster builds using a predefined GH action specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the composer.json and a potential composer.lock hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
  6. As the Duplicate Post plugin only contains BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on.
    However, as there is no committed composer.lock file and the composer.json file does not contain a platform - php configuration, just running a plain composer install will already take care of that.
    If, at some point in the future, the composer.lock file would be committed or a platform - php config would be added to the composer.json file, this will need revisiting.

Differences with the Travis implementation:

  • In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1.
    As PHP 8.1 has been released and the test run currently succeeds on PHP 8.1 without notices, this build will not be allowed to fail.

CI: switch to GitHub Actions - step 5: Deploy to dist

This commit:

  • Adds a GH Actions workflow for deploying to the dist repo
  • Removes the, now redundant, .travis.yml configuration.
  • Removes the, now redundant, config/travis/deploy_to_dist.sh script.
  • Removes the, now redundant, config/travis/deploy_keys/id_rsa_yoast_dist.enc file.
  • Updates the .gitattributes file to reflect the removal of these files.

Notes:

  1. Builds will run on:
    • Select pushes using branch filtering similar to before.
      Note: this branch has been set up to match one of the filters to allow for testing the deploy.
    • Pushed tags.
    • When manually triggered.
      Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  3. The workflow global env key contains information about the Yoast dist repo. This information is used as "variables" in select places in the rest of the script.
    Important: while the default branch for the development repo may be different, the DIST_DEFAULT_BRANCH env variable should reflect the default branch for the dist repo.
    Also note that the script presumes that the sister-repo in the Yoast-dist organisation will be called the same as the repo in the Yoast organisation. If ever this would not be the case, an additional env variable could be set here and used later on in the script to handle that.
  4. The deployment is split into two jobs:
    • A prepare job which prepares the artifacts to deploy on the dev repo and uploads it as a workflow artifact.
    • A deploy job which checks out the dist version of the repo, cleans out most existing files, downloads and unpacks the prepared artifact and then commits the changes before pushing the branch/tag to the dist repo.
      The artifact upload/download mechanism with two jobs is used for two reasons:
      1. The way the environment in GH Actions is set up, is not all too friendly to checking out two repos at the same time and then being able to commit to the correct repo. (in other words: it's more secure)
        By splitting the deployment into two steps and uploading the artifact in between, we can be sure the commit ends up in the correct repo.
      2. To allow for examining the files in the artifacts for debugging.
  5. Both jobs are explicitly prevented from being run on forks outside the Yoast organisation.
  6. Both jobs contain a number of steps containing debug information, just in case.
  7. In various places in both jobs, data from the original commit is used, such as the branch name, the committer and the commit message.
    This type of data should always be regarded as untrusted input and when these github.... variables are used directly within the run context, they can lead to script injection and unintended execution of commands.
    To mitigate the risk of these type of script injection attacks, untrusted data is first set as a step-specific interim environment variable and only after that the environment variable (not the github variables directly) is used in the run context.
    This complies with the current best practices regarding defending against these type of attacks as per January 2022.
    A warning to this effect is at the top of the workflow to try and prevent people trying to be clever and "simplifying" the script.
    For more information, see: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions
  8. The prepare job is explicitly set up to use PHP 5.6 for the Composer installs and Node 14 for yarn and grunt.
    • PHP 5.6 is used as that's the minimum supported PHP version for this repo and there is nothing preventing a successfull install on PHP 5.6.
    • Node 14 is used for consistency with other scripts which really need Node 14.
  9. The prepare job will do an explicit composer install ahead of creating the artifact.
    The grunt jobs actually run composer install multiple times, both with --no-dev and with --dev. By running the composer install ahead of the grunt job, we can use the ramsey/composer-install action to cache the downloads Composer needs, thus warming up the cache and speeding up the build.
  10. In the prepare job, the update of the version number in the source code will only be executed for tags, not for manually triggered deploys as that would lead to weird/undesired versioning.
  11. In the deploy job, the first two steps use some bash logic to create a short version of the commit sha and determine the branch which needs to be checked out and the commit title.
    While this type of logic, if simple, can oftentimes be used inline, it is often more descriptive to extract it to a separate step and this also prevents repetitive logic in multiple places. More complex logic will, in most cases, needs such an interim step setting a variable anyway.
  12. In the deploy job, a GitHub access token for the dist repo is used, which has been saved to the dev repos "secrets".
    This access token has been created with full access to the organisation and can commit to the dist repo.
    The use of GH access tokens and secrets this way is an advantage of using GH Actions and removes the need for creating an SSH key based on the config/travis/deploy_keys/id_rsa_yoast_dist.enc file as was previously done in Travis.
  13. In the deploy job, the commands which were previously executed in the config/travis/deploy_to_dist.sh script have been transformed into job steps.
  14. The commit for the deploy will be made by the "user" who created the tag.
  15. In contrast to Travis, the commit message, will no longer be the commit message of the commit which was pushed.
    Instead, the commit message will contain a range of information, including the commit message of the commit which was pushed, to be able to identify the commit and branch which triggered the deploy correctly.

Differences with the Travis implementation:

  • The develop branch is not included in the branch filtering as that branch doesn't exist for this repo.
  • Previously the deploy would run on the arbitrary PHP 7.2 version. This has been changed to the minimum supported PHP version for this plugin (PHP 5.6).

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

These workflows have been extensively tested already and need no further testing.

Aside from the builds run for this PR, you can find the results of various specific tests also on the "Actions" page.
For each job, it has been verified that the build(s) will actually show as failed when code has been altered to induce a fail condition.

This updates the `.gitattributes` files to:
* ... be more in line with the same file in other repos format-wise.
* ... contain an up-to-date list of files to `export-ignore`.
This commit:
* Adds a GH Actions workflow for the CI code style check and to validate the `composer.json` file.
* Removes those actions from the `.travis.yml` configuration.

Notes:
1. Builds will run on all pushes and on pull requests, except pushes to `master`.
2. Builds can also be manually triggered.
    Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
3. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
5. The CS check will display the results in the actions script output log, as well as display any violations found inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool.

 Differences with the Travis implementation:
* This check will now run on ALL pushes and pulls (except pushes to `master`).
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
* The `composer validate` command will now only be run against the PHP version used in the `cs` script (PHP 7.4), not against multiple PHP versions.
As this repo does not have a committed `composer.lock` file, we can just require the `roave/security-advisories` repository to handle the security check.

The CS, Lint and Test workflows all run a `composer install`, which will fail if with that package in place if it would be attempted to install an insecure dependency.
This commit:
* Adds a GH Actions workflow for the PHP lint CI check.
* Removes all references to that action from the `.travis.yml` configuration, as well as does some additional clean up of parts of the Travis script which have now become unused.

Notes:
1. Builds will run on:
    - Select pushes using branch filtering similar to before.
    - All pull requests.
    - When manually triggered.
        Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration.
    This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown.
    Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off.
    Refs:
    * shivammathur/setup-php#450
    * shivammathur/setup-php#469
4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file.
    IMO, that would make this a typical point of failure where updates would be done in one, but not the other.
    If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited.
5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool.

Differences with the Travis implementation:
* There is a minor difference in the branch filtering being done for `push` events.
    Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though.
* The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage.
* Linting will now also be executed against PHP 8.1 and 8.2 (nightly).
    Linting runs against PHP 8.2 will be "allowed to fail" for the time being.
    Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red `x` next to a PR.
    This is a known issue in GHA: https://github.com/actions/toolkit/issues/399
    There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
This commit:
* Adds a GH Actions workflow for the PHPUnit CI check.
* Removes all references to that check from the `.travis.yml` configuration.

Notes:
1. Builds will run on:
    - Select pushes using branch filtering similar to before.
    - All pull requests.
    - When manually triggered.
        Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration.
    This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown.
    Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off.
    Refs:
    * shivammathur/setup-php#450
    * shivammathur/setup-php#469
4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils.
5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
6. As the Duplicate Post plugin only contains BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on.
    However, as there is no committed `composer.lock` file and the `composer.json` file does not contain a `platform - php` configuration, just running a plain `composer install` will already take care of that.
    If, at some point in the future, the `composer.lock` file would be committed or a `platform - php` config would be added to the `composer.json` file, this will need revisiting.

Differences with the Travis implementation:
* In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1.
    As PHP 8.1 has been released and the test run currently succeeds on PHP 8.1 without notices, this build will not be allowed to fail.
This commit:
* Adds a GH Actions workflow for deploying to the dist repo
* Removes the, now redundant, `.travis.yml` configuration.
* Removes the, now redundant, `config/travis/deploy_to_dist.sh` script.
* Removes the, now redundant, `config/travis/deploy_keys/id_rsa_yoast_dist.enc` file.
* Updates the `.gitattributes` file to reflect the removal of these files.

Notes:
1. Builds will run on:
    - Select pushes using branch filtering similar to before.
        Note: this branch has been set up to match one of the filters to allow for testing the deploy.
    - Pushed tags.
    - When manually triggered.
        Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
3. The workflow global `env` key contains information about the Yoast dist repo. This information is used as "variables" in select places in the rest of the script.
    **Important**: while the _default branch_ for the _development_ repo may be different, the `DIST_DEFAULT_BRANCH` env variable should reflect the _default branch_ for the _dist_ repo.
    Also note that the script presumes that the sister-repo in the `Yoast-dist` organisation will be called the same as the repo in the `Yoast` organisation. If ever this would not be the case, an additional env variable could be set here and used later on in the script to handle that.
4. The deployment is split into two jobs:
    - A `prepare` job which prepares the artifacts to deploy on the dev repo and uploads it as a workflow artifact.
    - A `deploy` job which checks out the `dist` version of the repo, cleans out most existing files, downloads and unpacks the prepared artifact and then commits the changes before pushing the branch/tag to the `dist` repo.
        The artifact upload/download mechanism with two jobs is used for two reasons:
        1. The way the environment in GH Actions is set up, is not all too friendly to checking out two repos at the same time and then being able to commit to the correct repo. (in other words: it's more secure)
            By splitting the deployment into two steps and uploading the artifact in between, we can be sure the commit ends up in the correct repo.
        2. To allow for examining the files in the artifacts for debugging.
5. Both jobs are explicitly prevented from being run on forks outside the Yoast organisation.
6. Both jobs contain a number of steps containing debug information, just in case.
7. In various places in both jobs, data from the original commit is used, such as the branch name, the committer and the commit message.
    This type of data should always be regarded as **untrusted** input and when these `github....` variables are used directly within the `run` context, they can lead to script injection and unintended execution of commands.
    To mitigate the risk of these type of script injection attacks, untrusted data is first set as a step-specific interim environment variable and only after that the environment variable (not the github variables directly) is used in the `run` context.
    This complies with the current best practices regarding defending against these type of attacks as per January 2022.
    A warning to this effect is at the top of the workflow to try and prevent people trying to be clever and "simplifying" the script.
    For more information, see: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions
8. The `prepare` job is explicitly set up to use PHP 5.6 for the Composer installs and Node 14 for yarn and grunt.
    - PHP 5.6 is used as that's the minimum supported PHP version for this repo and there is nothing preventing a successfull install on PHP 5.6.
    - Node 14 is used for consistency with other scripts which really _need_ Node 14.
9. The `prepare` job will do an explicit `composer install` ahead of creating the artifact.
    The `grunt` jobs actually run `composer install` multiple times, both with `--no-dev` and with `--dev`. By running the `composer install` ahead of the `grunt` job, we can use the `ramsey/composer-install` action to cache the downloads Composer needs, thus warming up the cache and speeding up the build.
10. In the `prepare` job, the update of the version number in the source code will only be executed for tags, not for manually triggered deploys as that would lead to weird/undesired versioning.
11. In the `deploy` job, the first two steps use some bash logic to create a short version of the commit sha and determine the branch which needs to be checked out and the commit title.
    While this type of logic, if simple, _can_ oftentimes be used inline, it is often more descriptive to extract it to a separate step and this also prevents repetitive logic in multiple places. More complex logic will, in most cases, needs such an interim step setting a variable anyway.
12. In the `deploy` job, a GitHub access token for the `dist` repo is used, which has been saved to the `dev` repos "secrets".
    This access token has been created with full access to the organisation and can commit to the `dist` repo.
    The use of GH access tokens and secrets this way is an advantage of using GH Actions and removes the need for creating an SSH key based on the `config/travis/deploy_keys/id_rsa_yoast_dist.enc` file as was previously done in Travis.
13. In the `deploy` job, the commands which were previously executed in the `config/travis/deploy_to_dist.sh` script have been transformed into job steps.
14. The commit for the deploy will be made by the "user" who created the tag.
15. In contrast to Travis, the _commit message_, will no longer _be_ the commit message of the commit which was pushed.
    Instead, the commit message will contain a range of information, _including the commit message of the commit which was pushed_, to be able to identify the commit and branch which triggered the deploy correctly.

Differences with the Travis implementation:
* The `develop` branch is not included in the branch filtering as that branch doesn't exist for this repo.
* Previously the deploy would run on the arbitrary PHP 7.2 version. This has been changed to the minimum supported PHP version for this plugin (PHP 5.6).
@diedexx diedexx merged commit 185ec70 into trunk Jan 12, 2022
@diedexx diedexx deleted the feature/switch-to-ghactions branch January 12, 2022 11:19
@jrfnl jrfnl modified the milestones: 4.4, 4.5 Jan 12, 2022
@diedexx
Copy link
Member

diedexx commented Jan 12, 2022

I've deleted the feature/switch-to-ghactions branch from yoast-dist, as requested in the PR description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants