In #97 it was missed to adjust the reusable workflow to pull changes from the forked commit. Instead, now we pull from base repository, which doesn't contain the external contributor's changes.
I've also improved the way we reference the reusable workflow to ensure we're using a trusted reusable workflow that won't change often.
Currently an external contributor can't have the acceptance tests run on their PR because pull_request doesn't give access to the secrets needed for them.
Therefore, in this PR we create a new workflow that is identical to the one for existing acceptance tests, with the following differences:
This workflow can be triggered with the command /ok-to-test sha="<contributor's latest commit sha>" by one of this repo's maintainers.
After the acceptance tests finish, their result will be updated to the PR's list of checks.
This file contains the same acceptance test jobs with the following differences:
- They only run if the `ok-to-test` command triggered the workflow and a sha has been passed.
- They checkout from the external contributor's commit.
Lastly, this workflow contains an extra job which updates the status in the PR based on the jobs executed. The result of a job is the parent result of all the matrix variants executed as part of it.
This workflow is the acceptance tests executed based on the following inputs:
- secret references
- whether the secrets are provided as a step output or environment variables.
In a previous PR we used `branch` syntax to trigger the pipeline when a push on `main` was made. This was a mistake and `branches` is the correct syntax that achieves this.
In Oct 2023, @1password/front-end-style has been rewritten into 3 smaller packages:
- @1password/eslint-config
- @1password/prettier-config
- @1password/stylelint-config
These 3 new packages have the same configurations as the previous package, with the benefits of being up-to-date and better organized. In the case of this GitHub Action, we only need the first two. The last one is dedicated to CSS stylling, which is not used in this action. Therefore, we will replace the deprecated @1password/front-end-style with the following packages:
- @1password/eslint-config
- @1password/prettier-config
* Add lint in workflow
This will check for code formatting, as well as for any ES lint issues.
* Format code
run `npm run check:write`
* Run lint and fix errors
Run `npm run lint` and then fix the errors shown.
* Make function for executing script
* Migrate auth validation
* Migrate load secret functionality
We make use of the following in the migration:
- `op-js` package (make direct calls to the CLI and nicely get the output of the commands)
- `core.exportVariable` to nicely export a secret as an environment variable
- `core.setOutput` to nicely export a secret a the step’s output.
- `core.setSecret` to mask the value of the secret if logged on the action’s output.
Note: `core.exportVariable` and `core.setOutput` work with multiline secrets without any additional work on our side.
Also, we export the temporary path where the CLI is installed to make sure the `op-js` package can find it.
* Fix CLI installation process
* Fix conditional of appending protocol
Fix conditional of appending `http://` to the Connect host.
* Update CLI version and improve script
* Use core.addPath
This is a safer and nicer way to ensure the path to the CLI is included later in the pipeline (including this GitHub action).
* Use version from package.json
This eliminates the duplication of version in the code
* Upgrade to Typescript 5
* Prettify test.yml
* Move constants to constants.ts
This shows better what constants we use and they will be later used in both code and tests.
* Move 'validateAuth' to 'utils.ts'
* Add validate auth tests
* Extract functionality for extracting a secret
This will enable us to easily test the functionality of the action regarding the extraction of secret and how it provides it to the rest of the pipeline based on user's input
* Add tests for extracting secret
* Move 'unsetPrevious' to 'utils.ts'
* Add unit test pipeline
* Add tests for 'unsetPrevious'
* Improve disabling eslint rules
Disable the ES Lint rules only for the next line and add a comment explaining why it’s disabled.
* Improve code based on PR review feedback
This contains code improvements that were easy to address based on PR review feedback.
* Improve CLI installation functionality
Two key elements are improved:
- The action will now automatically fetch the latest stable version of the CLI. There’s no longer the need to hardcode the version and manually update it.
- The action will now perform a check if the CLI exists in the pipeline and install it if it’s not available.
* Simplify extractSecret functionality
Eliminate the nested conditionals to have a cleaner and more readable code.
* Fix CLI version
The curl would return the version number, but we forgot to append the `v` in the version (i.e. from 2.18.0 to v2.18.0). Now it should be fixed.
* Move loadSecrets function to utils.ts
This is done to keep things modular and narrow down the scope and complexity of index.ts.
`installCLI` will be kept in `index.ts` for the following reasons:
- Moving it to utils brings complications (`import.meta.url` doesn’t work)
- This code will be removed once the action will make use of the separate install CLI action
* Simplify code related to mocking
* Use semverToInt from op-js
Version `0.1.9` of the `op-js` exports function `semverToInt`, therefore we no longer need to duplicate it in our code.
* Improve CLI installation script
- Add architectures for Linux runners. Fail if the architecture is not supported.
- Fail if the runner’s operating system is not supported.
* Change from debug messages to info
In pre-TS GitHub Action, we’d print some messages to the output as info (e.g. authenticated as, populating variable, unsetting previous values). Therefore, we apply the same principle here since there’s useful info.
* use toHaveBeenCalled consistently in tests
`toBeCalled` is an alias for `toHaveBeenCalled` and `toBeCalledWith` is an alias for `toHaveBeenCalledWith`. For consistency, we will use `toHaveBeenCalled` and `toHaveBeenCalledWith` consistently across our tests.
* Add warning if both configs are provided
1Password CLI will prioritize Connect config (with `OP_CONNECT_HOST` and `OP_CONNECT_TOKEN`) over service account one (with `OP_SERVICE_ACCOUNT_TOKEN`). This shouldn’t happen, therefore we print a warning to the user if both are provided.
* Add comment about cli validation process
The code itself seems a bit confusing, therefore we add a comment explaining how it works.
* test: assertions for loadSecrets function
* Improve loadSecrets function
Return early if no env vars with valid secret references are found
* Update dependencies
* Upgrade action to use Node20
---------
Co-authored-by: Dustin Ruetz <dustin.ruetz@agilebits.com>
