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

fly command not accepting user input after returning from auto login flow #2414

Closed
mhuangpivotal opened this issue Jul 25, 2018 · 5 comments
Assignees
Milestone

Comments

@mhuangpivotal
Copy link
Contributor

@mhuangpivotal mhuangpivotal commented Jul 25, 2018

Bug Report

Step to repro:

  1. fly -t ci logout
  2. fly -t ci set-pipeline -p test -c something.yml
  3. click the link to auth in browser
  4. try to type y to apply pipeline configuration
  5. notice some of the user inputs do not register

Related PR: concourse/fly#200

The following can also be handy:

  • Concourse version: v4.0.0
  • Deployment type (BOSH/Docker/binary): BOSH
  • Infrastructure/IaaS: GCP
  • Browser (if applicable): Chrome
  • Did this used to work? Not sure
@mhuangpivotal

This comment has been minimized.

Copy link
Contributor Author

@mhuangpivotal mhuangpivotal commented Oct 5, 2018

There are two ways to complete fly login, either by following the login URL or by entering the auth token manually. Two goroutines are responsible for handing the callback from the login URL and scanning user input for the manual entry. However, when the auth is complete through the login URL, the input scan in the second goroutine is not terminated, and would swallow inputs from a subsequent command.

https://github.com/concourse/concourse/blob/master/fly/commands/login.go#L174-L204

Specifically, Scanf on https://github.com/concourse/concourse/blob/master/fly/commands/login.go#L291 blocks until the user enters a token.

@mhuangpivotal

This comment has been minimized.

Copy link
Contributor Author

@mhuangpivotal mhuangpivotal commented Oct 16, 2018

Had to revert the commit since it hangs in Linux/Windows.

@mhuangpivotal mhuangpivotal reopened this Oct 16, 2018
@cirocosta

This comment has been minimized.

Copy link
Member

@cirocosta cirocosta commented Oct 18, 2018

Hey @mhuangpivotal ,

Maybe doing some like described here (http://ixday.github.io/post/golang-cancel-copy/) might allow us to fix this issue.

We could switch from scanf to performing io.Copy w/ cancellation and then cancel the reading from stdin.

Wdyt?

thx!

@xtremerui

This comment has been minimized.

Copy link
Contributor

@xtremerui xtremerui commented Nov 12, 2018

@cirocosta with current login flow there is no easy way to terminate the go routine that blocked by scanf. In the example you mentioned it would still block on io.read if it doesn't return promptly (mentioned in the comment), which is exactly the issue for scanf here.

I think we might need to review the current login flow for listening manual token input as this is really a rare use case. Ideas are welcome!

@jama22

This comment has been minimized.

Copy link
Member

@jama22 jama22 commented Nov 19, 2018

Would it be easier if this autologin flow only supports browser callback based token transmission?

mhuangpivotal added a commit that referenced this issue Nov 30, 2018
since the remote option is only available to the login command

#2414
@jama22 jama22 removed the accepted label Dec 3, 2018
@jama22 jama22 added the accepted label Dec 10, 2018
@jama22 jama22 closed this Dec 10, 2018
@vito vito added this to the v5.0.0 milestone Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.