Skip to content

Commit 05b0a4a

Browse files
henrybarretogustavosbarreto
authored andcommitted
refactor(ssh): change device lookup params to modern naming
Previously, the device lookup API accepted `domain` and `name` parameters to represent the namespace and device name, respectively. This outdated naming caused confusion when reading and modifying the code. To simplify the implementation and improve readability, we’ve renamed them to match the modern naming for these values,`namespace` and `device`, clarifying intent throughout the codebase and avoiding misunderstanding. This change only affects the internal API.
1 parent 0f9787d commit 05b0a4a

File tree

9 files changed

+77
-157
lines changed

9 files changed

+77
-157
lines changed

api/routes/device.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const (
1717
DeleteDeviceURL = "/devices/:uid"
1818
RenameDeviceURL = "/devices/:uid"
1919
OfflineDeviceURL = "/devices/:uid/offline"
20-
LookupDeviceURL = "/lookup"
20+
LookupDeviceURL = "/device/lookup"
2121
UpdateDeviceStatusURL = "/devices/:uid/:status"
2222
CreateTagURL = "/devices/:uid/tags" // Add a tag to a device.
2323
UpdateTagURL = "/devices/:uid/tags" // Update device's tags with a new set.
@@ -206,7 +206,7 @@ func (h *Handler) LookupDevice(c gateway.Context) error {
206206
return err
207207
}
208208

209-
device, err := h.service.LookupDevice(c.Ctx(), req.Domain, req.Name)
209+
device, err := h.service.LookupDevice(c.Ctx(), req.TenantID, req.Name)
210210
if err != nil {
211211
return err
212212
}

api/routes/device_test.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -446,11 +446,8 @@ func TestLookupDevice(t *testing.T) {
446446
expected Expected
447447
}{
448448
{
449-
title: "fails when bind fails to validate uid",
450-
request: requests.DeviceLookup{
451-
Username: "user1",
452-
IPAddress: "192.168.1.100",
453-
},
449+
title: "fails when bind fails to validate uid",
450+
request: requests.DeviceLookup{},
454451
requiredMocks: func(_ requests.DeviceLookup) {},
455452
expected: Expected{
456453
expectedSession: nil,
@@ -460,13 +457,11 @@ func TestLookupDevice(t *testing.T) {
460457
{
461458
title: "fails when try to look up of a existing device",
462459
request: requests.DeviceLookup{
463-
Domain: "example.com",
464-
Name: "device1",
465-
Username: "user1",
466-
IPAddress: "192.168.1.100",
460+
TenantID: "example.com",
461+
Name: "device1",
467462
},
468463
requiredMocks: func(req requests.DeviceLookup) {
469-
mock.On("LookupDevice", gomock.Anything, req.Domain, req.Name).Return(nil, svc.ErrDeviceNotFound).Once()
464+
mock.On("LookupDevice", gomock.Anything, req.TenantID, req.Name).Return(nil, svc.ErrDeviceNotFound).Once()
470465
},
471466
expected: Expected{
472467
expectedSession: nil,
@@ -476,13 +471,11 @@ func TestLookupDevice(t *testing.T) {
476471
{
477472
title: "success when try to look up of a existing device",
478473
request: requests.DeviceLookup{
479-
Domain: "example.com",
480-
Name: "device1",
481-
Username: "user1",
482-
IPAddress: "192.168.1.100",
474+
TenantID: "example.com",
475+
Name: "device1",
483476
},
484477
requiredMocks: func(req requests.DeviceLookup) {
485-
mock.On("LookupDevice", gomock.Anything, req.Domain, req.Name).Return(&models.Device{}, nil)
478+
mock.On("LookupDevice", gomock.Anything, req.TenantID, req.Name).Return(&models.Device{}, nil)
486479
},
487480
expected: Expected{
488481
expectedSession: &models.Device{},
@@ -500,7 +493,7 @@ func TestLookupDevice(t *testing.T) {
500493
assert.NoError(t, err)
501494
}
502495

503-
req := httptest.NewRequest(http.MethodGet, "/internal/lookup", strings.NewReader(string(jsonData)))
496+
req := httptest.NewRequest(http.MethodGet, "/internal/device/lookup", strings.NewReader(string(jsonData)))
504497
req.Header.Set("Content-Type", "application/json")
505498
req.Header.Set("X-Role", authorizer.RoleOwner.String())
506499
rec := httptest.NewRecorder()

api/services/mocks/services.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/store/mocks/store.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/internalclient/device.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type deviceAPI interface {
2828
Lookup(lookup map[string]string) (string, []error)
2929

3030
// DeviceLookup performs a lookup operation based on the provided parameters.
31-
DeviceLookup(lookup map[string]string) (*models.Device, []error)
31+
DeviceLookup(tenantID, name string) (*models.Device, error)
3232

3333
// LookupTunnel gets a tunnel from its addrss.
3434
// TODO: Create a API interface for Tunnel routes.
@@ -68,23 +68,28 @@ func (c *client) Lookup(lookup map[string]string) (string, []error) {
6868
return device.UID, nil
6969
}
7070

71-
func (c *client) DeviceLookup(lookup map[string]string) (*models.Device, []error) {
71+
func (c *client) DeviceLookup(tenantID, name string) (*models.Device, error) {
7272
device := new(models.Device)
73-
7473
resp, err := c.http.
7574
R().
76-
SetQueryParams(lookup).
75+
SetQueryParam("tenant_id", tenantID).
76+
SetQueryParam("name", name).
7777
SetResult(&device).
78-
Get("/internal/lookup")
78+
Get("/internal/device/lookup")
7979
if err != nil {
80-
return nil, []error{err}
80+
return nil, ErrConnectionFailed
8181
}
8282

83-
if resp.StatusCode() != http.StatusOK {
84-
return nil, []error{errors.New("fail to get the device from the API")}
83+
switch resp.StatusCode() {
84+
case http.StatusOK:
85+
return device, nil
86+
case http.StatusNotFound:
87+
return nil, ErrNotFound
88+
case http.StatusForbidden:
89+
return nil, ErrForbidden
90+
default:
91+
return nil, ErrUnknown
8592
}
86-
87-
return device, nil
8893
}
8994

9095
func (c *client) ListDevices() ([]models.Device, error) {

pkg/api/internalclient/mocks/internalclient.go

Lines changed: 11 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/requests/device.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,8 @@ type DeviceOffline struct {
5353

5454
// DeviceLookup is the structure to represent the request data for lookup device endpoint.
5555
type DeviceLookup struct {
56-
Domain string `query:"domain" validate:"required"`
57-
Name string `query:"name" validate:"required"`
58-
Username string `query:"username" validate:""`
59-
IPAddress string `query:"ip_address" validate:""`
56+
TenantID string `query:"tenant_id" validate:"required"`
57+
Name string `query:"name" validate:"required"`
6058
}
6159

6260
// DeviceStatus is the structure to represent the request data for update device status to pending endpoint.

ssh/session/session.go

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ type Data struct {
3838
Type string
3939
// Term is the terminal used for the client.
4040
Term string
41-
// TODO:
42-
Lookup map[string]string
4341
// Handled check if the session is already handling a "shell", "exec" or a "subsystem".
4442
Handled bool
4543
}
@@ -260,9 +258,9 @@ func NewSession(ctx gliderssh.Context, tunnel *httptunnel.Tunnel, cache cache.Ca
260258
return nil, err
261259
}
262260

263-
var domain, hostname string
261+
var namespaceName, deviceName string
264262
if target.IsSSHID() {
265-
domain, hostname, err = target.SplitSSHID()
263+
namespaceName, deviceName, err = target.SplitSSHID()
266264
if err != nil {
267265
return nil, err
268266
}
@@ -294,21 +292,16 @@ func NewSession(ctx gliderssh.Context, tunnel *httptunnel.Tunnel, cache cache.Ca
294292
return nil, err
295293
}
296294

297-
domain = device.Namespace
298-
hostname = device.Name
295+
namespaceName = device.Namespace
296+
deviceName = device.Name
299297
}
300298

301-
lookup := map[string]string{
302-
"domain": domain,
303-
"name": hostname,
304-
}
305-
306-
device, errs := api.DeviceLookup(lookup)
307-
if len(errs) > 0 {
308-
return nil, errs[0]
299+
lookupDevice, err := api.DeviceLookup(namespaceName, deviceName)
300+
if err != nil {
301+
return nil, err
309302
}
310303

311-
namespace, errs := api.NamespaceLookup(device.TenantID)
304+
namespace, errs := api.NamespaceLookup(lookupDevice.TenantID)
312305
if len(errs) > 1 {
313306
return nil, errs[0]
314307
}
@@ -331,10 +324,9 @@ func NewSession(ctx gliderssh.Context, tunnel *httptunnel.Tunnel, cache cache.Ca
331324
Data: Data{
332325
IPAddress: hos.Host,
333326
Target: target,
334-
Device: device,
327+
Device: lookupDevice,
335328
Namespace: namespace,
336-
Lookup: lookup,
337-
SSHID: fmt.Sprintf("%s@%s.%s", target.Username, domain, hostname),
329+
SSHID: fmt.Sprintf("%s@%s.%s", target.Username, namespaceName, deviceName),
338330
},
339331
once: new(sync.Once),
340332
Seats: NewSeats(),
@@ -346,9 +338,6 @@ func NewSession(ctx gliderssh.Context, tunnel *httptunnel.Tunnel, cache cache.Ca
346338
},
347339
}
348340

349-
session.Data.Lookup["username"] = target.Username
350-
session.Data.Lookup["ip_address"] = hos.Host
351-
352341
snap.save(session, StateCreated)
353342

354343
return session, nil
@@ -397,7 +386,13 @@ func (s *Session) NewAgentChannel(name string, seat int) (*AgentChannel, error)
397386
}
398387

399388
func (s *Session) checkFirewall() (bool, error) {
400-
if err := s.api.FirewallEvaluate(s.Data.Lookup); err != nil {
389+
// TODO: Refactor firewall evaluation to remove the map requirement.
390+
if err := s.api.FirewallEvaluate(map[string]string{
391+
"domain": s.Namespace.Name,
392+
"name": s.Device.Name,
393+
"username": s.Target.Username,
394+
"ip_address": s.IPAddress,
395+
}); err != nil {
401396
defer log.WithError(err).WithFields(log.Fields{
402397
"uid": s.UID,
403398
"sshid": s.SSHID,

0 commit comments

Comments
 (0)