From 1067c02ff6b4b121010f1a50705ae8f21f484466 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 12:45:47 -0400
Subject: [PATCH 01/12] wip test FromReader

---
 config/reader.go               |  12 ++++
 config/reader_test.go          | 127 +++++++++++++++++++++++++++++++++
 config/testutils/fake_cloud.go |   2 +
 3 files changed, 141 insertions(+)

diff --git a/config/reader.go b/config/reader.go
index dc3918a3336..90cea9a1273 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"log"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -214,6 +215,7 @@ func readFromCloud(
 
 	// process the config
 	cfg, err := processConfigFromCloud(unprocessedConfig, logger)
+	log.Println(">>> post-process", cfg.Cloud)
 	if err != nil {
 		// If we cannot process the config from the cache we should clear it.
 		if cached {
@@ -275,6 +277,7 @@ func readFromCloud(
 		}
 	}
 
+	log.Println(">>> pre-merge", cfg.Cloud)
 	fqdn := cfg.Cloud.FQDN
 	localFQDN := cfg.Cloud.LocalFQDN
 	signalingAddress := cfg.Cloud.SignalingAddress
@@ -282,6 +285,8 @@ func readFromCloud(
 	managedBy := cfg.Cloud.ManagedBy
 	locationSecret := cfg.Cloud.LocationSecret
 	locationSecrets := cfg.Cloud.LocationSecrets
+	primaryOrgID := cfg.Cloud.PrimaryOrgID
+	locationID := cfg.Cloud.LocationID
 
 	mergeCloudConfig := func(to *Config) {
 		*to.Cloud = *cloudCfg
@@ -294,9 +299,13 @@ func readFromCloud(
 		to.Cloud.LocationSecrets = locationSecrets
 		to.Cloud.TLSCertificate = tls.certificate
 		to.Cloud.TLSPrivateKey = tls.privateKey
+		// TODO: are org-id/location-id missing?
+		to.Cloud.PrimaryOrgID = primaryOrgID
+		to.Cloud.LocationID = locationID
 	}
 
 	mergeCloudConfig(cfg)
+	log.Println(">>> post-merge", cfg.Cloud)
 	// TODO(RSDK-1960): add more tests around config caching
 	unprocessedConfig.Cloud.TLSCertificate = tls.certificate
 	unprocessedConfig.Cloud.TLSPrivateKey = tls.privateKey
@@ -398,6 +407,7 @@ func fromReader(
 
 	if shouldReadFromCloud && cfgFromDisk.Cloud != nil {
 		cfg, err := readFromCloud(ctx, cfgFromDisk, nil, true, true, logger)
+		log.Println(">>> got config from cloud", cfg.Cloud)
 		return cfg, err
 	}
 
@@ -635,6 +645,7 @@ func getFromCloudOrCache(ctx context.Context, cloudCfg *Cloud, shouldReadFromCac
 		return nil, cached, err
 	}
 
+	log.Printf(">>> got cfg from cloud OR cache %#v, %t", cfg.Cloud, cached)
 	return cfg, cached, nil
 }
 
@@ -665,6 +676,7 @@ func getFromCloudGRPC(ctx context.Context, cloudCfg *Cloud, logger logging.Logge
 		return nil, shouldCheckCacheOnFailure, err
 	}
 
+	log.Printf(">>> got cfg from cloud service %#v", cfg.Cloud)
 	return cfg, false, nil
 }
 
diff --git a/config/reader_test.go b/config/reader_test.go
index ac75e431243..5679fd525f1 100644
--- a/config/reader_test.go
+++ b/config/reader_test.go
@@ -2,16 +2,143 @@ package config
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
+	pb "go.viam.com/api/app/v1"
 	"go.viam.com/test"
 
+	"go.viam.com/rdk/config/testutils"
 	"go.viam.com/rdk/logging"
 )
 
+func TestFromReader(t *testing.T) {
+	const (
+		robotPartID = "forCachingTest"
+		secret      = testutils.FakeCredentialPayLoad
+	)
+	var (
+		logger = logging.NewTestLogger(t)
+		ctx    = context.Background()
+	)
+
+	// clear cache
+	setupClearCache := func(t *testing.T) {
+		t.Helper()
+		clearCache(robotPartID)
+		_, err := readFromCache(robotPartID)
+		test.That(t, os.IsNotExist(err), test.ShouldBeTrue)
+	}
+
+	t.Run("online", func(t *testing.T) {
+		setupClearCache(t)
+
+		fakeServer, err := testutils.NewFakeCloudServer(context.Background(), logger)
+		test.That(t, err, test.ShouldBeNil)
+		defer func() {
+			test.That(t, fakeServer.Shutdown(), test.ShouldBeNil)
+		}()
+
+		appAddress := fmt.Sprintf("http://%s", fakeServer.Addr().String())
+		expectedCloud := &Cloud{
+			ManagedBy:        "acme",
+			SignalingAddress: "abc",
+			ID:               robotPartID,
+			Secret:           secret,
+			FQDN:             "fqdn",
+			LocalFQDN:        "localFqdn",
+			TLSCertificate:   "cert",
+			TLSPrivateKey:    "key",
+			RefreshInterval:  time.Duration(10000000000),
+			LocationSecrets:  []LocationSecret{},
+			AppAddress:       appAddress,
+			LocationID:       "the-location",
+			PrimaryOrgID:     "the-primary-org",
+		}
+		cloudConfProto, err := CloudConfigToProto(expectedCloud)
+		test.That(t, err, test.ShouldBeNil)
+		protoConfig := &pb.RobotConfig{Cloud: cloudConfProto}
+		protoCertificate := &pb.CertificateResponse{
+			TlsCertificate: "cert",
+			TlsPrivateKey:  "key",
+		}
+
+		fakeServer.StoreDeviceConfig(robotPartID, protoConfig, protoCertificate)
+		defer fakeServer.Clear()
+
+		cfgText := fmt.Sprintf(`{"cloud":{"id":%q,"app_address":%q,"secret":%q}}`, robotPartID, appAddress, secret)
+		gotCfg, err := FromReader(ctx, "", strings.NewReader(cfgText), logger)
+		defer clearCache(robotPartID)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, gotCfg.Cloud, test.ShouldResemble, expectedCloud)
+
+		cachedCfg, err := readFromCache(robotPartID)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, cachedCfg.Cloud, test.ShouldResemble, expectedCloud)
+	})
+
+	// t.Run("offline", func(t *testing.T) {
+	// 	setupClearCache(t)
+	// 	newOfflineTestReader := func(
+	// 		ctx context.Context,
+	// 		cloud *Cloud,
+	// 		logger logging.Logger,
+	// 	) (*configReader, func() error) {
+	// 		return &configReader{nil}, func() error { return nil }
+	// 	}
+	//
+	// 	cloud := &Cloud{
+	// 		ManagedBy:        "acme",
+	// 		SignalingAddress: "abc",
+	// 		ID:               robotPartID,
+	// 		Secret:           "ghi",
+	// 		FQDN:             "fqdn",
+	// 		LocalFQDN:        "localFqdn",
+	// 		TLSCertificate:   "cert",
+	// 		TLSPrivateKey:    "key",
+	// 		AppAddress:       "https://app.viam.dev:443",
+	// 		LocationID:       "the-location",
+	// 		PrimaryOrgID:     "the-primary-org",
+	// 		LocationSecrets:  []LocationSecret{},
+	// 	}
+	// 	cfg := &Config{Cloud: cloud}
+	//
+	// 	// store our config to the cloud
+	// 	err := storeToCache(cfg.Cloud.ID, cfg)
+	// 	test.That(t, err, test.ShouldBeNil)
+	// 	defer clearCache(robotPartID)
+	//
+	// 	cfgText := fmt.Sprintf(`{"cloud":{"id":%q,"secret":"ghi"}}`, robotPartID)
+	// 	gotCfg, err := fromReader(ctx, "", strings.NewReader(cfgText), logger, newOfflineTestReader)
+	//
+	// 	expectedCloud := &Cloud{
+	// 		ManagedBy:        "acme",
+	// 		SignalingAddress: "abc",
+	// 		ID:               robotPartID,
+	// 		Secret:           "ghi",
+	// 		FQDN:             "fqdn",
+	// 		LocalFQDN:        "localFqdn",
+	// 		TLSCertificate:   "cert",
+	// 		TLSPrivateKey:    "key",
+	// 		RefreshInterval:  time.Duration(10000000000),
+	// 		LocationSecrets:  []LocationSecret{},
+	// 	}
+	// 	test.That(t, gotCfg.Cloud, test.ShouldResemble, expectedCloud)
+	//
+	// 	// TODO: why isn't this included in the result that comes from `fromReader`
+	// 	expectedCloud.LocationID = "the-location"
+	// 	expectedCloud.PrimaryOrgID = "the-primary-org"
+	// 	expectedCloud.AppAddress = "https://app.viam.dev:443"
+	// 	cachedCfg, err := readFromCache(robotPartID)
+	// 	test.That(t, err, test.ShouldBeNil)
+	// 	test.That(t, cachedCfg.Cloud, test.ShouldResemble, expectedCloud)
+	// })
+}
+
 func TestStoreToCache(t *testing.T) {
 	logger := logging.NewTestLogger(t)
 	ctx := context.Background()
diff --git a/config/testutils/fake_cloud.go b/config/testutils/fake_cloud.go
index e321c924695..e00dec81b55 100644
--- a/config/testutils/fake_cloud.go
+++ b/config/testutils/fake_cloud.go
@@ -4,6 +4,7 @@ package testutils
 import (
 	"context"
 	"errors"
+	"log"
 	"net"
 	"net/http"
 	"sync"
@@ -147,6 +148,7 @@ func (s *FakeCloudServer) Config(ctx context.Context, req *pb.ConfigRequest) (*p
 		return nil, status.Error(codes.NotFound, "config for device not found")
 	}
 
+	log.Println(">>> cfg response", d.cfg.Cloud)
 	return &pb.ConfigResponse{Config: d.cfg}, nil
 }
 

From aca3fdf1b2108d9f15939fa94f64612e192057b4 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:09:42 -0400
Subject: [PATCH 02/12] wip add offline test

---
 config/reader.go               |   8 +-
 config/reader_test.go          | 142 ++++++++++++++++-----------------
 config/testutils/fake_cloud.go |  25 ++++--
 3 files changed, 86 insertions(+), 89 deletions(-)

diff --git a/config/reader.go b/config/reader.go
index 90cea9a1273..41fc30282b4 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"log"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -206,6 +205,7 @@ func readFromCloud(
 	logger.Debug("reading configuration from the cloud")
 	cloudCfg := originalCfg.Cloud
 	unprocessedConfig, cached, err := getFromCloudOrCache(ctx, cloudCfg, shouldReadFromCache, logger)
+
 	if err != nil {
 		if !cached {
 			err = errors.Wrap(err, "error getting cloud config")
@@ -215,7 +215,6 @@ func readFromCloud(
 
 	// process the config
 	cfg, err := processConfigFromCloud(unprocessedConfig, logger)
-	log.Println(">>> post-process", cfg.Cloud)
 	if err != nil {
 		// If we cannot process the config from the cache we should clear it.
 		if cached {
@@ -277,7 +276,6 @@ func readFromCloud(
 		}
 	}
 
-	log.Println(">>> pre-merge", cfg.Cloud)
 	fqdn := cfg.Cloud.FQDN
 	localFQDN := cfg.Cloud.LocalFQDN
 	signalingAddress := cfg.Cloud.SignalingAddress
@@ -305,7 +303,6 @@ func readFromCloud(
 	}
 
 	mergeCloudConfig(cfg)
-	log.Println(">>> post-merge", cfg.Cloud)
 	// TODO(RSDK-1960): add more tests around config caching
 	unprocessedConfig.Cloud.TLSCertificate = tls.certificate
 	unprocessedConfig.Cloud.TLSPrivateKey = tls.privateKey
@@ -407,7 +404,6 @@ func fromReader(
 
 	if shouldReadFromCloud && cfgFromDisk.Cloud != nil {
 		cfg, err := readFromCloud(ctx, cfgFromDisk, nil, true, true, logger)
-		log.Println(">>> got config from cloud", cfg.Cloud)
 		return cfg, err
 	}
 
@@ -645,7 +641,6 @@ func getFromCloudOrCache(ctx context.Context, cloudCfg *Cloud, shouldReadFromCac
 		return nil, cached, err
 	}
 
-	log.Printf(">>> got cfg from cloud OR cache %#v, %t", cfg.Cloud, cached)
 	return cfg, cached, nil
 }
 
@@ -676,7 +671,6 @@ func getFromCloudGRPC(ctx context.Context, cloudCfg *Cloud, logger logging.Logge
 		return nil, shouldCheckCacheOnFailure, err
 	}
 
-	log.Printf(">>> got cfg from cloud service %#v", cfg.Cloud)
 	return cfg, false, nil
 }
 
diff --git a/config/reader_test.go b/config/reader_test.go
index 5679fd525f1..296cde19da2 100644
--- a/config/reader_test.go
+++ b/config/reader_test.go
@@ -34,109 +34,103 @@ func TestFromReader(t *testing.T) {
 		test.That(t, os.IsNotExist(err), test.ShouldBeTrue)
 	}
 
-	t.Run("online", func(t *testing.T) {
-		setupClearCache(t)
+	setupFakeServer := func(t *testing.T) (*testutils.FakeCloudServer, func()) {
+		t.Helper()
+
+		var logger = logging.NewTestLogger(t)
 
 		fakeServer, err := testutils.NewFakeCloudServer(context.Background(), logger)
 		test.That(t, err, test.ShouldBeNil)
-		defer func() {
+		cleanup := func() {
 			test.That(t, fakeServer.Shutdown(), test.ShouldBeNil)
-		}()
+		}
 
-		appAddress := fmt.Sprintf("http://%s", fakeServer.Addr().String())
-		expectedCloud := &Cloud{
+		return fakeServer, cleanup
+	}
+
+	t.Run("online", func(t *testing.T) {
+		setupClearCache(t)
+
+		fakeServer, cleanup := setupFakeServer(t)
+		defer cleanup()
+
+		cloudResponse := &Cloud{
 			ManagedBy:        "acme",
 			SignalingAddress: "abc",
 			ID:               robotPartID,
 			Secret:           secret,
 			FQDN:             "fqdn",
 			LocalFQDN:        "localFqdn",
-			TLSCertificate:   "cert",
-			TLSPrivateKey:    "key",
-			RefreshInterval:  time.Duration(10000000000),
 			LocationSecrets:  []LocationSecret{},
-			AppAddress:       appAddress,
 			LocationID:       "the-location",
 			PrimaryOrgID:     "the-primary-org",
 		}
-		cloudConfProto, err := CloudConfigToProto(expectedCloud)
-		test.That(t, err, test.ShouldBeNil)
-		protoConfig := &pb.RobotConfig{Cloud: cloudConfProto}
-		protoCertificate := &pb.CertificateResponse{
+		certProto := &pb.CertificateResponse{
 			TlsCertificate: "cert",
 			TlsPrivateKey:  "key",
 		}
 
-		fakeServer.StoreDeviceConfig(robotPartID, protoConfig, protoCertificate)
-		defer fakeServer.Clear()
+		cloudConfProto, err := CloudConfigToProto(cloudResponse)
+		test.That(t, err, test.ShouldBeNil)
+		protoConfig := &pb.RobotConfig{Cloud: cloudConfProto}
+		fakeServer.StoreDeviceConfig(robotPartID, protoConfig, certProto)
 
+		appAddress := fmt.Sprintf("http://%s", fakeServer.Addr().String())
 		cfgText := fmt.Sprintf(`{"cloud":{"id":%q,"app_address":%q,"secret":%q}}`, robotPartID, appAddress, secret)
 		gotCfg, err := FromReader(ctx, "", strings.NewReader(cfgText), logger)
 		defer clearCache(robotPartID)
 		test.That(t, err, test.ShouldBeNil)
-		test.That(t, gotCfg.Cloud, test.ShouldResemble, expectedCloud)
+
+		expectedCloud := *cloudResponse
+		expectedCloud.AppAddress = appAddress
+		expectedCloud.TLSCertificate = certProto.TlsCertificate
+		expectedCloud.TLSPrivateKey = certProto.TlsPrivateKey
+		expectedCloud.RefreshInterval = time.Duration(10000000000)
+		test.That(t, gotCfg.Cloud, test.ShouldResemble, &expectedCloud)
 
 		cachedCfg, err := readFromCache(robotPartID)
 		test.That(t, err, test.ShouldBeNil)
-		test.That(t, cachedCfg.Cloud, test.ShouldResemble, expectedCloud)
+		expectedCloud.AppAddress = ""
+		test.That(t, cachedCfg.Cloud, test.ShouldResemble, &expectedCloud)
 	})
 
-	// t.Run("offline", func(t *testing.T) {
-	// 	setupClearCache(t)
-	// 	newOfflineTestReader := func(
-	// 		ctx context.Context,
-	// 		cloud *Cloud,
-	// 		logger logging.Logger,
-	// 	) (*configReader, func() error) {
-	// 		return &configReader{nil}, func() error { return nil }
-	// 	}
-	//
-	// 	cloud := &Cloud{
-	// 		ManagedBy:        "acme",
-	// 		SignalingAddress: "abc",
-	// 		ID:               robotPartID,
-	// 		Secret:           "ghi",
-	// 		FQDN:             "fqdn",
-	// 		LocalFQDN:        "localFqdn",
-	// 		TLSCertificate:   "cert",
-	// 		TLSPrivateKey:    "key",
-	// 		AppAddress:       "https://app.viam.dev:443",
-	// 		LocationID:       "the-location",
-	// 		PrimaryOrgID:     "the-primary-org",
-	// 		LocationSecrets:  []LocationSecret{},
-	// 	}
-	// 	cfg := &Config{Cloud: cloud}
-	//
-	// 	// store our config to the cloud
-	// 	err := storeToCache(cfg.Cloud.ID, cfg)
-	// 	test.That(t, err, test.ShouldBeNil)
-	// 	defer clearCache(robotPartID)
-	//
-	// 	cfgText := fmt.Sprintf(`{"cloud":{"id":%q,"secret":"ghi"}}`, robotPartID)
-	// 	gotCfg, err := fromReader(ctx, "", strings.NewReader(cfgText), logger, newOfflineTestReader)
-	//
-	// 	expectedCloud := &Cloud{
-	// 		ManagedBy:        "acme",
-	// 		SignalingAddress: "abc",
-	// 		ID:               robotPartID,
-	// 		Secret:           "ghi",
-	// 		FQDN:             "fqdn",
-	// 		LocalFQDN:        "localFqdn",
-	// 		TLSCertificate:   "cert",
-	// 		TLSPrivateKey:    "key",
-	// 		RefreshInterval:  time.Duration(10000000000),
-	// 		LocationSecrets:  []LocationSecret{},
-	// 	}
-	// 	test.That(t, gotCfg.Cloud, test.ShouldResemble, expectedCloud)
-	//
-	// 	// TODO: why isn't this included in the result that comes from `fromReader`
-	// 	expectedCloud.LocationID = "the-location"
-	// 	expectedCloud.PrimaryOrgID = "the-primary-org"
-	// 	expectedCloud.AppAddress = "https://app.viam.dev:443"
-	// 	cachedCfg, err := readFromCache(robotPartID)
-	// 	test.That(t, err, test.ShouldBeNil)
-	// 	test.That(t, cachedCfg.Cloud, test.ShouldResemble, expectedCloud)
-	// })
+	t.Run("offline with cached config", func(t *testing.T) {
+		setupClearCache(t)
+
+		cachedCloud := &Cloud{
+			ManagedBy:        "acme",
+			SignalingAddress: "abc",
+			ID:               robotPartID,
+			Secret:           secret,
+			FQDN:             "fqdn",
+			LocalFQDN:        "localFqdn",
+			TLSCertificate:   "cert",
+			TLSPrivateKey:    "key",
+			LocationID:       "the-location",
+			PrimaryOrgID:     "the-primary-org",
+		}
+		cachedConf := &Config{Cloud: cachedCloud}
+		err := storeToCache(robotPartID, cachedConf)
+		test.That(t, err, test.ShouldBeNil)
+		defer clearCache(robotPartID)
+
+		fakeServer, cleanup := setupFakeServer(t)
+		defer cleanup()
+		fakeServer.FailOnConfigAndCertsWith(context.DeadlineExceeded)
+		fakeServer.StoreDeviceConfig(robotPartID, nil, nil)
+
+		appAddress := fmt.Sprintf("http://%s", fakeServer.Addr().String())
+		cfgText := fmt.Sprintf(`{"cloud":{"id":%q,"app_address":%q,"secret":%q}}`, robotPartID, appAddress, secret)
+		gotCfg, err := FromReader(ctx, "", strings.NewReader(cfgText), logger)
+		test.That(t, err, test.ShouldBeNil)
+
+		expectedCloud := *cachedCloud
+		expectedCloud.AppAddress = appAddress
+		expectedCloud.TLSCertificate = "cert"
+		expectedCloud.TLSPrivateKey = "key"
+		expectedCloud.RefreshInterval = time.Duration(10000000000)
+		test.That(t, gotCfg.Cloud, test.ShouldResemble, &expectedCloud)
+	})
 }
 
 func TestStoreToCache(t *testing.T) {
diff --git a/config/testutils/fake_cloud.go b/config/testutils/fake_cloud.go
index e00dec81b55..08d220771e6 100644
--- a/config/testutils/fake_cloud.go
+++ b/config/testutils/fake_cloud.go
@@ -4,7 +4,6 @@ package testutils
 import (
 	"context"
 	"errors"
-	"log"
 	"net"
 	"net/http"
 	"sync"
@@ -32,7 +31,7 @@ type FakeCloudServer struct {
 
 	deviceConfigs map[string]*configAndCerts
 
-	failOnConfigAndCerts bool
+	errConfigAndCerts error
 
 	mu sync.Mutex
 }
@@ -93,9 +92,20 @@ func NewFakeCloudServer(ctx context.Context, logger logging.Logger) (*FakeCloudS
 // FailOnConfigAndCerts if `failure` is true the server will return an Internal error on
 // all certficate and config requests.
 func (s *FakeCloudServer) FailOnConfigAndCerts(failure bool) {
+	if failure {
+		s.FailOnConfigAndCertsWith(status.Error(codes.Internal, "oops failure"))
+	} else {
+		s.FailOnConfigAndCertsWith(nil)
+	}
+}
+
+// FailOnConfigAndCertsWith will cause the server to return a given `error` on all
+// certficate and config requests. If `error == nil` then certficate and config
+// requests will succeed.
+func (s *FakeCloudServer) FailOnConfigAndCertsWith(err error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.failOnConfigAndCerts = failure
+	s.errConfigAndCerts = err
 }
 
 // Addr returns the listeners address.
@@ -139,8 +149,8 @@ func (s *FakeCloudServer) Config(ctx context.Context, req *pb.ConfigRequest) (*p
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if s.failOnConfigAndCerts {
-		return nil, status.Error(codes.Internal, "oops failure")
+	if s.errConfigAndCerts != nil {
+		return nil, s.errConfigAndCerts
 	}
 
 	d, ok := s.deviceConfigs[req.Id]
@@ -148,7 +158,6 @@ func (s *FakeCloudServer) Config(ctx context.Context, req *pb.ConfigRequest) (*p
 		return nil, status.Error(codes.NotFound, "config for device not found")
 	}
 
-	log.Println(">>> cfg response", d.cfg.Cloud)
 	return &pb.ConfigResponse{Config: d.cfg}, nil
 }
 
@@ -157,8 +166,8 @@ func (s *FakeCloudServer) Certificate(ctx context.Context, req *pb.CertificateRe
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if s.failOnConfigAndCerts {
-		return nil, status.Error(codes.Internal, "oops failure")
+	if s.errConfigAndCerts != nil {
+		return nil, s.errConfigAndCerts
 	}
 
 	d, ok := s.deviceConfigs[req.Id]

From 413e07817789553bb803605e7984a2267845480d Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:19:31 -0400
Subject: [PATCH 03/12] check for deadline exceeded error type

---
 config/reader.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/reader.go b/config/reader.go
index 41fc30282b4..9280942417c 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -262,7 +262,7 @@ func readFromCloud(
 		certData, err := readCertificateDataFromCloudGRPC(ctxWithTimeout, cfg.Cloud.SignalingInsecure, cloudCfg, logger)
 		if err != nil {
 			cancel()
-			if !errors.Is(err, context.DeadlineExceeded) {
+			if !errors.As(err, &context.DeadlineExceeded) {
 				return nil, err
 			}
 			if tls.certificate == "" || tls.privateKey == "" {

From ffc24d07942e5bf4888f023d0fa67853a5e139c2 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:25:23 -0400
Subject: [PATCH 04/12] tls struct [RSDK-539]

---
 config/reader.go | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/config/reader.go b/config/reader.go
index 9280942417c..7e6cbba6c6c 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -131,10 +131,10 @@ func readCertificateDataFromCloudGRPC(ctx context.Context,
 	signalingInsecure bool,
 	cloudConfigFromDisk *Cloud,
 	logger logging.Logger,
-) (*Cloud, error) {
+) (tlsConfig, error) {
 	conn, err := CreateNewGRPCClient(ctx, cloudConfigFromDisk, logger)
 	if err != nil {
-		return nil, err
+		return tlsConfig{}, err
 	}
 	defer utils.UncheckedErrorFunc(conn.Close)
 
@@ -142,22 +142,21 @@ func readCertificateDataFromCloudGRPC(ctx context.Context,
 	res, err := service.Certificate(ctx, &apppb.CertificateRequest{Id: cloudConfigFromDisk.ID})
 	if err != nil {
 		// Check cache?
-		return nil, err
+		return tlsConfig{}, err
 	}
 
 	if !signalingInsecure {
 		if res.TlsCertificate == "" {
-			return nil, errors.New("no TLS certificate yet from cloud; try again later")
+			return tlsConfig{}, errors.New("no TLS certificate yet from cloud; try again later")
 		}
 		if res.TlsPrivateKey == "" {
-			return nil, errors.New("no TLS private key yet from cloud; try again later")
+			return tlsConfig{}, errors.New("no TLS private key yet from cloud; try again later")
 		}
 	}
 
-	// TODO(RSDK-539): we might want to use an internal type here. The gRPC api will not return a Cloud json struct.
-	return &Cloud{
-		TLSCertificate: res.TlsCertificate,
-		TLSPrivateKey:  res.TlsPrivateKey,
+	return tlsConfig{
+		certificate: res.TlsCertificate,
+		privateKey:  res.TlsPrivateKey,
 	}, nil
 }
 
@@ -270,9 +269,7 @@ func readFromCloud(
 			}
 			logger.Warnw("failed to refresh certificate data; using cached for now", "error", err)
 		} else {
-			tls.certificate = certData.TLSCertificate
-			tls.privateKey = certData.TLSPrivateKey
-			cancel()
+			tls = certData
 		}
 	}
 

From 59f332570893cc101eecceedb683876c04aa950d Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:25:44 -0400
Subject: [PATCH 05/12] remove TODOs

---
 config/reader.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/config/reader.go b/config/reader.go
index 7e6cbba6c6c..479fec1331a 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -294,13 +294,11 @@ func readFromCloud(
 		to.Cloud.LocationSecrets = locationSecrets
 		to.Cloud.TLSCertificate = tls.certificate
 		to.Cloud.TLSPrivateKey = tls.privateKey
-		// TODO: are org-id/location-id missing?
 		to.Cloud.PrimaryOrgID = primaryOrgID
 		to.Cloud.LocationID = locationID
 	}
 
 	mergeCloudConfig(cfg)
-	// TODO(RSDK-1960): add more tests around config caching
 	unprocessedConfig.Cloud.TLSCertificate = tls.certificate
 	unprocessedConfig.Cloud.TLSPrivateKey = tls.privateKey
 

From 0308c10959db90916b7a997cc2da25690161260c Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:35:16 -0400
Subject: [PATCH 06/12] lint

---
 config/reader.go      | 1 -
 config/reader_test.go | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/config/reader.go b/config/reader.go
index 479fec1331a..b76e0384368 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -204,7 +204,6 @@ func readFromCloud(
 	logger.Debug("reading configuration from the cloud")
 	cloudCfg := originalCfg.Cloud
 	unprocessedConfig, cached, err := getFromCloudOrCache(ctx, cloudCfg, shouldReadFromCache, logger)
-
 	if err != nil {
 		if !cached {
 			err = errors.Wrap(err, "error getting cloud config")
diff --git a/config/reader_test.go b/config/reader_test.go
index 296cde19da2..bb76abb956b 100644
--- a/config/reader_test.go
+++ b/config/reader_test.go
@@ -37,7 +37,7 @@ func TestFromReader(t *testing.T) {
 	setupFakeServer := func(t *testing.T) (*testutils.FakeCloudServer, func()) {
 		t.Helper()
 
-		var logger = logging.NewTestLogger(t)
+		logger := logging.NewTestLogger(t)
 
 		fakeServer, err := testutils.NewFakeCloudServer(context.Background(), logger)
 		test.That(t, err, test.ShouldBeNil)

From 1ae6504720a60e2f2af312ae7894b591b3426c77 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 15:35:24 -0400
Subject: [PATCH 07/12] reinstate cancel

---
 config/reader.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/reader.go b/config/reader.go
index b76e0384368..a9b2a910817 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -269,6 +269,7 @@ func readFromCloud(
 			logger.Warnw("failed to refresh certificate data; using cached for now", "error", err)
 		} else {
 			tls = certData
+			cancel()
 		}
 	}
 

From 1a2e8423d784d3c342b8b983870b6035a990f461 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 16:47:15 -0400
Subject: [PATCH 08/12] rename GetCloudMetadata -> CloudMetadata [RSDK-6945]

---
 robot/client/client.go              |  4 ++--
 robot/client/client_test.go         |  6 +++---
 robot/impl/local_robot.go           |  4 ++--
 robot/impl/local_robot_test.go      |  5 +++--
 robot/impl/resource_manager_test.go |  4 ++--
 robot/robot.go                      |  4 ++--
 robot/server/server.go              |  2 +-
 robot/server/server_test.go         |  2 +-
 testutils/inject/robot.go           | 12 ++++++------
 9 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/robot/client/client.go b/robot/client/client.go
index d33847c60dd..c43fb3a9107 100644
--- a/robot/client/client.go
+++ b/robot/client/client.go
@@ -933,8 +933,8 @@ func (rc *RobotClient) Log(ctx context.Context, log zapcore.Entry, fields []zap.
 	return err
 }
 
-// GetCloudMetadata returns app-related information about the robot.
-func (rc *RobotClient) GetCloudMetadata(ctx context.Context) (cloud.Metadata, error) {
+// CloudMetadata returns app-related information about the robot.
+func (rc *RobotClient) CloudMetadata(ctx context.Context) (cloud.Metadata, error) {
 	cloudMD := cloud.Metadata{}
 	req := &pb.GetCloudMetadataRequest{}
 	resp, err := rc.client.GetCloudMetadata(ctx, req)
diff --git a/robot/client/client_test.go b/robot/client/client_test.go
index 6266bdfd915..a253605153e 100644
--- a/robot/client/client_test.go
+++ b/robot/client/client_test.go
@@ -2012,7 +2012,7 @@ func TestLoggingInterceptor(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 }
 
-func TestGetCloudMetadata(t *testing.T) {
+func TestCloudMetadata(t *testing.T) {
 	logger := logging.NewTestLogger(t)
 	listener, err := net.Listen("tcp", "localhost:0")
 	test.That(t, err, test.ShouldBeNil)
@@ -2026,7 +2026,7 @@ func TestGetCloudMetadata(t *testing.T) {
 	injectRobot := &inject.Robot{
 		ResourceNamesFunc:   func() []resource.Name { return nil },
 		ResourceRPCAPIsFunc: func() []resource.RPCAPI { return nil },
-		GetCloudMetadataFunc: func(ctx context.Context) (cloud.Metadata, error) {
+		CloudMetadataFunc: func(ctx context.Context) (cloud.Metadata, error) {
 			return injectCloudMD, nil
 		},
 	}
@@ -2045,7 +2045,7 @@ func TestGetCloudMetadata(t *testing.T) {
 		test.That(t, client.Close(context.Background()), test.ShouldBeNil)
 	}()
 
-	md, err := client.GetCloudMetadata(context.Background())
+	md, err := client.CloudMetadata(context.Background())
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, md, test.ShouldResemble, injectCloudMD)
 }
diff --git a/robot/impl/local_robot.go b/robot/impl/local_robot.go
index 7e6667a6884..4ad5e082453 100644
--- a/robot/impl/local_robot.go
+++ b/robot/impl/local_robot.go
@@ -1173,8 +1173,8 @@ func (r *localRobot) checkMaxInstance(api resource.API, max int) error {
 	return nil
 }
 
-// GetCloudMetadata returns app-related information about the robot.
-func (r *localRobot) GetCloudMetadata(ctx context.Context) (cloud.Metadata, error) {
+// CloudMetadata returns app-related information about the robot.
+func (r *localRobot) CloudMetadata(ctx context.Context) (cloud.Metadata, error) {
 	md := cloud.Metadata{}
 	cfg := r.Config()
 	if cfg == nil {
diff --git a/robot/impl/local_robot_test.go b/robot/impl/local_robot_test.go
index 173f64f2cb0..53419178d31 100644
--- a/robot/impl/local_robot_test.go
+++ b/robot/impl/local_robot_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/pkg/errors"
 	"go.mongodb.org/mongo-driver/bson/primitive"
 	"go.uber.org/zap"
+
 	// registers all components.
 	commonpb "go.viam.com/api/common/v1"
 	armpb "go.viam.com/api/component/arm/v1"
@@ -3233,7 +3234,7 @@ func TestCloudMetadata(t *testing.T) {
 		cfg := &config.Config{}
 		robot, shutdown := initTestRobot(t, ctx, cfg, logger)
 		defer shutdown()
-		_, err := robot.GetCloudMetadata(ctx)
+		_, err := robot.CloudMetadata(ctx)
 		test.That(t, err, test.ShouldBeError, errors.New("cloud metadata not available"))
 	})
 	t.Run("with cloud data", func(t *testing.T) {
@@ -3246,7 +3247,7 @@ func TestCloudMetadata(t *testing.T) {
 		}
 		robot, shutdown := initTestRobot(t, ctx, cfg, logger)
 		defer shutdown()
-		md, err := robot.GetCloudMetadata(ctx)
+		md, err := robot.CloudMetadata(ctx)
 		test.That(t, err, test.ShouldBeNil)
 		test.That(t, md, test.ShouldResemble, cloud.Metadata{
 			RobotPartID:  "the-robot-part",
diff --git a/robot/impl/resource_manager_test.go b/robot/impl/resource_manager_test.go
index 8d604f080ae..03fa01db43c 100644
--- a/robot/impl/resource_manager_test.go
+++ b/robot/impl/resource_manager_test.go
@@ -1839,8 +1839,8 @@ func (rr *dummyRobot) Logger() logging.Logger {
 	return rr.robot.Logger()
 }
 
-func (rr *dummyRobot) GetCloudMetadata(ctx context.Context) (cloud.Metadata, error) {
-	return rr.robot.GetCloudMetadata(ctx)
+func (rr *dummyRobot) CloudMetadata(ctx context.Context) (cloud.Metadata, error) {
+	return rr.robot.CloudMetadata(ctx)
 }
 
 func (rr *dummyRobot) Close(ctx context.Context) error {
diff --git a/robot/robot.go b/robot/robot.go
index aa6cee62c1b..4d581fae046 100644
--- a/robot/robot.go
+++ b/robot/robot.go
@@ -81,8 +81,8 @@ type Robot interface {
 	// Status takes a list of resource names and returns their corresponding statuses. If no names are passed in, return all statuses.
 	Status(ctx context.Context, resourceNames []resource.Name) ([]Status, error)
 
-	// GetCloudMetadata returns app-related information about the robot.
-	GetCloudMetadata(ctx context.Context) (cloud.Metadata, error)
+	// CloudMetadata returns app-related information about the robot.
+	CloudMetadata(ctx context.Context) (cloud.Metadata, error)
 
 	// Close attempts to cleanly close down all constituent parts of the robot.
 	Close(ctx context.Context) error
diff --git a/robot/server/server.go b/robot/server/server.go
index ebf54588074..1b15de492d9 100644
--- a/robot/server/server.go
+++ b/robot/server/server.go
@@ -461,7 +461,7 @@ func (s *Server) Log(ctx context.Context, req *pb.LogRequest) (*pb.LogResponse,
 
 // GetCloudMetadata returns app-related information about the robot.
 func (s *Server) GetCloudMetadata(ctx context.Context, _ *pb.GetCloudMetadataRequest) (*pb.GetCloudMetadataResponse, error) {
-	md, err := s.robot.GetCloudMetadata(ctx)
+	md, err := s.robot.CloudMetadata(ctx)
 	if err != nil {
 		return nil, err
 	}
diff --git a/robot/server/server_test.go b/robot/server/server_test.go
index 07cb7c81cc5..8b83a5feadd 100644
--- a/robot/server/server_test.go
+++ b/robot/server/server_test.go
@@ -78,7 +78,7 @@ func TestServer(t *testing.T) {
 		injectRobot := &inject.Robot{}
 		server := server.New(injectRobot)
 		req := pb.GetCloudMetadataRequest{}
-		injectRobot.GetCloudMetadataFunc = func(ctx context.Context) (cloud.Metadata, error) {
+		injectRobot.CloudMetadataFunc = func(ctx context.Context) (cloud.Metadata, error) {
 			return cloud.Metadata{
 				RobotPartID:  "the-robot-part",
 				PrimaryOrgID: "the-primary-org",
diff --git a/testutils/inject/robot.go b/testutils/inject/robot.go
index 5c84f182758..62120380d51 100644
--- a/testutils/inject/robot.go
+++ b/testutils/inject/robot.go
@@ -48,7 +48,7 @@ type Robot struct {
 	TransformPointCloudFunc func(ctx context.Context, srcpc pointcloud.PointCloud, srcName, dstName string) (pointcloud.PointCloud, error)
 	StatusFunc              func(ctx context.Context, resourceNames []resource.Name) ([]robot.Status, error)
 	ModuleAddressFunc       func() (string, error)
-	GetCloudMetadataFunc    func(ctx context.Context) (cloud.Metadata, error)
+	CloudMetadataFunc       func(ctx context.Context) (cloud.Metadata, error)
 
 	ops        *operation.Manager
 	SessMgr    session.Manager
@@ -284,14 +284,14 @@ func (r *Robot) ModuleAddress() (string, error) {
 	return r.ModuleAddressFunc()
 }
 
-// GetCloudMetadata calls the injected GetCloudMetadata or the real one.
-func (r *Robot) GetCloudMetadata(ctx context.Context) (cloud.Metadata, error) {
+// CloudMetadata calls the injected CloudMetadata or the real one.
+func (r *Robot) CloudMetadata(ctx context.Context) (cloud.Metadata, error) {
 	r.Mu.RLock()
 	defer r.Mu.RUnlock()
-	if r.GetCloudMetadataFunc == nil {
-		return r.GetCloudMetadata(ctx)
+	if r.CloudMetadataFunc == nil {
+		return r.CloudMetadata(ctx)
 	}
-	return r.GetCloudMetadataFunc(ctx)
+	return r.CloudMetadataFunc(ctx)
 }
 
 type noopSessionManager struct{}

From 77f82ca1514a558f729d50c532540415d90efd4f Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 16:53:15 -0400
Subject: [PATCH 09/12] lint

---
 robot/impl/local_robot_test.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/robot/impl/local_robot_test.go b/robot/impl/local_robot_test.go
index 53419178d31..500d6a70084 100644
--- a/robot/impl/local_robot_test.go
+++ b/robot/impl/local_robot_test.go
@@ -18,7 +18,6 @@ import (
 	"github.com/pkg/errors"
 	"go.mongodb.org/mongo-driver/bson/primitive"
 	"go.uber.org/zap"
-
 	// registers all components.
 	commonpb "go.viam.com/api/common/v1"
 	armpb "go.viam.com/api/component/arm/v1"

From b430502818b1a4365a36cb70b9682a53df05a0cd Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Thu, 14 Mar 2024 17:23:56 -0400
Subject: [PATCH 10/12] return shutdown instead of error

---
 config/testutils/fake_cloud.go | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/config/testutils/fake_cloud.go b/config/testutils/fake_cloud.go
index 08d220771e6..25f20ee145f 100644
--- a/config/testutils/fake_cloud.go
+++ b/config/testutils/fake_cloud.go
@@ -7,8 +7,10 @@ import (
 	"net"
 	"net/http"
 	"sync"
+	"testing"
 
 	pb "go.viam.com/api/app/v1"
+	"go.viam.com/test"
 	"go.viam.com/utils"
 	"go.viam.com/utils/rpc"
 	"google.golang.org/grpc/codes"
@@ -42,10 +44,12 @@ type configAndCerts struct {
 }
 
 // NewFakeCloudServer creates and starts a new grpc server for the Viam Cloud.
-func NewFakeCloudServer(ctx context.Context, logger logging.Logger) (*FakeCloudServer, error) {
+func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger) (*FakeCloudServer, func()) { //revive:disable-line:context-as-argument
+	t.Helper()
+
 	listener, err := net.ListenTCP("tcp", &net.TCPAddr{Port: 0})
 	if err != nil {
-		return nil, err
+		return nil, func() {}
 	}
 
 	server := &FakeCloudServer{
@@ -63,7 +67,10 @@ func NewFakeCloudServer(ctx context.Context, logger logging.Logger) (*FakeCloudS
 		)),
 		rpc.WithWebRTCServerOptions(rpc.WebRTCServerOptions{Enable: false}))
 	if err != nil {
-		return nil, err
+		return nil, func() {}
+	}
+	shutdown := func() {
+		test.That(t, server.Shutdown(), test.ShouldBeNil)
 	}
 
 	err = server.rpcServer.RegisterServiceServer(
@@ -73,7 +80,7 @@ func NewFakeCloudServer(ctx context.Context, logger logging.Logger) (*FakeCloudS
 		pb.RegisterRobotServiceHandlerFromEndpoint,
 	)
 	if err != nil {
-		return nil, err
+		return nil, shutdown
 	}
 
 	server.exitWg.Add(1)
@@ -85,8 +92,7 @@ func NewFakeCloudServer(ctx context.Context, logger logging.Logger) (*FakeCloudS
 			logger.Warnf("Error shutting down grpc server", "error", err)
 		}
 	})
-
-	return server, nil
+	return server, shutdown
 }
 
 // FailOnConfigAndCerts if `failure` is true the server will return an Internal error on

From 86b4d8729469341bcfbd29723da5719704b3a855 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Fri, 15 Mar 2024 11:09:04 -0400
Subject: [PATCH 11/12] make generic test checks part of NewFakeCloudServer
 test utility

---
 config/reader_test.go          | 18 ++----------------
 config/testutils/fake_cloud.go | 13 ++++++-------
 config/watcher_test.go         |  7 ++-----
 3 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/config/reader_test.go b/config/reader_test.go
index bb76abb956b..c24cad9927e 100644
--- a/config/reader_test.go
+++ b/config/reader_test.go
@@ -34,24 +34,10 @@ func TestFromReader(t *testing.T) {
 		test.That(t, os.IsNotExist(err), test.ShouldBeTrue)
 	}
 
-	setupFakeServer := func(t *testing.T) (*testutils.FakeCloudServer, func()) {
-		t.Helper()
-
-		logger := logging.NewTestLogger(t)
-
-		fakeServer, err := testutils.NewFakeCloudServer(context.Background(), logger)
-		test.That(t, err, test.ShouldBeNil)
-		cleanup := func() {
-			test.That(t, fakeServer.Shutdown(), test.ShouldBeNil)
-		}
-
-		return fakeServer, cleanup
-	}
-
 	t.Run("online", func(t *testing.T) {
 		setupClearCache(t)
 
-		fakeServer, cleanup := setupFakeServer(t)
+		fakeServer, cleanup := testutils.NewFakeCloudServer(t, ctx, logger)
 		defer cleanup()
 
 		cloudResponse := &Cloud{
@@ -114,7 +100,7 @@ func TestFromReader(t *testing.T) {
 		test.That(t, err, test.ShouldBeNil)
 		defer clearCache(robotPartID)
 
-		fakeServer, cleanup := setupFakeServer(t)
+		fakeServer, cleanup := testutils.NewFakeCloudServer(t, ctx, logger)
 		defer cleanup()
 		fakeServer.FailOnConfigAndCertsWith(context.DeadlineExceeded)
 		fakeServer.StoreDeviceConfig(robotPartID, nil, nil)
diff --git a/config/testutils/fake_cloud.go b/config/testutils/fake_cloud.go
index 25f20ee145f..66921160e38 100644
--- a/config/testutils/fake_cloud.go
+++ b/config/testutils/fake_cloud.go
@@ -49,7 +49,7 @@ func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger
 
 	listener, err := net.ListenTCP("tcp", &net.TCPAddr{Port: 0})
 	if err != nil {
-		return nil, func() {}
+		t.Fatalf("failed to start listener for fake cloud server")
 	}
 
 	server := &FakeCloudServer{
@@ -67,10 +67,7 @@ func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger
 		)),
 		rpc.WithWebRTCServerOptions(rpc.WebRTCServerOptions{Enable: false}))
 	if err != nil {
-		return nil, func() {}
-	}
-	shutdown := func() {
-		test.That(t, server.Shutdown(), test.ShouldBeNil)
+		t.Fatalf("failed to create new fake cloud server")
 	}
 
 	err = server.rpcServer.RegisterServiceServer(
@@ -80,9 +77,8 @@ func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger
 		pb.RegisterRobotServiceHandlerFromEndpoint,
 	)
 	if err != nil {
-		return nil, shutdown
+		t.Fatalf("failed to register robot service for new fake cloud server")
 	}
-
 	server.exitWg.Add(1)
 	utils.PanicCapturingGo(func() {
 		defer server.exitWg.Done()
@@ -92,6 +88,9 @@ func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger
 			logger.Warnf("Error shutting down grpc server", "error", err)
 		}
 	})
+	shutdown := func() {
+		test.That(t, server.Shutdown(), test.ShouldBeNil)
+	}
 	return server, shutdown
 }
 
diff --git a/config/watcher_test.go b/config/watcher_test.go
index 2287c918b4f..2fbec70a0ac 100644
--- a/config/watcher_test.go
+++ b/config/watcher_test.go
@@ -187,11 +187,8 @@ func TestNewWatcherCloud(t *testing.T) {
 
 	deviceID := primitive.NewObjectID().Hex()
 
-	fakeServer, err := testutils.NewFakeCloudServer(context.Background(), logger)
-	test.That(t, err, test.ShouldBeNil)
-	defer func() {
-		test.That(t, fakeServer.Shutdown(), test.ShouldBeNil)
-	}()
+	fakeServer, cleanup := testutils.NewFakeCloudServer(t, context.Background(), logger)
+	defer cleanup()
 
 	storeConfigInServer := func(cfg config.Config) {
 		cloudConfProto, err := config.CloudConfigToProto(cfg.Cloud)

From 36654a216d28751dc1ff4520b7ab848ca6337ca4 Mon Sep 17 00:00:00 2001
From: Maxim Pertsov <maxim@viam.com>
Date: Fri, 15 Mar 2024 12:35:10 -0400
Subject: [PATCH 12/12] nolint

---
 config/testutils/fake_cloud.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/testutils/fake_cloud.go b/config/testutils/fake_cloud.go
index 66921160e38..4df27fc2f0f 100644
--- a/config/testutils/fake_cloud.go
+++ b/config/testutils/fake_cloud.go
@@ -44,7 +44,7 @@ type configAndCerts struct {
 }
 
 // NewFakeCloudServer creates and starts a new grpc server for the Viam Cloud.
-func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger) (*FakeCloudServer, func()) { //revive:disable-line:context-as-argument
+func NewFakeCloudServer(t *testing.T, ctx context.Context, logger logging.Logger) (*FakeCloudServer, func()) { //nolint:revive
 	t.Helper()
 
 	listener, err := net.ListenTCP("tcp", &net.TCPAddr{Port: 0})