Skip to content

Commit

Permalink
feat: use dash notation for inputs (deprecates underscore notation) (a…
Browse files Browse the repository at this point in the history
…ctions#59)

Fixes actions#57 

This PR implements the 3-step plan proposed by @gr2m in
actions#57 (comment):

> 1. Support both input types
> 2. Log a deprecation warning for the old notation
> 3. Add a test for deprecations

Although this PR supports both input formats simultaneously, I opted
_not_ to document the old format in the updated README. That’s a
decision I’m happy to revisit, if y’all would prefer to have
documentation for both the old and new formats.
  • Loading branch information
smockle committed Oct 6, 2023
1 parent bdb2377 commit 7b1d2ae
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 33 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ jobs:
- uses: actions/create-github-app-token@v1
id: app-token
with:
app_id: ${{ vars.RELEASER_APP_ID }}
private_key: ${{ secrets.RELEASER_APP_PRIVATE_KEY }}
app-id: ${{ vars.RELEASER_APP_ID }}
private-key: ${{ secrets.RELEASER_APP_PRIVATE_KEY }}
- uses: actions/checkout@v4
with:
token: ${{ steps.app-token.outputs.token }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ jobs:
- uses: ./ # Uses the action in the root directory
id: test
with:
app_id: ${{ vars.TEST_APP_ID }}
private_key: ${{ secrets.TEST_APP_PRIVATE_KEY }}
app-id: ${{ vars.TEST_APP_ID }}
private-key: ${{ secrets.TEST_APP_PRIVATE_KEY }}
- uses: octokit/[email protected]
id: get-repository
env:
Expand Down
26 changes: 13 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ jobs:
- uses: actions/create-github-app-token@v1
id: app-token
with:
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.PRIVATE_KEY }}
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.PRIVATE_KEY }}
- uses: peter-evans/create-or-update-comment@v3
with:
token: ${{ steps.app-token.outputs.token }}
Expand All @@ -44,8 +44,8 @@ jobs:
id: app-token
with:
# required
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.PRIVATE_KEY }}
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.PRIVATE_KEY }}
- uses: actions/checkout@v3
with:
token: ${{ steps.app-token.outputs.token }}
Expand All @@ -69,8 +69,8 @@ jobs:
- uses: actions/create-github-app-token@v1
id: app-token
with:
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.PRIVATE_KEY }}
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.PRIVATE_KEY }}
owner: ${{ github.repository_owner }}
- uses: peter-evans/create-or-update-comment@v3
with:
Expand All @@ -91,8 +91,8 @@ jobs:
- uses: actions/create-github-app-token@v1
id: app-token
with:
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.PRIVATE_KEY }}
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.PRIVATE_KEY }}
owner: ${{ github.repository_owner }}
repositories: "repo1,repo2"
- uses: peter-evans/create-or-update-comment@v3
Expand All @@ -114,8 +114,8 @@ jobs:
- uses: actions/create-github-app-token@v1
id: app-token
with:
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.PRIVATE_KEY }}
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.PRIVATE_KEY }}
owner: another-owner
- uses: peter-evans/create-or-update-comment@v3
with:
Expand All @@ -126,11 +126,11 @@ jobs:

## Inputs

### `app_id`
### `app-id`

**Required:** GitHub App ID.

### `private_key`
### `private-key`

**Required:** GitHub App private key.

Expand All @@ -145,7 +145,7 @@ jobs:
> [!NOTE]
> If `owner` is set and `repositories` is empty, access will be scoped to all repositories in the provided repository owner's installation. If `owner` and `repositories` are empty, access will be scoped to only the current repository.

### `skip_token_revoke`
### `skip-token-revoke`

**Optional:** If truthy, the token will not be revoked when the current job is complete.

Expand Down
16 changes: 14 additions & 2 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,33 @@ branding:
icon: "lock"
color: "gray-dark"
inputs:
app-id:
description: "GitHub App ID"
required: false # TODO: When 'app_id' is removed, make 'app-id' required
app_id:
description: "GitHub App ID"
required: true
required: false
deprecationMessage: "'app_id' is deprecated and will be removed in a future version. Use 'app-id' instead."
private-key:
description: "GitHub App private key"
required: false # TODO: When 'private_key' is removed, make 'private-key' required
private_key:
description: "GitHub App private key"
required: true
required: false
deprecationMessage: "'private_key' is deprecated and will be removed in a future version. Use 'private-key' instead."
owner:
description: "GitHub App owner (defaults to current repository owner)"
required: false
repositories:
description: "Repositories to install the GitHub App on (defaults to current repository if owner is unset)"
required: false
skip-token-revoke:
description: "If truthy, the token will not be revoked when the current job is complete"
required: false
skip_token_revoke:
description: "If truthy, the token will not be revoked when the current job is complete"
required: false
deprecationMessage: "'skip_token_revoke' is deprecated and will be removed in a future version. Use 'skip-token-revoke' instead."
outputs:
token:
description: "GitHub installation access token"
Expand Down
14 changes: 11 additions & 3 deletions dist/main.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10103,11 +10103,19 @@ if (!process.env.GITHUB_REPOSITORY) {
if (!process.env.GITHUB_REPOSITORY_OWNER) {
throw new Error("GITHUB_REPOSITORY_OWNER missing, must be set to '<owner>'");
}
var appId = import_core.default.getInput("app_id");
var privateKey = import_core.default.getInput("private_key");
var appId = import_core.default.getInput("app-id") || import_core.default.getInput("app_id");
if (!appId) {
throw new Error("Input required and not supplied: app-id");
}
var privateKey = import_core.default.getInput("private-key") || import_core.default.getInput("private_key");
if (!privateKey) {
throw new Error("Input required and not supplied: private-key");
}
var owner = import_core.default.getInput("owner");
var repositories = import_core.default.getInput("repositories");
var skipTokenRevoke = Boolean(import_core.default.getInput("skip_token_revoke"));
var skipTokenRevoke = Boolean(
import_core.default.getInput("skip-token-revoke") || import_core.default.getInput("skip_token_revoke")
);
main(
appId,
privateKey,
Expand Down
4 changes: 3 additions & 1 deletion dist/post.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2973,7 +2973,9 @@ var import_core = __toESM(require_core(), 1);

// lib/post.js
async function post(core2, request2) {
const skipTokenRevoke = Boolean(core2.getInput("skip_token_revoke"));
const skipTokenRevoke = Boolean(
core2.getInput("skip-token-revoke") || core2.getInput("skip_token_revoke")
);
if (skipTokenRevoke) {
core2.info("Token revocation was skipped");
return;
Expand Down
4 changes: 3 additions & 1 deletion lib/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* @param {import("@octokit/request").request} request
*/
export async function post(core, request) {
const skipTokenRevoke = Boolean(core.getInput("skip_token_revoke"));
const skipTokenRevoke = Boolean(
core.getInput("skip-token-revoke") || core.getInput("skip_token_revoke")
);

if (skipTokenRevoke) {
core.info("Token revocation was skipped");
Expand Down
16 changes: 13 additions & 3 deletions main.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,22 @@ if (!process.env.GITHUB_REPOSITORY_OWNER) {
throw new Error("GITHUB_REPOSITORY_OWNER missing, must be set to '<owner>'");
}

const appId = core.getInput("app_id");
const privateKey = core.getInput("private_key");
const appId = core.getInput("app-id") || core.getInput("app_id");
if (!appId) {
// The 'app_id' input was previously required, but it and 'app-id' are both optional now, until the former is removed. Still, we want to ensure that at least one of them is set.
throw new Error("Input required and not supplied: app-id");
}
const privateKey = core.getInput("private-key") || core.getInput("private_key");
if (!privateKey) {
// The 'private_key' input was previously required, but it and 'private-key' are both optional now, until the former is removed. Still, we want to ensure that at least one of them is set.
throw new Error("Input required and not supplied: private-key");
}
const owner = core.getInput("owner");
const repositories = core.getInput("repositories");

const skipTokenRevoke = Boolean(core.getInput("skip_token_revoke"));
const skipTokenRevoke = Boolean(
core.getInput("skip-token-revoke") || core.getInput("skip_token_revoke")
);

main(
appId,
Expand Down
12 changes: 11 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"esbuild": "^0.19.4",
"execa": "^8.0.1",
"open-cli": "^7.2.0",
"undici": "^5.25.2"
"undici": "^5.25.2",
"yaml": "^2.3.2"
},
"release": {
"branches": [
Expand All @@ -48,4 +49,4 @@
]
]
}
}
}
16 changes: 16 additions & 0 deletions tests/action-deprecated-inputs.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { readFileSync } from "node:fs";
import * as url from "node:url";
import YAML from "yaml";

const action = YAML.parse(
readFileSync(
url.fileURLToPath(new URL("../action.yml", import.meta.url)),
"utf8"
)
);

for (const [key, value] of Object.entries(action.inputs)) {
if ("deprecationMessage" in value) {
console.log(`${key}${value.deprecationMessage}`);
}
}
9 changes: 9 additions & 0 deletions tests/main-missing-app-id.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
process.env.GITHUB_REPOSITORY_OWNER = "actions";
process.env.GITHUB_REPOSITORY = "actions/create-github-app-token";

// Verify `main` exits with an error when neither the `app-id` nor `app_id` input is set.
try {
await import("../main.js");
} catch (error) {
console.error(error.message);
}
10 changes: 10 additions & 0 deletions tests/main-missing-private-key.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
process.env.GITHUB_REPOSITORY_OWNER = "actions";
process.env.GITHUB_REPOSITORY = "actions/create-github-app-token";
process.env["INPUT_APP-ID"] = "123456";

// Verify `main` exits with an error when neither the `private-key` nor `private_key` input is set.
try {
await import("../main.js");
} catch (error) {
console.error(error.message);
}
4 changes: 2 additions & 2 deletions tests/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ export async function test(cb = (_mockPool) => {}) {
process.env.GITHUB_REPOSITORY = "actions/create-github-app-token";
// inputs are set as environment variables with the prefix INPUT_
// https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#example-specifying-inputs
process.env.INPUT_APP_ID = "123456";
process.env.INPUT_PRIVATE_KEY = `-----BEGIN RSA PRIVATE KEY-----
process.env["INPUT_APP-ID"] = "123456";
process.env["INPUT_PRIVATE-KEY"] = `-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEA280nfuUM9w00Ib9E2rvZJ6Qu3Ua3IqR34ZlK53vn/Iobn2EL
Z9puc5Q/nFBU15NKwHyQNb+OG2hTCkjd1Xi9XPzEOH1r42YQmTGq8YCkUSkk6KZA
5dnhLwN9pFquT9fQgrf4r1D5GJj3rqvj8JDr1sBmunArqY5u4gziSrIohcjLIZV0
Expand Down
2 changes: 1 addition & 1 deletion tests/post-token-skipped.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ process.env.STATE_token = "secret123";

// inputs are set as environment variables with the prefix INPUT_
// https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#example-specifying-inputs
process.env.INPUT_SKIP_TOKEN_REVOKE = "true";
process.env["INPUT_SKIP-TOKEN-REVOKE"] = "true";

const mockAgent = new MockAgent();

Expand Down
32 changes: 32 additions & 0 deletions tests/snapshots/index.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,28 @@ The actual snapshot is saved in `index.js.snap`.

Generated by [AVA](https://avajs.dev).

## action-deprecated-inputs.test.js

> stderr
''

> stdout
`app_id — 'app_id' is deprecated and will be removed in a future version. Use 'app-id' instead.␊
private_key — 'private_key' is deprecated and will be removed in a future version. Use 'private-key' instead.␊
skip_token_revoke — 'skip_token_revoke' is deprecated and will be removed in a future version. Use 'skip-token-revoke' instead.`

## main-missing-app-id.test.js

> stderr
'Input required and not supplied: app-id'

> stdout
''

## main-missing-owner.test.js

> stderr
Expand All @@ -14,6 +36,16 @@ Generated by [AVA](https://avajs.dev).
''

## main-missing-private-key.test.js

> stderr
'Input required and not supplied: private-key'

> stdout
''

## main-missing-repository.test.js

> stderr
Expand Down
Binary file modified tests/snapshots/index.js.snap
Binary file not shown.

0 comments on commit 7b1d2ae

Please sign in to comment.