Skip to content

Conversation

arekinath
Copy link

This updates node-manta to use the latest version of smartdc-auth, which has integrated all of the local improvements in our auth.js, as well as a number of useful changes from node-smartdc and others.

The original PR on smartdc-auth (TritonDataCenter/node-smartdc-auth#3) has a lot more information about exactly what has changed in this update.

Other than adding the new dependency, this patch removes the old auth.js file, the old dep on ssh-agent, and adds manta: true to the argument of all calls to signUrl -- this tells the now shared signUrl code to use Manta-style sub-users rather than SDC-style ones.

@arekinath
Copy link
Author

arekinath@010825e is a diff between the old node-manta auth.js and its replacement in smartdc-auth, for inspecting the changes

@bahamas10
Copy link
Contributor

Huge improvement... I'm a big fan of negative lines added. I'll test in a bit and report back, but so far the code itself looks good... no obvious problems.

@bahamas10
Copy link
Contributor

Tested against flat private key file and also ssh-agent and can confirm it works as expected. LGTM

@arekinath
Copy link
Author

diff updated to smartdc-auth 2.0.1, auth.js diff to arekinath@c70dea0

@xer0x
Copy link

xer0x commented Oct 1, 2015

I should make a separate issue, however I tried out Alex's PR to see if it fixed an SSH error that I'm getting on OSX El Capitan. This didn't fix my problem.

mls /xer0x/stor
mls: Error: SHA256:MBcMbsS2QpZTxK23uWC5J111SfvnBtuYkvv/hygYtAQ not found in: /Users/drew/.ssh

@davepacheco
Copy link
Contributor

I only took a quick skim of the gist diff. Lines 252-265: I'm not sure how much this matters, but there's a race between checking and using here. Also, what's the significance of 64K? Should that be a constant like "MAX_KEY_LENGTH" or something? Ditto in the "else" block here.

A few nits, up to you:

  • KeyNotFoundError.join: maybe a comment explaining what this does?
  • "manta": maybe "mantaStyleSubuser"? I'm afraid calling it "manta" may make it tempting to overload it for unrelated changes.
  • The comments at line 164 and lines 171-173 are not sentences.

My main question is just making sure that it's interface-compatible. The node-manta client uses or exposes auth.signUrl, auth.cliSigner, auth.privateKeySigner, auth.sshAgentSigner, and auth.loadSSHKey, and it looks like those all at least exist.

@arekinath arekinath force-pushed the pubapi-1146 branch 2 times, most recently from 378f981 to 4a55a25 Compare October 1, 2015 17:24
@arekinath
Copy link
Author

Updated, I've incorporated those comments. New fakediff is arekinath@6cc122e

This also includes the TOOLS-1214 changes now too

…ly sent to server

TOOLS-1214 Large number of keys in ssh-agent leads to failure
@arekinath
Copy link
Author

Merged as e5e6e75

@arekinath arekinath closed this Oct 5, 2015
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.

4 participants