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

RSDK-9194: streamline app connection #4516

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
94 changes: 94 additions & 0 deletions app/viam_client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Package app contains all logic needed for communication and interaction with app.
package app

import (
"context"
"errors"
"net/url"
"regexp"
"strings"

"go.viam.com/utils/rpc"

"go.viam.com/rdk/logging"
)

// ViamClient is a gRPC client for method calls to Viam app.
type ViamClient struct {
conn rpc.ClientConn
}

// Options has the options necessary to connect through gRPC.
type Options struct {
baseURL string
entity string
credentials rpc.Credentials
}

// APIKeyOptions has what is necessary to connect through GRPC with an API key.
type APIKeyOptions struct {
baseURL string
apiKey string
apiKeyID string
}
Comment on lines +28 to +33
Copy link
Member

Choose a reason for hiding this comment

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

Delete this


var dialDirectGRPC = rpc.DialDirectGRPC

// CreateViamClientWithOptions creates a ViamClient with an Options struct.
func CreateViamClientWithOptions(ctx context.Context, options Options, logger logging.Logger) (*ViamClient, error) {
if options.baseURL == "" {
Copy link
Member

Choose a reason for hiding this comment

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

Does this work if options.baseURL isn't specified? The reason we went with options here is so that people don't have to specify an empty string.

options.baseURL = "https://app.viam.com"
} else if !strings.HasPrefix(options.baseURL, "http://") && !strings.HasPrefix(options.baseURL, "https://") {
return nil, errors.New("use valid URL")
}
serviceHost, err := url.Parse(options.baseURL + ":443")
if err != nil {
return nil, err
}

if options.credentials.Payload == "" || options.entity == "" {
return nil, errors.New("entity and payload cannot be empty")
}
opts := rpc.WithEntityCredentials(options.entity, options.credentials)

conn, err := dialDirectGRPC(ctx, serviceHost.Host, logger, opts)
if err != nil {
return nil, err
}
return &ViamClient{conn: conn}, nil
}

// CreateViamClientWithAPIKey creates a ViamClient with an API key.
func CreateViamClientWithAPIKey(ctx context.Context, apiKeyOptions APIKeyOptions, logger logging.Logger) (*ViamClient, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This should take in normal Options and apiKey and apiKeyID

if !validateAPIKeyFormat(apiKeyOptions.apiKey) {
return nil, errors.New("API key should be a 32-char all-lowercase alphanumeric string")
}
if !validateAPIKeyIDFormat(apiKeyOptions.apiKeyID) {
return nil, errors.New("API key ID should be an all-lowercase alphanumeric string with this format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")
}

opts := Options{
baseURL: apiKeyOptions.baseURL,
entity: apiKeyOptions.apiKeyID,
credentials: rpc.Credentials{
Type: rpc.CredentialsTypeAPIKey,
Payload: apiKeyOptions.apiKey,
},
}
return CreateViamClientWithOptions(ctx, opts, logger)
}

// Close closes the gRPC connection.
func (c *ViamClient) Close() error {
return c.conn.Close()
}

func validateAPIKeyFormat(apiKey string) bool {
regex := regexp.MustCompile("^[a-z0-9]{32}$")
return regex.MatchString(apiKey)
}

func validateAPIKeyIDFormat(apiKeyID string) bool {
regex := regexp.MustCompile("^[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}")
return regex.MatchString(apiKeyID)
}
123 changes: 123 additions & 0 deletions app/viam_client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package app

import (
"context"
"testing"

"github.com/viamrobotics/webrtc/v3"
"go.viam.com/utils"
"go.viam.com/utils/rpc"
"google.golang.org/grpc"

"go.viam.com/rdk/logging"
)

var (
logger = logging.NewLogger("test")
defaultURL = "https://app.viam.com"
testAPIKey = "abcdefghijklmnopqrstuv0123456789"
testAPIKeyID = "abcd0123-ef45-gh67-ij89-klmnopqr01234567"
)

type MockConn struct{}

func (m *MockConn) Invoke(ctx context.Context, method string, args, reply any, opts ...grpc.CallOption) error {
return nil
}

func (m *MockConn) NewStream(
ctx context.Context,
desc *grpc.StreamDesc,
method string,
opts ...grpc.CallOption,
) (grpc.ClientStream, error) {
return nil, nil
}
func (m *MockConn) PeerConn() *webrtc.PeerConnection { return nil }
func (m *MockConn) Close() error { return nil }
func mockDialDirectGRPC(
ctx context.Context,
address string,
logger utils.ZapCompatibleLogger,
opts ...rpc.DialOption,
) (rpc.ClientConn, error) {
return &MockConn{}, nil
}

func TestCreateViamClientWithOptions(t *testing.T) {
urlTests := []struct {
name string
baseURL string
entity string
payload string
expectErr bool
}{
{"Default URL", defaultURL, testAPIKeyID, testAPIKey, false},
{"Default URL", defaultURL, "", "", true},
{"Default URL", defaultURL, "", testAPIKey, true},
{"Default URL", defaultURL, testAPIKeyID, "", true},
{"Empty URL", "", testAPIKeyID, testAPIKey, false},
Copy link
Member

Choose a reason for hiding this comment

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

Try wil nil and with ""

{"Valid URL", "https://test.com", testAPIKeyID, testAPIKey, false},
{"Invalid URL", "test", testAPIKey, testAPIKey, true},
}
originalDialDirectGRPC := dialDirectGRPC
dialDirectGRPC = mockDialDirectGRPC
defer func() { dialDirectGRPC = originalDialDirectGRPC }()
for _, tt := range urlTests {
t.Run(tt.name, func(t *testing.T) {
opts := Options{
baseURL: tt.baseURL,
entity: tt.entity,
credentials: rpc.Credentials{
Type: rpc.CredentialsTypeAPIKey,
Payload: tt.payload,
},
}
client, err := CreateViamClientWithOptions(context.Background(), opts, logger)
if (err != nil) != tt.expectErr {
t.Errorf("Expected error: %v, got: %v", tt.expectErr, err)
}
if !tt.expectErr {
if client == nil {
t.Error("Expected a valid client, got nil")
} else {
client.Close()
}
}
})
}
}

func TestCreateViamClientWithAPIKeyTests(t *testing.T) {
apiKeyTests := []struct {
name string
apiKey string
apiKeyID string
expectErr bool
}{
{"Valid API Key", testAPIKey, testAPIKeyID, false},
{"Empty API Key", "", testAPIKeyID, true},
{"Empty API Key ID", testAPIKey, "", true},
{"Invalid API Key", "fake", testAPIKeyID, true},
{"Invalid API Key ID", testAPIKey, "fake", true},
}
for _, tt := range apiKeyTests {
t.Run(tt.name, func(t *testing.T) {
options := APIKeyOptions{
apiKey: tt.apiKey,
apiKeyID: tt.apiKeyID,
}
client, err := CreateViamClientWithAPIKey(context.Background(), options, logger)
if (err != nil) != tt.expectErr {
t.Errorf("Expected error: %v, got: %v", tt.expectErr, err)
}
if !tt.expectErr {
if client == nil {
t.Error("Expected a valid client, got nil")
} else {
client.Close()
}
}
})
}
}
Loading