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

Add ubus-tmate to expose tmate control for terminal sharing #839

Merged
merged 11 commits into from
Jan 26, 2021

Conversation

nicopace
Copy link
Member

This PR proposes an interface to a package that allows console sessions to be shared amongst many people.

This is a piece of a feature that will allow, through the lime-app, for users to request support to other networks and be able to monitor what they do with the access it was given, and remove the access once the intervention is done or you don't want them to be accessing your router anymore. It will have as a lime-app screen.

https://user-images.githubusercontent.com/1136597/104211357-bd7a8580-5412-11eb-9d9b-74123590c28e.mp4
(Chrome plays it, Firefox doesn't :( )

Opening it up so to get your general feedback on:

  • using tmate instead of lime-remotesupport: I think it is good that we can generalize it, we will eventually be able to upstream it

You will have to add the tmate feed to your OpenWRT buildroot to build it src-git tmate https://github.com/project-openwrt/openwrt-tmate.git. It needs OpenWRT 19.07.0 as it needs libssh 0.7.6.

@codecov-io
Copy link

codecov-io commented Jan 13, 2021

Codecov Report

Merging #839 (0e54867) into master (0dbbc8a) will increase coverage by 0.21%.
The diff coverage is 91.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #839      +/-   ##
==========================================
+ Coverage   72.19%   72.40%   +0.21%     
==========================================
  Files          38       39       +1     
  Lines        3334     3378      +44     
==========================================
+ Hits         2407     2446      +39     
- Misses        927      932       +5     
Impacted Files Coverage Δ
packages/ubus-tmate/files/usr/lib/lua/tmate.lua 88.63% <91.42%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dbbc8a...0e54867. Read the comment docs.

@spiccinini spiccinini changed the title Lime remotesupport Add tmate-ubus to expose tmate control for terminal sharing Jan 15, 2021
@nicopace nicopace changed the title Add tmate-ubus to expose tmate control for terminal sharing Add ubus-tmate to expose tmate control for terminal sharing Jan 15, 2021
Copy link
Contributor

@germanferrero germanferrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @nicopace. I'll test it out with final version of the limeapp and report back

@nicopace
Copy link
Member Author

How do we do with this package that requires a package from a different feed?
Should we add something in the readme to instruct which feeds need to go with this one?

Copy link
Contributor

@spiccinini spiccinini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR! Good work Nico!

This is just a "administrative" request (and a reminder for everyone included myself), plase can you modify your commit mesages so they start with the module name that is modified? (like ubus-tmate: fix bla bla) this greatly simplifies going through the commit logs. And while you are at it some cosmetics: please remove the final dot and use 'fix bla bla' instead of 'fixes bla bla'. Yes, we should add this recomendations in the contributing info.
A quick way to do it is to do: git rebase -i master and then replace all the pick by reword (save and close) and then all the commit mesages will be desplayed for you to be changed.

Copy link
Contributor

@germanferrero germanferrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've test it against libremesh/lime-app#289 and works great

@nicopace nicopace merged commit 21ca251 into libremesh:master Jan 26, 2021
@nicopace nicopace deleted the lime-remotesupport branch January 26, 2021 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants