pantheon-systems / terminus-github-actions

A GitHub Action for setting up Pantheon's CLI tool, Terminus.
MIT License
17 stars 11 forks source link

Only run login when input is provided. #6

Closed kopepasah closed 1 year ago

kopepasah commented 2 years ago

Currently login causes a failure when a Pantheon token is not supplied. Although it is 'required' in the inputs, Github does not currently have any checks for when a required input is not supplied.

kopepasah commented 2 years ago

Yeah, it's been a common complaint for many users. My guess is Github will add a check later, but until then it is best to add a check.

namespacebrian commented 2 years ago

Are there use cases for installing Terminus in CI without needing to authenticate it?

namespacebrian commented 2 years ago

If I'm writing a user story for this:

As a Pantheon customer using Terminus in CI, I don’t want a machine-token to be required because Github wouldn’t enforce that requirement anyway.

?

kopepasah commented 2 years ago

If I'm writing a user story for this:

As a Pantheon customer using Terminus in CI, I don’t want a machine-token to be required because Github wouldn’t enforce that requirement anyway.

?

@namespacebrian this story sounds correct.

Honestly, the only potential use case for running without the machine token would be to play around with terminus in Github Actions runner, while developing a workflow or action. Personally, I could go either way, but without the Machine Token, this action currently fails, although terminus is installed and I could run auth:login after install (if the runner did not error due to missing token).

I will note that if the check is not added, then we need to remove the test here, because it is just testing that terminus is installed and commands do not error, without a token.

namespacebrian commented 1 year ago

This is tracked internally as FEAT-988.

So currently if someone tries to add this action but doesn't also set a machine token, their whole action will fail with a "missing required field" message, right?

Is there any concern that making this optional might lead someone to basically forget to add a machine token and then complain that "the action is broken" when their terminus commands error out as a result?

I'm not super experienced with Github Actions.. would it be simple and reasonable to add a warning if a machine token isn't added that "terminus commands requiring authentication will not work without a machine token"?

G-Rath commented 1 year ago

So currently if someone tries to add this action but doesn't also set a machine token, their whole action will fail with a "missing required field" message, right?

No, because required inputs are currently not enforced.

would it be simple and reasonable to add a warning if a machine token isn't added that "terminus commands requiring authentication will not work without a machine token"?

You could add a standard echo call, which would be visible if someone looked at the step/action output - I wouldn't go further than that (i.e. emitting a warning annotation) because there could be valid use-cases for not setting the machine token via the action which'd make the warning incorrect.

However, I think at this point it's not worth the extra fuss that it will be a common problem/UX (especially since its already possible due to required not being enforced) - if it does turn out to be a common occurrence, then that would make it worth digging into.

namespacebrian commented 1 year ago

Can add a warning later if someone actually does complain