feat: Add a skip_token_revoke input for configuring token revocation
#54
Conversation
|
I've seen this option but couldn't think of a use case for it. Do you know why it was added to tibdex/github-app-token or can you think of a use case when someone would need it? No biggie but could you create an issue next time before putting in work into a pull request? That way we can have a discussion and avoid unnecessary work |
@gr2m I opened #55, and I wrote about our use case there. Let’s move discussion there. |
README.md
Outdated
| ### `revoke` | ||
|
|
||
| **Optional:** Whether to revoke the token when the current job is complete. Default: `"true"`. |
There was a problem hiding this comment.
I wonder if revoke is the best possible argument for this feature. As the default is true. I think it might be more elegant to have an option that is not set by default at all, and needs to be set to overwrite it.
For example: do_not_revoke_token_after_job. Long but clear 🤷🏼
There was a problem hiding this comment.
I wondered the same thing! Either of those suggestions works for me.
The 5 actions I surveyed in https://github.com/github/core-ux/discussions/142#discussioncomment-7177238 (only accessible to Hubbers) (under “comparative analysis…”) didn’t reveal a widely-used name, but I’ll review the actions listed in #42 too.
There was a problem hiding this comment.
actually I think we should use dashes for our arguments, it seems to be the convention for actions/* actions, e.g. see https://github.com/actions/checkout/. So we would do skip-token-revoke
There was a problem hiding this comment.
@gr2m The existing inputs use underscores—would we want to migrate those to dashes too?
There was a problem hiding this comment.
- tibdex/github-app-token — Configurable with
revoke. Default: Doesn’t revoke token.
It looks like this is a new feature for that action. The documentation isn't very clear about this, but it looks like the value of revoke is actually true by default.
I'm in favor of using revoke in the same way (as you did originally), but I don't feel strongly about it, so I'll leave it up to @gr2m.
There was a problem hiding this comment.
okay let's go with skip_token_revoke, let's have a separate discussion about whether to migrate arguments to dashes. I'd rather change it now when adaption is still early
There was a problem hiding this comment.
Sounds good! Just throwing out one more possible alternative name: do_not_revoke.
Take it or leave it; I don't feel strongly about this one either. 😄
There was a problem hiding this comment.
…let's have a separate discussion about whether to migrate arguments to dashes…
Opened #57 for this! 👍
There was a problem hiding this comment.
…let's go with
skip_token_revoke…
Done, via 5a2e1e7.
smockle
changed the title
revoke input for configuring token revocationskip_token_revoke input for configuring token revocation
# [1.4.0](v1.3.0...v1.4.0) (2023-10-06) ### Features * Add a `skip_token_revoke` input for configuring token revocation ([#54](#54)) ([9ec88c4](9ec88c4)), closes [1#L46-L47](https://github.com/1/issues/L46-L47) [1#L132](https://github.com/1/issues/L132)
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixes #55
Currently,
actions/create-github-app-tokenalways/unconditionally revokes the installation access token in apoststep, at the completion of the current job. This prevents tokens from being used in other jobs.This PR makes this behavior configurable:
skip-token-revokeinput is not specified (i.e. by default), the token is revoked in apoststep (i.e. the current behavior).skip-token-revokeinput is set to a truthy value (e.g."true"1), the token is not revoked in apoststep.This PR adds a test for the
skip-token-revoke: "true"case.This is configurable in other app token actions, e.g. tibdex/github-app-token and wow-actions/use-app-token.
Footnotes
Note that
"false"is also truthy:Boolean("false")istrue. If we think that’ll potentially confuse folks, I can requireskip-token-revoketo be set explicitly to"true". ↩