Skip to content

Commit

Permalink
Remove HTTP versions of RestartChecker, Config Reader, Log Writer
Browse files Browse the repository at this point in the history 
The http -> grpc migration is long done now and those code paths are
no longer needed.
  • Loading branch information
@AdamMagaluk
AdamMagaluk committed Mar 2, 2023
1 parent 3de25e0 commit d57dac5
Show file tree
Hide file tree
Showing 9 changed files with 288 additions and 454 deletions.
26 changes: 0 additions & 26 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,6 @@ func TestConfig3(t *testing.T) {
test.That(t, cfg.Remotes[0].ReconnectInterval, test.ShouldEqual, 3*time.Second)
}

func TestCreateCloudRequest(t *testing.T) {
cfg := config.Cloud{
ID: "a",
Secret: "b",
Path: "c",
}

version := "test-version"
gitRevision := "test-git-revision"
config.Version = version
config.GitRevision = gitRevision

r, err := config.CreateCloudRequest(context.Background(), &cfg)
test.That(t, err, test.ShouldBeNil)

test.That(t, r.Header.Get("Secret"), test.ShouldEqual, cfg.Secret)
test.That(t, r.URL.String(), test.ShouldEqual, "c?id=a")

userInfo := map[string]interface{}{}
userInfoJSON := r.Header.Get("User-Info")
json.Unmarshal([]byte(userInfoJSON), &userInfo)

test.That(t, userInfo["version"], test.ShouldEqual, version)
test.That(t, userInfo["gitRevision"], test.ShouldEqual, gitRevision)
}

func TestConfigEnsure(t *testing.T) {
var emptyConfig config.Config
test.That(t, emptyConfig.Ensure(false), test.ShouldBeNil)
Expand Down
172 changes: 5 additions & 167 deletions config/reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
"os"
"path/filepath"
Expand Down Expand Up @@ -206,48 +205,6 @@ func findServiceMapConverter(svcType resource.Subtype, model resource.Model) Att
return nil
}

const (
cloudConfigSecretField = "Secret"
cloudConfigUserInfoField = "User-Info"
cloudConfigUserInfoHostField = "host"
cloudConfigUserInfoOSField = "os"
cloudConfigUserInfoLocalIPsField = "ips"
cloudConfigVersionField = "version"
cloudConfigGitRevisionField = "gitRevision"
)

// CreateCloudRequest makes a request to fetch the robot config
// from a cloud endpoint.
func CreateCloudRequest(ctx context.Context, cloudCfg *Cloud) (*http.Request, error) {
url := fmt.Sprintf("%s?id=%s", cloudCfg.Path, cloudCfg.ID)

r, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return nil, errors.Wrapf(err, "error creating request for %s", url)
}
r.Header.Set(cloudConfigSecretField, cloudCfg.Secret)

agentInfo, err := getAgentInfo()
if err != nil {
return nil, err
}

userInfo := map[string]interface{}{}
userInfo[cloudConfigUserInfoHostField] = agentInfo.Host
userInfo[cloudConfigUserInfoOSField] = agentInfo.Os
userInfo[cloudConfigUserInfoLocalIPsField] = agentInfo.Ips
userInfo[cloudConfigVersionField] = agentInfo.Version
userInfo[cloudConfigGitRevisionField] = agentInfo.GitRevision

userInfoBytes, err := json.Marshal(userInfo)
if err != nil {
return nil, err
}
r.Header.Set(cloudConfigUserInfoField, string(userInfoBytes))

return r, nil
}

func getAgentInfo() (*apppb.AgentInfo, error) {
hostname, err := os.Hostname()
if err != nil {
Expand All @@ -268,20 +225,6 @@ func getAgentInfo() (*apppb.AgentInfo, error) {
}, nil
}

// createCloudCertificateRequest makes a request to fetch the robot's TLS
// certificate from a cloud endpoint.
func createCloudCertificateRequest(ctx context.Context, cloudCfg *Cloud) (*http.Request, error) {
url := fmt.Sprintf("%s?id=%s&cert=true", cloudCfg.Path, cloudCfg.ID)

r, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return nil, errors.Wrapf(err, "error creating request for %s", url)
}
r.Header.Set(cloudConfigSecretField, cloudCfg.Secret)

return r, nil
}

var viamDotDir = filepath.Join(os.Getenv("HOME"), ".viam")

func getCloudCacheFilePath(id string) string {
Expand Down Expand Up @@ -329,49 +272,6 @@ func clearCache(id string) {
})
}

// readCertificateDataFromCloud returns the certificate from the app. It returns it as properties of a new Cloud config.
// The argument `cloudConfigFromDisk` represents the Cloud config from disk and only the Path parameters are used to
// generate the url. This is different from the Cloud config returned from the HTTP or gRPC API which do not have it.
//
// TODO(RSDK-539): The TLS certificate data should not be part of the Cloud portion of the config.
func readCertificateDataFromCloud(ctx context.Context, signalingInsecure bool, cloudConfigFromDisk *Cloud) (*Cloud, error) {
certReq, err := createCloudCertificateRequest(ctx, cloudConfigFromDisk)
if err != nil {
return nil, err
}

var client http.Client
defer client.CloseIdleConnections()
resp, err := client.Do(certReq)
if err != nil {
return nil, err
}
defer func() {
utils.UncheckedError(resp.Body.Close())
}()

dec := json.NewDecoder(resp.Body)
var certData Cloud
if err := dec.Decode(&certData); err != nil {
return nil, errors.Wrap(err, "error decoding certificate data from cloud; try again later")
}

if !signalingInsecure {
if certData.TLSCertificate == "" {
return nil, errors.New("no TLS certificate yet from cloud; try again later")
}
if certData.TLSPrivateKey == "" {
return nil, 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: certData.TLSCertificate,
TLSPrivateKey: certData.TLSPrivateKey,
}, nil
}

func readCertificateDataFromCloudGRPC(ctx context.Context,
signalingInsecure bool,
cloudConfigFromDisk *Cloud,
Expand Down Expand Up @@ -506,16 +406,9 @@ func readFromCloud(
logger.Debug("reading tlsCertificate from the cloud")
// Use the SignalingInsecure from the Cloud config returned from the app not the initial config.

var certData *Cloud
if originalCfg.Cloud.AppAddress == "" {
certData, err = readCertificateDataFromCloud(ctx, cfg.Cloud.SignalingInsecure, cloudCfg)
} else {
certData, err = readCertificateDataFromCloudGRPC(ctx, cfg.Cloud.SignalingInsecure, cloudCfg, logger)
}

certData, err := readCertificateDataFromCloudGRPC(ctx, cfg.Cloud.SignalingInsecure, cloudCfg, logger)
if err != nil {
var urlErr *url.Error
if !errors.Is(err, context.DeadlineExceeded) && (!errors.As(err, &urlErr) || urlErr.Temporary()) {
if !errors.Is(err, context.DeadlineExceeded) {
return nil, err
}
if tlsCertificate == "" || tlsPrivateKey == "" {
Expand Down Expand Up @@ -706,19 +599,11 @@ func processConfig(unprocessedConfig *Config, fromCloud bool) (*Config, error) {
return cfg, nil
}

// getFromCloudOrCache returns the config from either the legacy HTTP endpoint or gRPC endpoint depending if the original config
// has the AppAddress set. If failures during cloud lookup fallback to the local cache if the error indicates it should.
// getFromCloudOrCache returns the config from the gRPC endpoint. If failures during cloud lookup fallback to the
// local cache if the error indicates it should.
func getFromCloudOrCache(ctx context.Context, cloudCfg *Cloud, shouldReadFromCache bool, logger golog.Logger) (*Config, bool, error) {
var cached bool
var cfg *Config
var errorShouldCheckCache bool
var err error
if cloudCfg.AppAddress == "" {
cfg, errorShouldCheckCache, err = getFromCloudHTTP(ctx, cloudCfg)
} else {
cfg, errorShouldCheckCache, err = getFromCloudGRPC(ctx, cloudCfg, logger)
}

cfg, errorShouldCheckCache, err := getFromCloudGRPC(ctx, cloudCfg, logger)
if err != nil {
if shouldReadFromCache && errorShouldCheckCache {
logger.Warnw("failed to read config from cloud, checking cache", "error", err)
Expand All @@ -742,53 +627,6 @@ func getFromCloudOrCache(ctx context.Context, cloudCfg *Cloud, shouldReadFromCac
return cfg, cached, nil
}

// getFromCloud actually does the fetching of the robot config and parses to an unprocessed Config struct.
func getFromCloudHTTP(ctx context.Context, cloudCfg *Cloud) (*Config, bool, error) {
shouldCheckCacheOnFailure := false
cloudReq, err := CreateCloudRequest(ctx, cloudCfg)
if err != nil {
return nil, false, err
}

unprocessedConfig := &Config{
ConfigFilePath: "",
}

var client http.Client
defer client.CloseIdleConnections()
resp, err := client.Do(cloudReq)
// Try to load from the cache
if err != nil {
var urlErr *url.Error
if !errors.Is(err, context.DeadlineExceeded) && (!errors.As(err, &urlErr) || urlErr.Temporary()) {
return nil, shouldCheckCacheOnFailure, err
}
shouldCheckCacheOnFailure = true
return nil, shouldCheckCacheOnFailure, err
}

defer func() {
utils.UncheckedError(resp.Body.Close())
}()

rd, err := io.ReadAll(resp.Body)
if err != nil {
return nil, shouldCheckCacheOnFailure, err
}

if resp.StatusCode != http.StatusOK {
if len(rd) != 0 {
return nil, shouldCheckCacheOnFailure, errors.Errorf("unexpected status %d: %s", resp.StatusCode, string(rd))
}
return nil, shouldCheckCacheOnFailure, errors.Errorf("unexpected status %d", resp.StatusCode)
}

if err := json.Unmarshal(rd, unprocessedConfig); err != nil {
return nil, shouldCheckCacheOnFailure, errors.Wrap(err, "cannot parse cloud config")
}
return unprocessedConfig, shouldCheckCacheOnFailure, nil
}

// getFromCloudGRPC actually does the fetching of the robot config from the gRPC endpoint.
func getFromCloudGRPC(ctx context.Context, cloudCfg *Cloud, logger golog.Logger) (*Config, bool, error) {
shouldCheckCacheOnFailure := true
Expand Down
7 changes: 4 additions & 3 deletions config/reader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestStoreToCache(t *testing.T) {
LocalFQDN: "localFqdn",
TLSCertificate: "cert",
TLSPrivateKey: "key",
AppAddress: "https://app.viam.dev:443",
}
cfg.Cloud = cloud

Expand All @@ -35,7 +36,7 @@ func TestStoreToCache(t *testing.T) {
test.That(t, err, test.ShouldBeNil)

// read config from cloud, confirm consistency
cloudCfg, err := readFromCloud(ctx, cfg, nil, true, true, logger)
cloudCfg, err := readFromCloud(ctx, cfg, nil, true, false, logger)
test.That(t, err, test.ShouldBeNil)
test.That(t, cloudCfg, test.ShouldResemble, cfg)

Expand All @@ -44,7 +45,7 @@ func TestStoreToCache(t *testing.T) {
cfg.Remotes = append(cfg.Remotes, newRemote)

// read config from cloud again, confirm that the cached config differs from cfg
cloudCfg2, err := readFromCloud(ctx, cfg, nil, true, true, logger)
cloudCfg2, err := readFromCloud(ctx, cfg, nil, true, false, logger)
test.That(t, err, test.ShouldBeNil)
test.That(t, cloudCfg2, test.ShouldNotResemble, cfg)

Expand All @@ -53,7 +54,7 @@ func TestStoreToCache(t *testing.T) {
test.That(t, err, test.ShouldBeNil)

// read updated cloud config, confirm that it now matches our updated cfg
cloudCfg3, err := readFromCloud(ctx, cfg, nil, true, true, logger)
cloudCfg3, err := readFromCloud(ctx, cfg, nil, true, false, logger)
test.That(t, err, test.ShouldBeNil)
test.That(t, cloudCfg3, test.ShouldResemble, cfg)
}
Expand Down
Loading

0 comments on commit d57dac5

Please sign in to comment.