Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thisisparker
Copy link

Creates a new error condition, evaluated after the check for empty OAuth credentials, that is triggered if neither an authkey not a tags is provided. IIUC, tags is not required if an authkey is provided because that authkey may be generated with tags, but we don't know that at runtime.

Fixes #78.

Signed-off-by: Parker Higgins <parker@tailscale.com>
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 == '' }}
Copy link
Contributor

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 which tailscaled 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/"
Copy link

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.

Suggested change
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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message for missing tag is misleading
3 participants