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

Initial setup of AblyD #1

Open
wants to merge 19 commits into
base: basic-branch
Choose a base branch
from
Open

Initial setup of AblyD #1

wants to merge 19 commits into from

Conversation

tomczoink
Copy link
Contributor

Demo which allows one to wrap up any terminal process on a device and make it executable and have its stdin/stdout available over Ably. This extends the websocketd functionality to make use of Ably rather than a WebSocket server hosted on the main device/server.

Main checks are to state what needs further clarification in the README/notes, and code improvements recommendations.

Instead of a message element to define the action, we instead specify this with the message name.
This currently only defines normal messages, not presence messages.
@tomczoink tomczoink requested review from Jmgr and Ugbot July 26, 2021 15:13
Incorrectly said the flag for the Ably API key was `ablykey` rather than `apikey`
Means PID in logging can identify each individual process
@Jmgr
Copy link

Jmgr commented Jul 28, 2021

I don't think I will be able to have a look at this today because of support queries and investigations. Could you have a look @cruickshankpg or @lmars please?

Rather than relying on a single level of channel namespace based on PID, this reflects the serverID and a variable base namespace to allow for further customization.

This includes improvements to allow for commands to be used on specific servers rather than any server in a shared command channel.
This should make it easier for subscribers to handle output from servers.
@tomczoink tomczoink requested review from lmars and removed request for Jmgr July 30, 2021 08:53
This is to help differentiate between AblyD instances and the processes it spools off.
README.md Outdated
Now all you need to do is run the main go file, passing in the command line command you want to be run as a parameter! For example, to use a bash file `count.sh`, which is included in this repo's `examples` folder, just run:

```bash
~ $ ./ablyD --apikey=YOUR_API_KEY ./examples/bash/count.sh
Copy link
Member

Choose a reason for hiding this comment

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

I recommend expecting the API key to be passed as an environment variable rather than a command line parameter, since command line parameters leak more easily.

README.md Outdated
~ $ ./ablyD --apikey=YOUR_API_KEY ./examples/bash/count.sh
```

The program is now running, waiting for a message in the `command` channel in Ably to be sent. The message's data field should match the structure, with the value `start` for the message's name:
Copy link
Member

Choose a reason for hiding this comment

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

I sent a message to the command channel and nothing happened, after digging a little bit it seems the channel needs to be ablyd:command.

I'd recommend giving a full example of the message that should be published, for example using cURL:

curl -X POST https://rest.ably.io/channels/ablyd:command/messages \
  -u "${API_KEY}" \
  -H 'Content-Type: application/json' \
  --data \
  '{
    "name": "start",
    "data": {
      "MessageID": "unique string value",
      "Args": [ "some", "additional", "args", "for", "the", "programs" ]
    },
    "format":"json"
  }'

}
```

The client can identify the process which has started for them by the `MessageID`, then use the `Prefix` to connect to an input and an output channel for the process. These will be of structure `{Prefix}{Pid}:serverinput` and `{Prefix}{Pid}:serveroutput`.
Copy link
Member

Choose a reason for hiding this comment

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

It would be useful if the command message included the names of the channels explicitly, for example:

{
  "MessageID": "unique string value matching the requesting MessageID",
  "Stdin": "ably:c42jl786n88h66cnsu70:5328:serverinput",
  "Stdout": "ably:c42jl786n88h66cnsu70:5328:serveroutput"
}


The client can identify the process which has started for them by the `MessageID`, then use the `Prefix` to connect to an input and an output channel for the process. These will be of structure `{Prefix}{Pid}:serverinput` and `{Prefix}{Pid}:serveroutput`.

Subscribing to the `serveroutput` channel will allow the client to receive any stdout messages from the server. The client can also publish messages into the `serverinput` channel which will be passed into the stdin of the process.
Copy link
Member

Choose a reason for hiding this comment

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

This design is a little racy, since I only get to know the channels to subscribe to after the process has started, and so in my testing I sometimes miss the first bit of output.

It would be better if I could create the command, get back an identifier, subscribe to the channels, and then start the command, this way I'll always be subscribed before anything gets published to the output channel.

I guess this would need ablyD to generate random identifiers for commands rather than using the PID (since the PID won't be known before starting the command), but I don't think that presents a problem.


Subscribing to the `serveroutput` channel will allow the client to receive any stdout messages from the server. The client can also publish messages into the `serverinput` channel which will be passed into the stdin of the process.

This will continue until the program naturally terminates, resulting in the process dying, or the client submits a message to the `serverinput` with data `KILL`.
Copy link
Member

Choose a reason for hiding this comment

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

We should consider adding a third signal channel the client can use to send OS signals to the process rather than trying to re-purpose the stdin channel.

That being said, we should also consider just using a single channel with typed messages, rather than using different channels for stdin / stdout / signals. For example, there could be a single "data" channel with messages like {"type":"STDIN","data":"..."} and {"type":"SIGNAL","data":"15"}. This is pretty much what the SSH protocol does (see https://datatracker.ietf.org/doc/html/rfc4254#section-5), but is probably out of scope for what you're trying to achieve here 🙂

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.

None yet

3 participants