RSDK-6344 Read partial config when getting cached TLS information (#3620)

Author: maximpertsov

config/config.go

@@ -605,6 +605,17 @@ func (config *Cloud) Validate(path string, fromCloud bool) error {
 	return nil
 }
 
+// ValidateTLS ensures TLS fields are valid.
+func (config *Cloud) ValidateTLS(path string) error {
+	if config.TLSCertificate == "" {
+		return resource.NewConfigValidationFieldRequiredError(path, "tls_certificate")
+	}
+	if config.TLSPrivateKey == "" {
+		return resource.NewConfigValidationFieldRequiredError(path, "tls_private_key")
+	}
+	return nil
+}
+
 // LocationSecret describes a location secret that can be used to authenticate to the rdk.
 type LocationSecret struct {
 	ID string `json:"id"`

config/reader.go

@@ -225,28 +225,16 @@ func readFromCloud(
 		return nil, errors.New("expected config to have cloud section")
 	}
 
-	// empty if not cached, since its a separate request, which we check next
-	tlsCertificate := cfg.Cloud.TLSCertificate
-	tlsPrivateKey := cfg.Cloud.TLSPrivateKey
+	tls := tlsConfig{
+		// both fields are empty if not cached, since its a separate request, which we
+		// check next
+		certificate: cfg.Cloud.TLSCertificate,
+		privateKey:  cfg.Cloud.TLSPrivateKey,
+	}
 	if !cached {
-		// get cached certificate data
-		// read cached config from fs.
-		// process the config with fromReader() use processed config as cachedConfig to update the cert data.
-		unproccessedCachedConfig, err := readFromCache(cloudCfg.ID)
-		if err == nil {
-			cachedConfig, err := processConfigFromCloud(unproccessedCachedConfig, logger)
-			if err != nil {
-				// clear cache
-				logger.Warn("Detected failure to process the cached config when retrieving TLS config, clearing cache.")
-				clearCache(cloudCfg.ID)
-				return nil, err
-			}
-
-			if cachedConfig.Cloud != nil {
-				tlsCertificate = cachedConfig.Cloud.TLSCertificate
-				tlsPrivateKey = cachedConfig.Cloud.TLSPrivateKey
-			}
-		} else if !os.IsNotExist(err) {
+		// Try to get TLS information from the cached config (if it exists) even if we
+		// got a new config from the cloud.
+		if err := tls.readFromCache(cloudCfg.ID, logger); err != nil {
 			return nil, err
 		}
 	}
@@ -255,7 +243,7 @@ func readFromCloud(
 		checkForNewCert = true
 	}
 
-	if checkForNewCert || tlsCertificate == "" || tlsPrivateKey == "" {
+	if checkForNewCert || tls.certificate == "" || tls.privateKey == "" {
 		logger.Debug("reading tlsCertificate from the cloud")
 		// Use the SignalingInsecure from the Cloud config returned from the app not the initial config.
 
@@ -264,13 +252,13 @@ func readFromCloud(
 			if !errors.Is(err, context.DeadlineExceeded) {
 				return nil, err
 			}
-			if tlsCertificate == "" || tlsPrivateKey == "" {
+			if tls.certificate == "" || tls.privateKey == "" {
 				return nil, errors.Wrap(err, "error getting certificate data from cloud; try again later")
 			}
 			logger.Warnw("failed to refresh certificate data; using cached for now", "error", err)
 		} else {
-			tlsCertificate = certData.TLSCertificate
-			tlsPrivateKey = certData.TLSPrivateKey
+			tls.certificate = certData.TLSCertificate
+			tls.privateKey = certData.TLSPrivateKey
 		}
 	}
 
@@ -291,14 +279,14 @@ func readFromCloud(
 		to.Cloud.ManagedBy = managedBy
 		to.Cloud.LocationSecret = locationSecret
 		to.Cloud.LocationSecrets = locationSecrets
-		to.Cloud.TLSCertificate = tlsCertificate
-		to.Cloud.TLSPrivateKey = tlsPrivateKey
+		to.Cloud.TLSCertificate = tls.certificate
+		to.Cloud.TLSPrivateKey = tls.privateKey
 	}
 
 	mergeCloudConfig(cfg)
 	// TODO(RSDK-1960): add more tests around config caching
-	unprocessedConfig.Cloud.TLSCertificate = tlsCertificate
-	unprocessedConfig.Cloud.TLSPrivateKey = tlsPrivateKey
+	unprocessedConfig.Cloud.TLSCertificate = tls.certificate
+	unprocessedConfig.Cloud.TLSPrivateKey = tls.privateKey
 
 	if err := storeToCache(cloudCfg.ID, unprocessedConfig); err != nil {
 		logger.Errorw("failed to cache config", "error", err)
@@ -307,6 +295,33 @@ func readFromCloud(
 	return cfg, nil
 }
 
+type tlsConfig struct {
+	certificate string
+	privateKey  string
+}
+
+func (tls *tlsConfig) readFromCache(id string, logger logging.Logger) error {
+	cachedCfg, err := readFromCache(id)
+	switch {
+	case os.IsNotExist(err):
+		logger.Warn("No cached config, using cloud TLS config.")
+	case err != nil:
+		return err
+	case cachedCfg.Cloud == nil:
+		logger.Warn("Cached config is not a cloud config, using cloud TLS config.")
+	default:
+		if err := cachedCfg.Cloud.ValidateTLS("cloud"); err != nil {
+			logger.Warn("Detected failure to process the cached config when retrieving TLS config, clearing cache.")
+			clearCache(id)
+			return err
+		}
+
+		tls.certificate = cachedCfg.Cloud.TLSCertificate
+		tls.privateKey = cachedCfg.Cloud.TLSPrivateKey
+	}
+	return nil
+}
+
 // Read reads a config from the given file.
 func Read(
 	ctx context.Context,

config/reader_test.go

@@ -121,3 +121,70 @@ func TestProcessConfig(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, *cfg, test.ShouldResemble, unprocessedConfig)
 }
+
+func TestReadTLSFromCache(t *testing.T) {
+	logger := logging.NewTestLogger(t)
+	ctx := context.Background()
+	cfg, err := FromReader(ctx, "", strings.NewReader(`{}`), logger)
+	test.That(t, err, test.ShouldBeNil)
+
+	robotPartID := "forCachingTest"
+	t.Run("no cached config", func(t *testing.T) {
+		clearCache(robotPartID)
+		test.That(t, err, test.ShouldBeNil)
+
+		tls := tlsConfig{}
+		err = tls.readFromCache(robotPartID, logger)
+		test.That(t, err, test.ShouldBeNil)
+	})
+
+	t.Run("cache config without cloud", func(t *testing.T) {
+		defer clearCache(robotPartID)
+		cfg.Cloud = nil
+
+		err = storeToCache(robotPartID, cfg)
+		test.That(t, err, test.ShouldBeNil)
+
+		tls := tlsConfig{}
+		err = tls.readFromCache(robotPartID, logger)
+		test.That(t, err, test.ShouldBeNil)
+	})
+
+	t.Run("invalid cached TLS", func(t *testing.T) {
+		defer clearCache(robotPartID)
+		cloud := &Cloud{
+			TLSPrivateKey: "key",
+		}
+		cfg.Cloud = cloud
+
+		err = storeToCache(robotPartID, cfg)
+		test.That(t, err, test.ShouldBeNil)
+
+		tls := tlsConfig{}
+		err = tls.readFromCache(robotPartID, logger)
+		test.That(t, err, test.ShouldNotBeNil)
+
+		_, err = readFromCache(robotPartID)
+		test.That(t, os.IsNotExist(err), test.ShouldBeTrue)
+	})
+
+	t.Run("valid cached TLS", func(t *testing.T) {
+		defer clearCache(robotPartID)
+		cloud := &Cloud{
+			TLSCertificate: "cert",
+			TLSPrivateKey:  "key",
+		}
+		cfg.Cloud = cloud
+
+		err = storeToCache(robotPartID, cfg)
+		test.That(t, err, test.ShouldBeNil)
+
+		// the config is missing several fields required to start the robot, but this
+		// should not prevent us from reading TLS information from it.
+		_, err = processConfigFromCloud(cfg, logger)
+		test.That(t, err, test.ShouldNotBeNil)
+		tls := tlsConfig{}
+		err = tls.readFromCache(robotPartID, logger)
+		test.That(t, err, test.ShouldBeNil)
+	})
+}