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

Milestone 2 - Server and Client #2

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*.dll
*.so
*.dylib
bin/*

# Test binary, built with `go test -c`
*.test
Expand Down
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,27 @@

# BitBox
Rudementary implementation of containerization in linux.


## Server
For available endpoints view the [gRPC deffinition](grpc/bitbox.proto).
```bash
# Build the Server.
go build -o bin/bitbox cmd/server/main.go
# Run it.
./bin/bitbox
```


## CLI Client
```bash
# Build the CLI
go build -o bin/bitboxcli cmd/client/*.go
# Run it.
./bin/bitboxcli start <process>
```

## Development Tools
[gRPC & protoc](https://grpc.io/docs/languages/go/quickstart/) are used by `go generate` to update [bitbox/grpc](grpc/).

[gRPCox](https://github.com/gusaul/grpcox) is a lightweight docker container for easy manual testing.
75 changes: 75 additions & 0 deletions cmd/client/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package main

import (
"context"
"fmt"
"log"
"net"
"os"

bbgrpc "github.com/jmbarzee/bitbox/grpc"
"github.com/spf13/cobra"
"google.golang.org/grpc"
)

var defaultPort = optionalEnvString("BIT_BOX_PORT", "8443")

var defaultAddress = optionalEnvString("BIT_BOX_ADDR", func() string {
ip, err := getOutboundIP()
if err != nil {
panic(err)
}
return ip.String()
}())

var root = &cobra.Command{
Use: "bitboxcli",
Short: "A CLI tool for remote BitBox operations",
Long: "Execute remote linux processes on a BitBox server",
}

func main() {
root.AddCommand(cmdStart)
root.AddCommand(cmdStop)
root.AddCommand(cmdStatus)
root.AddCommand(cmdQuery)
if err := root.Execute(); err != nil {
panic(err)
}
}

func optionalEnvString(key, defaultValue string) string {
if value, ok := os.LookupEnv(key); ok {
return value
}
log.Printf("Missing optional environment variable %s, using default %s", key, defaultValue)
return defaultValue
}

func getOutboundIP() (net.IP, error) {
conn, err := net.Dial("udp", "8.8.8.8:80")

Choose a reason for hiding this comment

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

that's assuming the client is on a public network
if you're sure you want to go this route, it'd be better to loop through the network interfaces and get your ip from there

Copy link

Choose a reason for hiding this comment

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

The way this is ultimately used is as a target for the outbound grpc connection on line 66. I'm not sure a public interface makes more sense than loopback if the client is one the same box as the server -- and using loopback would cut all of this code. If the client isn't on the same box as the server, there is no sane default -- so loopback or making this required seems logical. IMO loopback is fine since that is probably how most of the development/testing will be done in this project.

if err != nil {
return net.IP{}, err
}
defer conn.Close()

localAddr := conn.LocalAddr().(*net.UDPAddr)

return localAddr.IP, nil
}

func getClient(ctx context.Context) bbgrpc.BitBoxClient {
address := fmt.Sprintf("%s:%s", defaultAddress, defaultPort)

conn, err := grpc.DialContext(
context.TODO(),
address,
//TODO: replace with mTLS
grpc.WithInsecure(),
grpc.WithBlock())
if err != nil {
panic(fmt.Errorf("Failed to dial connection during reconnect: %w", err))
}

return bbgrpc.NewBitBoxClient(conn)
}
70 changes: 70 additions & 0 deletions cmd/client/query.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package main

import (
"context"
"errors"
"fmt"
"io"
"log"

"github.com/google/uuid"
"github.com/spf13/cobra"

bbgrpc "github.com/jmbarzee/bitbox/grpc"
)

var cmdQuery = &cobra.Command{
Use: "query",
Short: "query",
Long: "Query a process on the bitbox server",
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) != 1 {
return errors.New("Require a single id as an argument")
}

uuid, err := uuid.Parse(args[0])
if err != nil {
return fmt.Errorf("failed to parse uuid: %s", args[0])
}

job := jobQuery{
id: uuid,
}
ctx := context.Background()
bbClient := getClient(ctx)
return job.execute(ctx, bbClient)
},
}

type jobQuery struct {
id uuid.UUID
}

// Execute querys a job on the remote bitBox
func (j jobQuery) execute(ctx context.Context, c bbgrpc.BitBoxClient) error {
request := &bbgrpc.QueryRequest{
ID: j.id[:],
}

queryClient, err := c.Query(ctx, request)
if err != nil {
return fmt.Errorf("failed to stop process %s: %w", j.id, err)
}

for {
reply, err := queryClient.Recv()
if err == io.EOF {
log.Println("<End of Stream>")
}
if err != nil {
return fmt.Errorf("failed to fetch reply: %w", err)
}
switch output := reply.GetOutput().(type) {
case *bbgrpc.QueryReply_Stdouterr:
log.Println(output.Stdouterr)
case *bbgrpc.QueryReply_ExitCode:
log.Printf("Process %v exited with code %v", j.id, output.ExitCode)
return nil
}
}
}
56 changes: 56 additions & 0 deletions cmd/client/start.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package main

import (
"context"
"errors"
"fmt"
"log"

"github.com/google/uuid"
"github.com/spf13/cobra"

bbgrpc "github.com/jmbarzee/bitbox/grpc"
)

var cmdStart = &cobra.Command{
Use: "start",
Short: "start",
Long: "Start a process on the bitbox server",
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) < 1 {
return errors.New("Require atleast a single command as an argument")
}

job := jobStart{
command: args[0],
arguments: args[1:],
}
Comment on lines +24 to +27

Choose a reason for hiding this comment

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

have you tried this with implicitly invoking a shell?
example /bin/bash -c "ls -lah ~"

ctx := context.Background()
bbClient := getClient(ctx)
return job.execute(ctx, bbClient)
},
}

type jobStart struct {
command string
arguments []string
}

// Execute starts a job on the remote bitBox
func (j jobStart) execute(ctx context.Context, c bbgrpc.BitBoxClient) error {
request := &bbgrpc.StartRequest{
Command: j.command,
Arguments: j.arguments,
}

reply, err := c.Start(ctx, request)
if err != nil {
return fmt.Errorf("failed to run %s: %w", j.command, err)
}
uuid, err := uuid.FromBytes(reply.GetID())
jmbarzee marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return fmt.Errorf("failed to parse uuid: %s", reply.GetID())
}
log.Println("Successfully started process: ", uuid.String())
return nil
}
55 changes: 55 additions & 0 deletions cmd/client/status.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package main

import (
"context"
"errors"
"fmt"
"log"

"github.com/google/uuid"
"github.com/spf13/cobra"

bbgrpc "github.com/jmbarzee/bitbox/grpc"
)

var cmdStatus = &cobra.Command{
Use: "status",
Short: "status",
Long: "Stop a process on the bitbox server",
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) != 1 {
return errors.New("Require a single id as an argument")
}

uuid, err := uuid.Parse(args[0])
if err != nil {
return fmt.Errorf("failed to parse uuid: %s", args[0])
}

job := jobStatus{
id: uuid,
}
ctx := context.Background()
bbClient := getClient(ctx)
return job.execute(ctx, bbClient)
},
}

type jobStatus struct {
id uuid.UUID
}

// Execute returns the status of a job on the remote bitBox
func (j jobStatus) execute(ctx context.Context, c bbgrpc.BitBoxClient) error {
request := &bbgrpc.StatusRequest{
ID: j.id[:],
}

reply, err := c.Status(ctx, request)
if err != nil {
return fmt.Errorf("failed to query process %s: %w", j.id, err)
}
log.Println("Successfully queried status of process: ", j.id, ", ", reply.Status.String())

return nil
}
54 changes: 54 additions & 0 deletions cmd/client/stop.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package main

import (
"context"
"errors"
"fmt"
"log"

"github.com/google/uuid"
"github.com/spf13/cobra"

bbgrpc "github.com/jmbarzee/bitbox/grpc"
)

var cmdStop = &cobra.Command{
Use: "stop",
Short: "stop",
Long: "Stop a process on the bitbox server",
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) != 1 {
return errors.New("Require a single id as an argument")
}

uuid, err := uuid.Parse(args[0])
if err != nil {
return fmt.Errorf("failed to parse uuid: %s", args[0])
}

job := jobStop{
id: uuid,
}
ctx := context.Background()
bbClient := getClient(ctx)
return job.execute(ctx, bbClient)
},
}

type jobStop struct {
id uuid.UUID
}
Comment on lines +38 to +40
Copy link

Choose a reason for hiding this comment

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

Why do you chose a struct and a method here, as opposed to a function or inlining? I ask here, but this could apply to any of the client side files that follow the same pattern.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's a pattern that I use in many of the projects I work on, though the job here is so lightweight that it doesn't really display the value of the pattern. I'll try to give you a picture of why this is valuable as the complexity of the command/job scales by explaining the responsibility of each part.

  • cmdStop.Run is mostly responsible for parsing input of the cli/user into the format required for the job, but it also is the entry point for the job operation. It may eventually handle different forms or sources of input.
  • jobStop is an enumeration of the data/type that the job needs to execute.
  • jobStop.execute is the definition of the job's execution. It pays no mind to what form the input was in or what source it came from, only that it's in jobStop

To be clear, I don't think I would write this code this way the first time, but experience with this library has encouraged me to make the division of responsibility this way.

Copy link

@NajiObeid NajiObeid Mar 24, 2021

Choose a reason for hiding this comment

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

For me where this starts to become awkward is when you start writing tests for the client.
Obviously we trust grpc so we might not be interested in testing their code, so we'd want some level of separation between the cli itself and the server calls, in that way we can properly write tests for the UI while using mock server calls.

Separation also comes with other benefits such as using the same cli to make requests using different technologies, such as rest...

Copy link
Owner Author

Choose a reason for hiding this comment

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

For me where this starts to become awkward is when you start writing tests for the client.
Would you clarify what "this" is? I don't know if I understand what you are advocating for.

The client package does depend on BitBoxClient, which is an interface. Would you like tests on the clients?

Choose a reason for hiding this comment

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

By this I meant It's a pattern that I use

I did say awkward not impossible. Ideally you wouldn't want to rework your cli tests, or your cli as a matter of fact, if api changes occurred for example. You also might find yourself held hostage by a third party library if you start mocking their interfaces. It's always good to have a layer of separation between the two.
I can keep on going with why this is not ideal.
This might be totally flying over my head, please correct me if I am wrong.

And no I wouldn't want you to write tests other than the ones you already planned on doing.


// Execute stops a job on the remote bitBox
func (j jobStop) execute(ctx context.Context, c bbgrpc.BitBoxClient) error {
request := &bbgrpc.StopRequest{
ID: j.id[:],
}

_, err := c.Stop(ctx, request)
if err != nil {
return fmt.Errorf("failed to stop process %s: %w", j.id, err)
}
log.Println("Successfully stopped process: ", j.id)
return nil
}
45 changes: 45 additions & 0 deletions cmd/server/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package main

import (
// temp import for build
"fmt"
"log"
"net"
"os"

"github.com/jmbarzee/bitbox"
bbgrpc "github.com/jmbarzee/bitbox/grpc"
"google.golang.org/grpc"
"google.golang.org/grpc/reflection"
)

var defaultPort = optionalEnvString("BIT_BOX_PORT", "8443")
var defaultAddress = optionalEnvString("BIT_BOX_ADDR", "")

func main() {

address := fmt.Sprintf("%s:%s", defaultAddress, defaultPort)

lis, err := net.Listen("tcp", address)
if err != nil {
panic(fmt.Errorf("failed to listen on %s: %w", address, err))
}

bitBoxServer := bitbox.NewServer()

server := grpc.NewServer()

bbgrpc.RegisterBitBoxServer(server, bitBoxServer)
// Register reflection service on gRPC server.
reflection.Register(server)

err = server.Serve(lis)
}

func optionalEnvString(key, defaultValue string) string {
if value, ok := os.LookupEnv(key); ok {
return value
}
log.Printf("Missing optional environment variable %s, using default %s", key, defaultValue)
return defaultValue
}
Loading