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

user.Current issue with new build process #32

Closed
copumpkin opened this issue Mar 27, 2015 · 3 comments · Fixed by #34
Closed

user.Current issue with new build process #32

copumpkin opened this issue Mar 27, 2015 · 3 comments · Fixed by #34

Comments

@copumpkin
Copy link
Contributor

hologram use calls user.Current() (why? SSH authentication on the server side seems to just iterate over all known SSH keys) which seems to fail on darwin when cross-compiled from linux:

golang/go#6376

@frangarciam
Copy link
Contributor

Thanks for catching that, seems like even using the new packages myself for a couple of days was not enough!

Yep, that's a bug alright, we try to avoid things that might depend on cgo on other parts of the code but seems like that one slipped through. I'll fix it on Monday, either removing any reference to the user or relying on a more portable method get the current user.

This gives me another incentive to keep trying to improve our testing framework, which should be easier now that we can automate our builds.

@copumpkin
Copy link
Contributor Author

Still curious what the username is used for there, if you have a chance to explain! I tried tracing it through the code but as far as I could tell, they don't get used on the server. The AssumeRoleMsg gets populated with the user but that doesn't seem to ever get read back out?

@frangarciam
Copy link
Contributor

From what I can see, the username was used at some point in an older version (before hologram was open sourced). From my testing it seems safe to delete and I'll have a PR doing that shortly, but I want to do some more testing first to ensure compatibility (new client with old server, old server with new client, etc...)

Thanks again for reporting and sorry we let this slip, we're working on tightening our testing process.

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 a pull request may close this issue.

2 participants