Design Document #1

arazmj · 2025-03-20T01:58:04Z

No description provided.

tigrato · 2025-03-20T10:05:55Z

DESIGN.md

+
+GetJobStatus(JobStatusRequest) returns (JobStatusResponse)
+
+GetJobLogs(JobLogsRequest) returns (JobLogsResponse)


What if the job is still running?

It will return the updated logs if the job is still running. (Similar behavior as kubectl)

DESIGN.md

tigrato · 2025-03-20T10:14:02Z

DESIGN.md

+* Job output is captured and stored in memory and sent to CLI clients.
+
+### Job Execution:
+* The process runs within its assigned cgroup.


How is the process added to the assigned cgroup?

Can you clarify at what point the job PID is written to cgroup.procs? When does the child actually start executing, and when does it join the cgroup?

The job is first executed, once we received the PID we add it to cgroup.procs

DESIGN.md

tigrato · 2025-03-20T12:39:36Z

DESIGN.md

+  * The action will run if the client identity is defined in `sentry-roles.toml` file and the user has permission for the action.
+* The server validates request and spawns a new process using `/bin/sh -c`.
+* The process is assigned to a cgroup with defined CPU, memory, and I/O constraints.
+* Job output is captured and stored in memory and sent to CLI clients.


How does the library informs the gRPC that new data was written by the job and needs to be forwarded to clients?
can you iterate a bit on it?

The library has a public method StreamOutput() that takes a callback function as a parameter.

type OutputCallback func(data []byte) error func (m *JobManager) StreamOutput(jobID int32, callback OutputCallback) error {}

The server will call this method with a function closure that calls the stream.

s.manager.StreamOutput(req.JobId, func(data []byte) error { return stream.Send(&pb.JobOutput{ JobId: req.JobId, Data: data, })

What happens if one callback completes slowly, for example if the stream.Send() is on a slow connection?

Once the job started a running goroutine in the background reads the stdout/stdout and calls stream.Send() for each client them to clients.

tigrato · 2025-03-20T12:40:14Z

DESIGN.md

+### Job Initialization:
+* User starts a job via CLI.
+* The server authorizes the user against for the request against the roles defined in sentry-security. 
+  * The client identity is the CN field of the CA


Which TLS parameters like version, cyphers... do you plan to use here?

I will use TLS1.3 with the default AEAD cipher suites and curve preferences.

var defaultCipherSuitesTLS13 = []uint16{ TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256, }

var defaultCurvePreferences = []CurveID{ X25519, // X25519 (ECDH/curve25519) CurveP256, // NIST P-256 CurveP384, // NIST P-384 CurveP521, // NIST P-521 }

DESIGN.md

rosstimothy · 2025-03-20T17:44:54Z

DESIGN.md

+  rpc KillJob (KillJobRequest) returns (KillJobResponse) {}
+  rpc GetJobStatus (JobStatusRequest) returns (JobStatusResponse) {}
+  rpc StreamJobLogs (JobLogsRequest) returns (stream JobOutput) {}
+  rpc ListJobs (ListJobsRequest) returns (ListJobsResponse) {}


Suggestion: remove support for listing - it is not a requirement for the challenge.

But I need the job ids and the state of running jobs for testing?

You should be able to get that from GetJobStatus though right?

The start request could also return the identifier instead of an empty response.

Right, removing it makes the testing more difficult while having the ListJob doesn't add so much to the code.
But I can still remove it if you have strong openion.

DESIGN.md

rosstimothy · 2025-03-20T17:47:16Z

DESIGN.md

+  bool is_running = 1;
+  string status = 2;


Suggestion: use an enum instead of multiple fields?

Removed status field. It is not needed.

Does is_running provide enough information about a job though? Is it not running because the job ended or because a user stopped it?

Would you explain how many states for status do we need?

Stopped

Finished

Running

DESIGN.md

timothyb89 · 2025-03-20T23:53:26Z

DESIGN.md

+* Job output is captured and stored in memory and sent to CLI clients.
+
+### Job Execution:
+* The process runs within its assigned cgroup.


Can you clarify at what point the job PID is written to cgroup.procs? When does the child actually start executing, and when does it join the cgroup?

timothyb89 · 2025-03-20T23:57:07Z

DESIGN.md

+  * The action will run if the client identity is defined in `sentry-roles.toml` file and the user has permission for the action.
+* The server validates request and spawns a new process using `/bin/sh -c`.
+* The process is assigned to a cgroup with defined CPU, memory, and I/O constraints.
+* Job output is captured and stored in memory and sent to CLI clients.


What happens if one callback completes slowly, for example if the stream.Send() is on a slow connection?

timothyb89 · 2025-03-21T00:16:57Z

DESIGN.md

+```
+
+## Job IDs
+Jobs are identified using UUIDs rather than process IDs or sequential counters. This choice addresses several key requirements:


nit, but it seems like some content is missing here?

timothyb89 · 2025-03-21T00:19:31Z

DESIGN.md

+}
+
+message StartJobRequest {
+  string command = 1;


If this includes command + arguments, how will it be executed? Will you be parsing the string yourself? If so, can you think of any design tweaks that might remove the need to do so?

Yes, Ideally we want to be able to do something like sentry start -cmd bash -c 'i=1; while true; do echo "$i"; i=$((i+1)); sleep 1; done'.

Right, this still would involve some level of parsing on both client and server since command is just a single string value, right? Can you think of a change to the data model that might simplify this?

Yes, I have some ideas. I can handle that. I will change the data type.

@timothyb89 Would something like this be acceptable? Note the -- after bash.

sentry start -cmd bash -- -c 'i=1; while true; do echo "$i"; i=$((i+1)); sleep 1; done'

I think that's a good start, but does the proto change at all to accommodate this?

It will look like this

message StartJobRequest { string command = 1; repeated string commandArgs = 2; }

timothyb89 · 2025-03-21T00:20:07Z

DESIGN.md

+}
+
+message JobOutput {
+  int32 job_id = 1;


Is job_id still an int32?

Design Document

6dcec6e

tigrato reviewed Mar 20, 2025

View reviewed changes

Added more details

fd721ba

tigrato reviewed Mar 20, 2025

View reviewed changes

Reversion 3

04d826d

rosstimothy reviewed Mar 20, 2025

View reviewed changes

arazmj added 2 commits March 20, 2025 15:31

reversion 4

3f7799e

reversion 4

088c485

timothyb89 reviewed Mar 21, 2025

View reviewed changes

reversion 5

5bea0b8

arazmj merged commit 3a03702 into main Mar 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design Document #1

Design Document #1

arazmj commented Mar 20, 2025

tigrato Mar 20, 2025

arazmj Mar 20, 2025

tigrato Mar 20, 2025

arazmj Mar 20, 2025

timothyb89 Mar 20, 2025

arazmj Mar 22, 2025

tigrato Mar 20, 2025

arazmj Mar 20, 2025 •

edited

Loading

timothyb89 Mar 20, 2025

arazmj Mar 22, 2025

tigrato Mar 20, 2025

arazmj Mar 20, 2025 •

edited

Loading

rosstimothy Mar 20, 2025

arazmj Mar 20, 2025

rosstimothy Mar 20, 2025

rosstimothy Mar 21, 2025

arazmj Mar 22, 2025

rosstimothy Mar 20, 2025

arazmj Mar 20, 2025

rosstimothy Mar 20, 2025

arazmj Mar 22, 2025

timothyb89 Mar 20, 2025

timothyb89 Mar 20, 2025

timothyb89 Mar 21, 2025

arazmj Mar 21, 2025

timothyb89 Mar 21, 2025 •

edited

Loading

arazmj Mar 21, 2025

timothyb89 Mar 21, 2025

arazmj Mar 21, 2025

arazmj Mar 22, 2025

timothyb89 Mar 22, 2025

arazmj Mar 22, 2025 •

edited

Loading

timothyb89 Mar 21, 2025

arazmj Mar 21, 2025


		GetJobStatus(JobStatusRequest) returns (JobStatusResponse)

		GetJobLogs(JobLogsRequest) returns (JobLogsResponse)

Design Document #1

Design Document #1

Conversation

arazmj commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arazmj Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arazmj Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timothyb89 Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arazmj Mar 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arazmj Mar 20, 2025 •

edited

Loading

arazmj Mar 20, 2025 •

edited

Loading

timothyb89 Mar 21, 2025 •

edited

Loading

arazmj Mar 22, 2025 •

edited

Loading