Migrate action to Typescript #36

edif2008 · 2023-04-10T10:51:51Z

This PR migrates most of the functionality of the GitHub Action to Typescript.

Key elements used in the migration

Element	Explanation
`op-js`	Typescript package that wraps 1Password CLI commands. It enables us to easily work with the output returned by the 1Password CLI.
`core.setOutput`	Make the secret available as a step's output (works with multiline secrets as well).
`core.exportVariable`	Make the secret available as an environment variable (works with multiline secrets as well).
`core.setSecret`	Ensures the secret is masked in logs.
`core.setPath`	Add the path to the CLI to be able to call it in further parts of the action, as well as other steps in the pipeline.

Migration process

The commits should show an overview of the steps of the migration. I will present here the details of the main migration step highlighted in 6c9a28c:

Functionality from `entrypoint.sh`	Functionality in `index.ts`
`unset_prev_secrets`	`unsetPrevious`
auth validation (no dedicated function)	`validateAuth`
`install_cli`	`installCLI`
`extract_secrets`	`loadSecrets`
`populating_secret`	`extractSecret`

Notes

installCLI in index.tx only executes the shell script that will perform the CLI installation. No real migration has been performed for this part of the action.

I've made lint to ignore the following lines:

Code	Lint rule	Reason
`!process.env[envConnectHost].startsWith("http://")`	`no-restricted-syntax`	Error message doesn't apply (`Use https instead of http in URLs`) since we perform a check for the prefix `http://`.
`const installCLI = async (): Promise<void> =>`	`@typescript-eslint/naming-convention`	We have an exception here with the `CLI` being all capitalized.
`expect(process.env[envConnectHost]).toBe("http://localhost:8080");`	`no-restricted-syntax`	Error message doesn't apply (`Use https instead of http in URLs`) since we perform a check on whether the prefix `http://` was properly appended.

I'm open for suggestions if there are better function names we can use for our new implementation.

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 conditional of appending `http://` to the Connect host.

This is a safer and nicer way to ensure the path to the CLI is included later in the pipeline (including this GitHub action).

This eliminates the duplication of version in the code

This shows better what constants we use and they will be later used in both code and tests.

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

src/utils.ts

src/utils.test.ts

src/index.ts

entrypoint.sh

config/jest.config.js

package.json

src/constants.ts

src/utils.ts

jodyheavener

Took a quick pass at this, I love what you've done here so great job! I'm in agreement with Dustin's comments as well.

.github/workflows/test.yml

entrypoint.sh

src/constants.ts

entrypoint.sh

src/utils.ts

src/index.ts

src/utils.ts

src/index.ts

src/utils.ts

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.

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

jodyheavener

Awesome, Eddy! This looks so good.

kylekurz · 2023-06-22T15:58:47Z

Any chance this can get merged and an update to this action released? We'd like to incorporate it into some of our pipelines, but would like to have it pull in a modern build instead of the 2.10 that is in the current "live" action.

Verion `0.1.9` of the `op-js` exports function `semverToInt`, therefore we no longer need to duplicate it in our code.

src/index.ts

- Add architectures for Linux runners. Fail if the architecture is not supported. - Fail if the runner’s operating system is not supported.

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.

`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.

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.

The code itself seems a bit confusing, therefore we add a comment explaining how it works.

commit 10ed075 Author: Eduard Filip <[email protected]> Date: Wed Jul 5 16:50:31 2023 +0100 Improve the shell script (#49) * Improve CLI installation script - Add additional architectures for Linux. - Stop the action if the runner is executed in an unsupported OS. - Fetch automatically the latest stable CLI version. * Switch to new syntax for setting step output. GitHub has deprecated the syntax we were using for setting a step’s output (https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/). Therefore, we’re switching to the new one. * Stop action if arch is unsupported for Linux runners. commit 539eaa6 Author: Eduard Filip <[email protected]> Date: Wed Jul 5 11:36:30 2023 +0100 Improve the repo’s README (#48) Use a new template for the README file to better present the content. commit 08315da Merge: d8ac5d7 0e91b4a Author: Eduard Filip <[email protected]> Date: Mon Jul 3 16:54:07 2023 +0100 Merge pull request #44 from settlemint/main feat: install the right op for arm on linux commit 0e91b4a Author: roderik.eth <[email protected]> Date: Thu May 18 21:48:54 2023 +0200 fix: arm uname commit 9c2d98e Author: roderik.eth <[email protected]> Date: Thu May 18 20:51:49 2023 +0200 feat: install the right op for arm on linux commit d8ac5d7 Merge: 8fd274c 15d95ae Author: volodymyrZotov <[email protected]> Date: Tue May 16 20:09:59 2023 +0300 Merge pull request #42 from 1Password/ruetz-service-accounts-exiting-beta Remove 'BETA' references from Service Accounts commit 15d95ae Author: Dustin Ruetz <[email protected]> Date: Tue May 9 14:23:45 2023 -0400 style: auto-formatting fixes via Prettier commit a48d1fc Author: Dustin Ruetz <[email protected]> Date: Tue May 9 14:21:13 2023 -0400 docs: remove 'BETA' references from Service Accounts commit 8fd274c Merge: 663ac22 f4303b2 Author: Eduard Filip <[email protected]> Date: Mon Apr 24 11:19:45 2023 +0200 Merge pull request #39 from 1Password/eddy/service-account-docs Add documentation related to Service Accounts (currently in beta) commit f4303b2 Author: Eddy Filip <[email protected]> Date: Mon Apr 24 11:17:52 2023 +0200 Improve wording commit f4a99d4 Author: Eddy Filip <[email protected]> Date: Thu Apr 20 20:50:59 2023 +0200 Make small edits commit 9c1afd6 Author: Eddy Filip <[email protected]> Date: Thu Apr 20 18:53:49 2023 +0200 Adjust action versions used in examples In this way we keep them relevant with the latest versions commit a02ee66 Author: Eddy Filip <[email protected]> Date: Thu Apr 20 18:50:07 2023 +0200 Add documentation for service accounts

Return early if no env vars with valid secret references are found

# Conflicts: # dist/index.js # package-lock.json # package.json

edif2008 added 3 commits April 10, 2023 10:58

Make function for executing script

94471ca

Migrate auth validation

c7236d5

edif2008 changed the title ~~Draft: Migrate action to Typescript~~ Migrate action to Typescript Apr 10, 2023

edif2008 marked this pull request as draft April 10, 2023 10:55

edif2008 added 10 commits April 10, 2023 14:11

Fix CLI installation process

c8ce875

Fix conditional of appending protocol

1cf5241

Fix conditional of appending `http://` to the Connect host.

Improve code

98fac4e

Update CLI version and improve script

4d1bf78

Use core.addPath

ade7b48

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

cdeff7c

This eliminates the duplication of version in the code

Update dependencies

01b4970

Upgrade to Typescript 5

e25f48a

Merge branch 'main' into eddy/migrate-action-ts

c625e2e

Prettify test.yml

e6d1e6d

edif2008 mentioned this pull request Apr 11, 2023

GitHub Actions set-output is deprecated and scheduled for removal #27

Closed

edif2008 added 8 commits April 11, 2023 20:13

Move constants to constants.ts

e57ec50

This shows better what constants we use and they will be later used in both code and tests.

Move 'validateAuth' to 'utils.ts'

06d0dba

Add validate auth tests

6e0c1c6

Extract functionality for extracting a secret

208d260

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

33c5cc6

Move 'unsetPrevious' to 'utils.ts'

aeb83b5

Add unit test pipeline

5d5973b

Add tests for 'unsetPrevious'

1db0fc1

edif2008 marked this pull request as ready for review April 12, 2023 12:52

dustin-ruetz requested changes Apr 13, 2023

View reviewed changes

jodyheavener reviewed Apr 13, 2023

View reviewed changes

dustin-ruetz reviewed Apr 14, 2023

View reviewed changes

src/utils.ts Show resolved Hide resolved

dustin-ruetz reviewed Apr 14, 2023

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

dustin-ruetz reviewed Apr 14, 2023

View reviewed changes

src/utils.ts Outdated Show resolved Hide resolved

edif2008 added 2 commits May 22, 2023 19:25

Fix CLI version

1cbdee3

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.

Update packages

e46d4d6

edif2008 mentioned this pull request May 24, 2023

Export semverToInt 1Password/op-js#126

Merged

edif2008 added 3 commits May 25, 2023 15:03

Improve code

3963458

Simplify code related to mocking

1dd370e

jodyheavener approved these changes May 25, 2023

View reviewed changes

dustin-ruetz approved these changes Jun 13, 2023

View reviewed changes

edif2008 added 2 commits July 4, 2023 12:31

Update packages

57ddba0

Use semverToInt from op-js

35650f8

Verion `0.1.9` of the `op-js` exports function `semverToInt`, therefore we no longer need to duplicate it in our code.

dustin-ruetz reviewed Jul 5, 2023

View reviewed changes

src/index.ts Show resolved Hide resolved

edif2008 and others added 11 commits July 6, 2023 10:14

Imporve CLI installation script

1b79bb1

- 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

3daee42

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

8c1801e

`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

27cd56a

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

2c980ca

The code itself seems a bit confusing, therefore we add a comment explaining how it works.

Build index.js

22a24ec

Merge branch 'main' into eddy/migrate-action-ts

37781db

Update packages

433acb3

test: assertions for loadSecrets function

6fea62b

Improve loadSecrets function

62959c4

Return early if no env vars with valid secret references are found

edif2008 force-pushed the eddy/migrate-action-ts branch from 6abf126 to 62959c4 Compare July 10, 2023 10:14

edif2008 added 3 commits February 19, 2024 17:12

Merge branch 'main' into eddy/migrate-action-ts

190efe7

# Conflicts: # dist/index.js # package-lock.json # package.json

Update dependencies

54bdc9a

Upgrade action to use Node20

b6d91a5

edif2008 merged commit 2792fed into main Feb 21, 2024
11 checks passed

edif2008 deleted the eddy/migrate-action-ts branch February 21, 2024 16:38

edif2008 mentioned this pull request Mar 20, 2024

Update action to use node20 #64

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate action to Typescript #36

Migrate action to Typescript #36

edif2008 commented Apr 10, 2023 •

edited

Loading

jodyheavener left a comment

jodyheavener left a comment

kylekurz commented Jun 22, 2023

Migrate action to Typescript #36

Migrate action to Typescript #36

Conversation

edif2008 commented Apr 10, 2023 • edited Loading

Key elements used in the migration

Migration process

Notes

jodyheavener left a comment

Choose a reason for hiding this comment

jodyheavener left a comment

Choose a reason for hiding this comment

kylekurz commented Jun 22, 2023

edif2008 commented Apr 10, 2023 •

edited

Loading