Revert "RSDK-12900: use viam-defaults.json only when cloud config is … (#219)

Author: cheukt

manager.go

@@ -752,7 +752,7 @@ func (m *Manager) GetConfig(ctx context.Context) (time.Duration, error) {
 		m.logger.Error(errw.Wrapf(err, "processing update data for %s", viamserver.SubsysName))
 	}
 
-	cfgFromCloud, err := utils.StackProtoConfig(resp)
+	cfgFromCloud, err := utils.StackConfigs(resp)
 	if err != nil {
 		m.logger.Warn(errw.Wrap(err, "processing config"))
 	}

subsystems/networking/networkmanager_linux.go

@@ -3,7 +3,6 @@ package networking
 import (
 	"bytes"
 	"context"
-	"encoding/json"
 	"errors"
 	"io"
 	"net/http"
@@ -260,25 +259,10 @@ func (n *Subsystem) startProvisioning(ctx context.Context, inputChan chan<- user
 	}
 	n.internalOpMu.Lock()
 	defer n.internalOpMu.Unlock()
-
 	if ctx.Err() != nil {
 		return ctx.Err()
 	}
 
-	// try to rebase the current config onto the default. since we no longer merge configs once a cloud config is available,
-	// it may not include provisioning settings that were in the viam-defaults.json.
-	if provisioningCfg, err := rebaseNetworkConfiguration(n.cfg); err != nil {
-		n.logger.Infof("merging current networking config over viam-defaults.json failed with err. Continuing with current config.",
-			"err", err, "current_cfg", n.cfg)
-	} else if provisioningCfg != n.cfg {
-		// only log if merge produced diffs: current cfg should be either base+cloud or base+defaults; rebased cfg should be base+defaults+cloud.
-		// diffs should be entirely the fields set in viam-defaults that are not present in cloud.
-		n.logger.Infof("starting provisioning. temporarily merging current networking config with 'viam-defaults' (if available)",
-			"defaults_path", utils.DefaultsFilePath)
-		// this has either 1) no change if we've never been online 2) will be restored to options from cloud-only once we're online & refetch.
-		n.cfg = provisioningCfg
-	}
-
 	n.portalData.resetInputData(inputChan)
 	hotspotErr := n.startProvisioningHotspot(ctx)
 	if hotspotErr != nil {
@@ -297,28 +281,6 @@ func (n *Subsystem) startProvisioning(ctx context.Context, inputChan chan<- user
 	return errors.Join(hotspotErr, bluetoothErr)
 }
 
-// rebaseNetworkConfiguration reapplies a NetworkConfiguration over the default configuration.
-func rebaseNetworkConfiguration(nCfg utils.NetworkConfiguration) (utils.NetworkConfiguration, error) {
-	asJSON, err := json.Marshal(nCfg)
-	if err != nil {
-		return nCfg, err
-	}
-
-	// get default cfg. Hardcoded values + viam_defaults.json (if available - does not err if file does not exist).
-	newCfg, err := utils.StackOfflineConfig()
-	if err != nil {
-		return nCfg, err
-	}
-
-	// merge current cfg on over
-	err = json.Unmarshal(asJSON, &newCfg.NetworkConfiguration)
-	if err != nil {
-		return nCfg, err
-	}
-
-	return newCfg.NetworkConfiguration, err
-}
-
 // startProvisioningHotspot should only be called by 'StartProvisioning' (to
 // ensure opMutex is acquired).
 func (n *Subsystem) startProvisioningHotspot(ctx context.Context) error {

utils/config.go

@@ -299,15 +299,15 @@ func LoadConfigFromCache() (AgentConfig, error) {
 	cacheBytes, err := os.ReadFile(cachePath)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			return StackOfflineConfig()
+			return StackConfigs(&pb.DeviceAgentConfigResponse{})
 		} else {
-			cfg, newErr := StackOfflineConfig()
+			cfg, newErr := StackConfigs(&pb.DeviceAgentConfigResponse{})
 			return cfg, errors.Join(errw.Wrap(err, "reading config cache"), newErr)
 		}
 	} else {
 		err = json.Unmarshal(cacheBytes, &cfg)
 		if err != nil {
-			cfg, newErr := StackOfflineConfig()
+			cfg, newErr := StackConfigs(&pb.DeviceAgentConfigResponse{})
 			return cfg, errors.Join(errw.Wrap(err, "parsing config cache"), newErr)
 		}
 	}
@@ -328,29 +328,21 @@ func ApplyCLIArgs(cfg AgentConfig) AgentConfig {
 	return newCfg
 }
 
-// StackOldProvisioningConfig reads viam-provisioning.json if available and merges it over startCfg.
-func stackOldProvisioningConfig(startCfg AgentConfig) (AgentConfig, error) {
+func StackConfigs(proto *pb.DeviceAgentConfigResponse) (AgentConfig, error) {
+	cfg := DefaultConfig()
 	var errOut error
 
-	// parse/apply deprecated /etc/viam-provisioning.json (NetworkConfiguration only)
+	// parse/apply deprecated /etc/viam-provisioning.json
 	oldCfg, err := LoadOldProvisioningConfig()
 	if err != nil {
 		if !errors.Is(err, fs.ErrNotExist) {
 			errOut = errors.Join(errOut, errw.Wrap(err, "reading deprecated /etc/viam-provisioning.json"))
 		}
 	} else {
-		startCfg.NetworkConfiguration = *oldCfg
+		cfg.NetworkConfiguration = *oldCfg
 	}
-	return startCfg, errOut
-}
-
-// StackOldProvisioningConfig reads viam-defaults.json if available and merges it over startCfg.
-func stackViamDefaultsConfig(startCfg AgentConfig) (AgentConfig, error) {
-	cfg := startCfg
-	var errOut error
 
 	// manufacturer config from local disk (/etc/viam-defaults.json)
-	// use only if cloud read wasn't provided or unmarshall failed (don't merge the two).
 	jsonBytes, err := os.ReadFile(DefaultsFilePath)
 	if err != nil {
 		if !errors.Is(err, fs.ErrNotExist) {
@@ -362,70 +354,21 @@ func stackViamDefaultsConfig(startCfg AgentConfig) (AgentConfig, error) {
 		}
 	}
 
-	return cfg, errOut
-}
-
-// StackConfigs merges nextCfg over startCfg.
-func StackConfigs(startCfg, nextCfg AgentConfig) (AgentConfig, error) {
-	cfg := startCfg
-	var errOut error
-
-	jsonBytes, err := json.Marshal(nextCfg)
-	if err != nil {
-		errOut = errors.Join(errOut, err)
-	} else {
-		if err := json.Unmarshal(jsonBytes, &cfg); err != nil {
-			errOut = errors.Join(errOut, err)
-		}
-	}
-	return cfg, errOut
-}
-
-// StackOfflineConfig returns a merged config resulting from applying in order:
-// DefaultConfig -> deprecated viam-provisioning.json -> viam-defaults.json.
-func StackOfflineConfig() (AgentConfig, error) {
-	return StackProtoConfig(nil)
-}
-
-// StackProtoConfig returns a merged config resulting from applying in order:
-// DefaultConfig -> deprecated viam-provisioning.json -> cloud config if available & valid / otherwise viam-defaults.json.
-func StackProtoConfig(fromCloudProto *pb.DeviceAgentConfigResponse) (AgentConfig, error) {
-	cfg := DefaultConfig()
-	var errOut error
-
-	cfgTmp, err := stackOldProvisioningConfig(cfg)
+	// cloud-provided config
+	cloudCfg, err := ProtoToConfig(proto)
 	if err != nil {
 		errOut = errors.Join(errOut, err)
 	} else {
-		cfg = cfgTmp
-	}
-
-	var cloudCfgSuccess bool
-	if fromCloudProto != nil {
-		cloudCfg, err := ProtoToConfig(fromCloudProto)
+		jsonBytes, err = json.Marshal(cloudCfg)
 		if err != nil {
 			errOut = errors.Join(errOut, err)
 		} else {
-			cfgTmp, err := StackConfigs(cfg, cloudCfg)
-			if err != nil {
+			if err := json.Unmarshal(jsonBytes, &cfg); err != nil {
 				errOut = errors.Join(errOut, err)
-			} else {
-				cfg = cfgTmp
-				cloudCfgSuccess = true
 			}
 		}
 	}
 
-	// use viam-defaults (stack on top of base) only if cloud config is invalid
-	if !cloudCfgSuccess {
-		cfgTmp, err := stackViamDefaultsConfig(cfg)
-		if err != nil {
-			errOut = errors.Join(errOut, err)
-		} else {
-			cfg = cfgTmp
-		}
-	}
-
 	// validate/enforce/limit values
 	validatedCfg, err := validateConfig(cfg)
 	errOut = errors.Join(errOut, err)

utils/config_test.go

@@ -2,13 +2,9 @@ package utils
 
 import (
 	"encoding/json"
-	"os"
-	"path/filepath"
 	"testing"
 
-	agentpb "go.viam.com/api/app/agent/v1"
 	"go.viam.com/test"
-	"google.golang.org/protobuf/types/known/structpb"
 )
 
 // basic test for the config structure names.
@@ -90,69 +86,3 @@ func TestConvertJson(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, *newConfig, test.ShouldResemble, testConfig)
 }
-
-func TestStackConfigs(t *testing.T) {
-	tempDir := t.TempDir()
-
-	DefaultsFilePath = filepath.Join(tempDir, "viam-defaults.json")
-
-	jsonBytes := `
-{
-	"advanced_settings": {
-		"debug": false,
-		"viam_server_start_timeout_minutes": 12
-	},
-	"network_configuration": {
-		"manufacturer": "viam",
-		"model": "custom",
-		"fragment_id": "",
-		"hotspot_prefix": "viam-setup",
-		"hotspot_ssid": "viamsetuptestssid123",
-		"hotspot_password": "viamsetup",
-		"disable_captive_portal_redirect": false,
-		"offline_before_starting_hotspot_minutes": 2,
-		"user_idle_minutes": 6,
-		"retry_connection_timeout_minutes": 10,
-		"wifi_power_save": null,
-		"bluetooth_trust_all": false
-	},
-	"system_configuration": {
-			"logging_journald_system_max_use_megabytes": 513,
-			"logging_journald_runtime_max_use_megabytes": 512,
-			"os_auto_upgrade_type": "",
-			"forward_system_logs": ""
-	}
-}
-`
-	viamDefaultsCfg := DefaultConfig()
-	err := json.Unmarshal([]byte(jsonBytes), &viamDefaultsCfg)
-	test.That(t, err, test.ShouldBeNil)
-
-	err = os.WriteFile(DefaultsFilePath, []byte(jsonBytes), 0o644)
-	test.That(t, err, test.ShouldBeNil)
-
-	agentConfigNoCloud, err := StackOfflineConfig()
-	test.That(t, err, test.ShouldBeNil)
-	test.That(t, agentConfigNoCloud, test.ShouldResemble, viamDefaultsCfg)
-
-	// if a readable cloud proto is provided, only its values should be used.
-	// if a value is not provided, it should be the hardcoded default from DefaultConfig. viam-defaults.json should not be used at all
-	adv, err := structpb.NewStruct(map[string]any{
-		"debug": true,
-	})
-	test.That(t, err, test.ShouldBeNil)
-	fromCloudProto := &agentpb.DeviceAgentConfigResponse{
-		AdvancedSettings: adv,
-	}
-	agentCfgFromCloud, err := StackProtoConfig(fromCloudProto)
-	test.That(t, err, test.ShouldBeNil)
-	test.That(t, agentCfgFromCloud.AdvancedSettings.Debug, test.ShouldNotEqual, viamDefaultsCfg.AdvancedSettings.Debug)
-	//nolint:lll
-	test.That(t, agentCfgFromCloud.AdvancedSettings.ViamServerStartTimeoutMinutes, test.ShouldNotEqual, viamDefaultsCfg.AdvancedSettings.ViamServerStartTimeoutMinutes)
-	// defaults to hostname. we don't use field from defaults file.
-	test.That(t, agentCfgFromCloud.NetworkConfiguration.HotspotSSID, test.ShouldNotEqual, viamDefaultsCfg.NetworkConfiguration.HotspotSSID)
-	//nolint:lll
-	test.That(t, agentCfgFromCloud.NetworkConfiguration.UserIdleMinutes, test.ShouldNotEqual, viamDefaultsCfg.NetworkConfiguration.UserIdleMinutes)
-	//nolint:lll
-	test.That(t, agentCfgFromCloud.SystemConfiguration.LoggingJournaldSystemMaxUseMegabytes, test.ShouldNotEqual, viamDefaultsCfg.SystemConfiguration.LoggingJournaldSystemMaxUseMegabytes)
-}