-
Notifications
You must be signed in to change notification settings - Fork 87
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split OAuth empty error message to create new absent tags error message #82
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Parker Higgins <[email protected]>
shell: bash | ||
run: | | ||
echo "::error title=⛔ error hint::OAuth identity empty, Maybe you need to populate it in the Secrets for your workflow, see more in https://docs.github.com/en/actions/security-guides/encrypted-secrets and https://tailscale.com/s/oauth-clients" | ||
exit 1 | ||
- name: Check Tags Provided | ||
if: ${{ inputs.authkey == '' && inputs.tags == '' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Action supports two styles of keys:
inputs.authkey = 'tskey-...'
for an original authkey, tied to a User.inputs['oauth-secret'] = '...'
for an OAuth client, from whichtailscaled
will allocate an authkey to use for itself.
A inputs.tags is required if inputs['oauth-secret'] is set. As written, this PR uses inputs.authkey == ''
to mean "inputs['oauth-secret'] must not be empty or we wouldn't have gotten this far."
I'd suggest instead:
if: ${{ inputs['oauth-secret'] != '' && inputs.tags == '' }}
if: ${{ inputs.authkey == '' && inputs.tags == '' }} | ||
shell: bash | ||
run: | | ||
echo "::error title=⛔ error hint::At least one ACL tag is required for nodes created by this Action. Ensure an appropriate tag exists in your ACL and provide it in your workflow. See more in https://tailscale.com/kb/1068/acl-tags/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error hint is accurate only if the action is authorized via Tailscale OAuth client--not if an auth key is used instead.
Validity of a tag, like the suitability of the set of tags provided via the sometimes-required tags
input parameter, seems out of scope, here. Validity of any given tag, as well as suitability of the set of tags provided, can be determined by the server behind the API this action uses, but not by the action itself.
I suggest wording the message so as to guide the user to providing input parameters which satisfy the requirements for this action as they are documented on its GitHub Marketplace page.
echo "::error title=⛔ error hint::At least one ACL tag is required for nodes created by this Action. Ensure an appropriate tag exists in your ACL and provide it in your workflow. See more in https://tailscale.com/kb/1068/acl-tags/" | |
echo "::error title=⛔ error hint:Use of this action's `oauth-secret` input parameter requires that parameter `tags` be provided, but it is empty. See https://tailscale.com/kb/1276/tailscale-github-action/#add-the-tailscale-github-action-to-a-workflow and `devices` scope in https://tailscale.com/kb/1215/oauth-clients/#scopes" |
Creates a new error condition, evaluated after the check for empty OAuth credentials, that is triggered if neither an
authkey
not atags
is provided. IIUC,tags
is not required if anauthkey
is provided because thatauthkey
may be generated with tags, but we don't know that at runtime.Fixes #78.