-
Notifications
You must be signed in to change notification settings - Fork 56
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
T2 root #290
T2 root #290
Changes from all commits
5b740e4
ec33032
e6a8b58
fe72e82
32611c0
7aaf6ad
4d82d6e
c592d9c
61b1042
70c8002
4391bdd
f262fc8
ef36e92
9c1e15d
2c0c982
95a71da
e680cbc
e8dc023
c76a849
9860c3d
71a2820
24d3120
f69d51a
c3da285
370e1f7
eb7346e
a6d5551
9424ce0
4c3ed3b
804d493
67ff671
2e2334f
3fbe20c
99f3c43
0fa8ba8
3056a6c
a879c67
2cb829b
6f6dbff
17e0914
b882da5
eb50c0f
cdd15b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,6 +197,36 @@ parser.command('rename') | |
.help('Change the name of a Tessel to something new.'); | ||
|
||
|
||
// accessing the root shell of your tessels | ||
// Fixes issue https://github.com/tessel/t2-cli/issues/80 | ||
/** | ||
$ t2 root --help | ||
> Usage: tessel root [-i <path>] [--help] | ||
> -i <path>: provide a path to the desired ssh key | ||
$ | ||
|
||
$ t2 root | ||
> Accessing root... | ||
root@192.168.128.124 # | ||
*/ | ||
var default_id_rsa = '~/.tessel/id_rsa'; | ||
var functional_msg = '\nGain SSH root access to one of your authorized tessels (menu listing if multiple targets)'; | ||
parser.command('root') | ||
.usage(functional_msg + '\n\nUsage: t2 root [-i PATH] [--help]\n\n-i PATH: Optional targeting a different Private Key \n\n(Note: default target created by "t2 key generate" is ' + default_id_rsa + ')\n') | ||
.option('path', { | ||
abbr: 'i', | ||
full: 'path', | ||
metavar: 'PATH', | ||
default: default_id_rsa, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 this path is already used in another part of the codebase (probably There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 it is:
within controller.js That way I would replace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 I think it makes the most sense for the path to live in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 you mean changing
after changing provision.js like
|
||
help: 'Private Key (Note: created by "t2 key generate")' | ||
}) | ||
.callback(function(opts) { | ||
controller.root(opts) | ||
.then(closeSuccessfulCommand, closeFailedCommand); | ||
}) | ||
.help(functional_msg); | ||
|
||
|
||
module.exports = function(args) { | ||
parser.parse(args); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,10 @@ var sprintf = require('sprintf-js').sprintf; | |
var cp = require('child_process'); | ||
var async = require('async'); | ||
var controller = {}; | ||
controller.ssh = require('./root_controller'); | ||
var Menu = require('terminal-menu'); | ||
var networkInterfaces = require('os').networkInterfaces(); | ||
var debug = require('debug')('controller'); | ||
|
||
Tessel.list = function(opts) { | ||
|
||
|
@@ -22,21 +26,22 @@ Tessel.list = function(opts) { | |
// When a Tessel is found | ||
seeker.on('tessel', function displayResults(tessel) { | ||
var note = ''; | ||
|
||
// Add it to our array | ||
foundTessels.push(tessel); | ||
|
||
// Add a note if the user isn't authorized to use it yet | ||
if (tessel.connection.connectionType === 'LAN' && !tessel.connection.authorized) { | ||
note = '(USB connect and run `tessel provision` to authorize)'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remove these unrelated styling changes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 yes I did many times (also at other code parts) but the linter automatically edited this many times. So I gave up to do this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 did it only do this when you ran with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also |
||
} | ||
|
||
// Print out details... | ||
logs.basic(sprintf('\t%s\t%s\t%s', tessel.name, tessel.connection.connectionType, note)); | ||
}); | ||
|
||
// Called after CTRL+C or timeout | ||
function stopSearch() { | ||
if (foundTessels.length > 1) { | ||
debug('heuristics ? => found: ' + foundTessels.length + ' === 1 || (' + foundTessels.length + ' === 2 && ' + foundTessels[0].name + ' === ' + foundTessels[1].name + ' && ' + foundTessels[0].connections[0].connectionType + ' !== ' + foundTessels[1].connections[0].connectionType + ')'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 what is this debug line for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 it was related to debug network topology behaviors. I used it for my own but decided to keep it in because some people do not know they will find their Tessel twice due to network topology. A simple case I would expect lots of time is a iMac is connected by LAN and WLAN to the same router for different development reasons like multi IP services. Of cause people who do this, know it, but ... they will forgot it and need to be pointed on while contributing to Tessel... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 I think we should remove it and add infrastructure to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I am currently working on that feature in Tessel.list. At the moment I got crazy about sometimes there are objects with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 I don't think you need to worry about the number of connections for a Tessel. You can just check the That should give you the name and even ip address of the Tessel. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes ... that was new ... ;) |
||
|
||
} | ||
// If the seeker exists (it should) | ||
if (seeker !== undefined) { | ||
// Stop looking for more Tessels | ||
|
@@ -54,8 +59,10 @@ Tessel.list = function(opts) { | |
} | ||
// If we have only one Tessel or two Tessels with the same name (just USB and LAN) | ||
else if (foundTessels.length === 1 || | ||
(foundTessels.length === 2 && foundTessels[0].name === foundTessels[1].name)) { | ||
(foundTessels.length === 2 && foundTessels[0].name === foundTessels[1].name && foundTessels[0].connections[0].connectionType !== foundTessels[1].connections[0].connectionType)) { | ||
// Close all opened connections and resolve | ||
console.log(foundTessels[0].connections.connectionType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 can you remove these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh -- of cause ... don't know why I didn't see this |
||
console.log(foundTessels[1].connections.connectionType); | ||
controller.closeTesselConnections(foundTessels) | ||
.then(resolve); | ||
} | ||
|
@@ -170,6 +177,134 @@ Tessel.get = function(opts) { | |
} | ||
}); | ||
}; | ||
/* | ||
Because of sync problem with master trunk and parallel development on Tessel.list | ||
it was necessary to go my own way... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just pull & rebase as development proceeded? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 do you need some assistance rebasing this commit on master? How can we take advantage of the work that was done on master while you were working on this? What of this new code can we remove? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally, we should remove the entire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, if the logAndFinish got a menu and returns the selected Tessel (or set the process.env.TESSEL) the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
The seekTessels method never uses heuristics because all possible login variants are required. | ||
There is a new feature in: In the case your network topology causes a Tessels IP is found twice, the | ||
menu will only list it once. | ||
|
||
Due to my own stupid failing with mixing up a tessel is accessable via gateway and a Tessel is in the | ||
same Netmask, I've added a notice for the case someone run into same issue. | ||
*/ | ||
Tessel.seekTessels = function(opts) { | ||
return new Promise(function(resolve, reject) { | ||
|
||
if (opts.timeout && typeof opts.timeout === 'number') { | ||
// Stop the search after that duration | ||
setTimeout(stopSeeker, opts.timeout * 1000); | ||
} else { | ||
// default to 3 seconds searching | ||
setTimeout(stopSeeker, 3000); | ||
} | ||
logs.info('Searching accessable Tessels ...'); | ||
|
||
var seeker = new discover.TesselSeeker().start(); | ||
var tessels = []; | ||
|
||
// When we find Tessels | ||
seeker.on('tessel', function(tessel) { | ||
controller.closeTesselConnections(tessel) | ||
.then(function() { | ||
var known = false; | ||
if (tessels.length >= 1) { | ||
for (var i in tessels) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use |
||
if (tessel.connections[0].ip === tessels[i].connections[0].ip && tessel.connections[0].connectionType === 'LAN') { | ||
// Your network topology causes a single tessel is found twice | ||
debug('known = true'); | ||
known = true; | ||
} | ||
} | ||
|
||
} | ||
if (known) { | ||
debug('Due to your Network-Topology ' + tessel.name + ' is found twice! (' + tessel.connections[0].ip + ')'); | ||
} else { | ||
// Add it to our array | ||
tessels.push(tessel); | ||
} | ||
}); | ||
}); | ||
|
||
seeker.on('error', function(e) { | ||
reject(e); | ||
}); | ||
|
||
function stopSeeker() { | ||
try { | ||
// If there were no Tessels found | ||
if (!tessels || tessels.length === 0) { | ||
// Report the sadness | ||
debug('We are searching the following networks:\n', networkInterfaces); | ||
debug('\n(Important: Check your Netmask - we do not follow gateways for discovering!)'); | ||
logs.warn('No Tessels found (DEBUG=controller t2 root # might help!)'); | ||
resolve(); | ||
} | ||
seeker.stop(); | ||
seeker = null; // preventing memory leaks | ||
return resolve(tessels); | ||
} catch (e) { | ||
reject(e); | ||
} | ||
} | ||
}); | ||
}; | ||
/* | ||
The T2 root command is used to login into the firmware and gaining superuser access. | ||
The security is provided by RSA - your Tessel get the keys while provisioning via USB. | ||
|
||
If you have only one Tessel, the T2 root command will login directly else there is | ||
a ncurses like menu you can select the Tessel you like to gain root access... | ||
|
||
The structure of code and maybe unusual parts are paid due being testable. | ||
The lightweight terminal-menu isn't written testable what needs a little bit hacking. | ||
|
||
Finally some parts are causing from my personnal learning curve about writing unit tests using sinon! | ||
*/ | ||
controller.root = function(opts) { | ||
// ~ conversion to home because spawn isn't able to handle this right | ||
if (opts.path && opts.path.substring(0, 1) === '~') { | ||
var home = process.env.HOME; | ||
opts.path = opts.path.replace('~', home); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will break if there is a
Please use the method shown here: https://github.com/tessel/t2-cli/blob/master/lib/tessel/deploy.js#L105-L123 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rwaldron ... you are right. I didn't expect someone would use ~ in a different way ... |
||
} | ||
var rtm; | ||
if (!opts.menu) { | ||
// TODO: Tessel cooperate identity conform menu required (color, ascii logo) | ||
rtm = Menu({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Student007 we should pull the menu out of this PR, and place it into another PR where it can pop up a menu when any command is run and there is ambiguity about which T2 is the "right T2. In fact, it would fix another long standing issue, #235. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 I am currently working on this #235 and created a branch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 setting So it will simple keep the menu is opened for setting the default Tessel to work with. An other idea would be a The controller.js code doesn't show something about where and how you are using the selected Tessel. Heuristics only return one Tessel and logs.info is talking about... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A menu for selecting a Tessel should only be used with
@Student007 I don't recommend storing state in a file across CLI calls because things can get hairy if you run two commands at once or another program is using the CLI API. In the latter case, you could have two processes writing to the file at the same time. When logAndFinish get called after the search for a Tessel, if no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK like current idea of logSelected but moved to logAndFinish else {...} (btw: the new Menu is moved to prepareMenu)
You are right. But in this special case it overrides the file when the user is doing interaction. Another process what overrides the file would fail the same way two terminals try to pipe data into one file or do other stuff with the same target. I think it is the only useful way to fix a special Tessel to work with. I think there would be cases a developer will have e.g. 8 Tessels with different apps what should work together and he likes to select the current one without typing every time. He would use Another idea would be a new command There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case where a user has several Tessels they are trying to work with, they could more easily set a different There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 I still didn't think about this from that point of view. You are right, if someone opens several terminals (one for each Tessel 2) the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnnyman727 based on this ideas I am preparing an additional pull request, but I am not done at the moment. What will be in:
|
||
width: 50, | ||
x: 1, | ||
y: 2, | ||
bg: 'red' | ||
}); | ||
} else { | ||
// used while testing to override methods by stubs (testing doesn't like user interactions) | ||
rtm = opts.menu; | ||
} | ||
return new Promise(function(resolve, reject) { | ||
controller.ssh.seek(opts) | ||
.then(function(tessels) { | ||
|
||
if (tessels && tessels.length >= 2) { | ||
controller.ssh.multipleTessels(opts, tessels, rtm, resolve, reject); | ||
} else if (tessels && tessels.length === 1) { | ||
if (tessels[0].connection.authorized) { | ||
controller.ssh.runSSH(0, opts, tessels, resolve, reject); | ||
} else { | ||
logs.warn('Sorry, you are not authorized!'); | ||
logs.info('"t2 key generate" might help :-)'); | ||
resolve(); | ||
} | ||
} else { | ||
// everything works fine, but no tessels found | ||
resolve(); | ||
} | ||
|
||
}).catch(function(e) { | ||
reject(e); | ||
}); | ||
}); | ||
}; | ||
|
||
/* | ||
Takes a list of Tessels with connections that | ||
|
@@ -451,7 +586,6 @@ controller.connectToNetwork = function(opts) { | |
}); | ||
}); | ||
}; | ||
|
||
module.exports = controller; | ||
|
||
// Shared exports | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnyman727 eventually we're going to need to put these icky strings into template files and load them in at runtime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Student007 there is no need to add the
usage
function here.nomnom
takes care of it for you as long as you addhelp
tags to all the parameters. Check out the other commands for an example.@rwaldron ugh agreed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is, it would write
Usage: node t2 root
instead oft2 root
- I like the idea of templates.@Frijol it looks like you are figuring out some things at #274 - is there also a general way to fix the
node t2 root
problem ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have:
I get:
What's the issue? You want to get rid of
/usr/local/bin/iojs
part?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Frijol yes 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would be a new issue / contribution task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Yeah, that should be a separate issue. Good idea. A brief look through nomnom.js doesn't reveal a solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I couldn't find a provided solution in nomnom.js ... also reading code ... did this before some weeks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Student007 what do you think about leaving the usage string our for this PR and then we'll make a separate PR that grabs all strings from a template file. I'd rather keep them all consistent at any one time so that it's more straightforward for new contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've figured out ... and created a pull request:
#310
The reason I though it doesn't work was to use the "script" part on on the wrong object level.