Make config migration optional

- Allows tests to skip migration attempts

ClientLibrary/clientlib/clientlib.go

@@ -156,7 +156,7 @@ func StartTunnel(ctx context.Context,
 
 	// config.Commit must be called before calling config.SetClientParameters
 	// or attempting to connect.
-	err = config.Commit()
+	err = config.Commit(true)
 	if err != nil {
 		return nil, errors.TraceMsg(err, "config.Commit failed")
 	}

ConsoleClient/main.go

@@ -209,7 +209,7 @@ func main() {
 
 	// All config fields should be set before calling Commit.
 
-	err = config.Commit()
+	err = config.Commit(true)
 	if err != nil {
 		psiphon.SetEmitDiagnosticNotices(true, false)
 		psiphon.NoticeError("error loading configuration file: %s", err)

MobileLibrary/psi/psi.go

@@ -152,7 +152,7 @@ func Start(
 
 	// All config fields should be set before calling Commit.
 
-	err = config.Commit()
+	err = config.Commit(true)
 	if err != nil {
 		return fmt.Errorf("error committing configuration file: %s", err)
 	}

psiphon/config.go

@@ -765,7 +765,10 @@ func (config *Config) IsCommitted() bool {
 // Config fields should not be set after calling Config, as any changes may
 // not be reflected in internal data structures.
 //
-// File migrations:
+// If migrateFromLegacyFields is set to true, then an attempt to migrate from
+// legacy fields is made.
+//
+// Migration from legacy fields:
 // Config fields of the naming Migrate* (e.g. MigrateDataStoreDirectory) specify
 // a file migration operation which should be performed. These fields correspond
 // to deprecated fields, which previously could be used to specify where Psiphon
@@ -793,7 +796,7 @@ func (config *Config) IsCommitted() bool {
 // fails. It is better to not endlessly retry file migrations on each Commit()
 // because file system errors are expected to be rare and persistent files will
 // be re-populated over time.
-func (config *Config) Commit() error {
+func (config *Config) Commit(migrateFromLegacyFields bool) error {
 
 	// Do SetEmitDiagnosticNotices first, to ensure config file errors are
 	// emitted.
@@ -842,26 +845,28 @@ func (config *Config) Commit() error {
 	homepageFilePath := config.GetHomePageFilename()
 	noticesFilePath := config.GetNoticesFilename()
 
-	if needMigration {
+	if migrateFromLegacyFields {
+		if needMigration {
 
-		// Move notice files that exist at legacy file paths under the data root
-		// directory.
+			// Move notice files that exist at legacy file paths under the data root
+			// directory.
 
-		noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: need migration")
-		noticeMigrations := migrationsFromLegacyNoticeFilePaths(config)
+			noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: need migration")
+			noticeMigrations := migrationsFromLegacyNoticeFilePaths(config)
 
-		for _, migration := range noticeMigrations {
-			err := common.DoFileMigration(migration)
-			if err != nil {
-				alertMsg := fmt.Sprintf("Config migration: %s", errors.Trace(err))
-				noticeMigrationAlertMsgs = append(noticeMigrationAlertMsgs, alertMsg)
-			} else {
-				infoMsg := fmt.Sprintf("Config migration: moved %s to %s", migration.OldPath, migration.NewPath)
-				noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, infoMsg)
+			for _, migration := range noticeMigrations {
+				err := common.DoFileMigration(migration)
+				if err != nil {
+					alertMsg := fmt.Sprintf("Config migration: %s", errors.Trace(err))
+					noticeMigrationAlertMsgs = append(noticeMigrationAlertMsgs, alertMsg)
+				} else {
+					infoMsg := fmt.Sprintf("Config migration: moved %s to %s", migration.OldPath, migration.NewPath)
+					noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, infoMsg)
+				}
 			}
+		} else {
+			noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: migration already completed")
 		}
-	} else {
-		noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: migration already completed")
 	}
 
 	if config.UseNoticeFiles != nil {
@@ -1093,19 +1098,18 @@ func (config *Config) Commit() error {
 
 	// Migrate from old config fields. This results in files being moved under
 	// a config specified data root directory.
+	if migrateFromLegacyFields && needMigration {
 
-	// If unset, set MigrateDataStoreDirectory to the previous default value for
-	// DataStoreDirectory to ensure that datastore files are migrated.
-	if config.MigrateDataStoreDirectory == "" {
-		wd, err := os.Getwd()
-		if err != nil {
-			return errors.Trace(err)
+		// If unset, set MigrateDataStoreDirectory to the previous default value for
+		// DataStoreDirectory to ensure that datastore files are migrated.
+		if config.MigrateDataStoreDirectory == "" {
+			wd, err := os.Getwd()
+			if err != nil {
+				return errors.Trace(err)
+			}
+			NoticeInfo("MigrateDataStoreDirectory unset, using working directory %s", wd)
+			config.MigrateDataStoreDirectory = wd
 		}
-		NoticeInfo("MigrateDataStoreDirectory unset, using working directory %s", wd)
-		config.MigrateDataStoreDirectory = wd
-	}
-
-	if needMigration {
 
 		// Move files that exist at legacy file paths under the data root
 		// directory.

psiphon/config_test.go

@@ -104,7 +104,7 @@ func TestConfigTestSuite(t *testing.T) {
 func (suite *ConfigTestSuite) Test_LoadConfig_BasicGood() {
 	config, err := LoadConfig(suite.confStubBlob)
 	if err == nil {
-		err = config.Commit()
+		err = config.Commit(false)
 	}
 	suite.Nil(err, "a basic config should succeed")
 }
@@ -129,7 +129,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 			`{"f1": 11, "f2": "two", "DataRootDirectory" : %s}`,
 			suite.testDirectory)))
 	if err == nil {
-		err = config.Commit()
+		err = config.Commit(false)
 	}
 	suite.NotNil(err, "JSON with none of our fields should fail")
 
@@ -141,7 +141,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 		testObjJSON, _ = json.Marshal(testObj)
 		config, err = LoadConfig(testObjJSON)
 		if err == nil {
-			err = config.Commit()
+			err = config.Commit(false)
 		}
 		suite.NotNil(err, "JSON with one of our required fields missing should fail: %s", field)
 
@@ -151,7 +151,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 		testObjJSON, _ = json.Marshal(testObj)
 		config, err = LoadConfig(testObjJSON)
 		if err == nil {
-			err = config.Commit()
+			err = config.Commit(false)
 		}
 		suite.NotNil(err, "JSON with one of our required fields with the wrong type should fail: %s", field)
 
@@ -161,7 +161,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 		testObjJSON, _ = json.Marshal(testObj)
 		config, err = LoadConfig(testObjJSON)
 		if err == nil {
-			err = config.Commit()
+			err = config.Commit(false)
 		}
 		suite.NotNil(err, "JSON with one of our required fields set to null should fail: %s", field)
 
@@ -171,7 +171,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 		testObjJSON, _ = json.Marshal(testObj)
 		config, err = LoadConfig(testObjJSON)
 		if err == nil {
-			err = config.Commit()
+			err = config.Commit(false)
 		}
 		suite.NotNil(err, "JSON with one of our required fields set to an empty string should fail: %s", field)
 	}
@@ -184,7 +184,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_BadJson() {
 		testObjJSON, _ = json.Marshal(testObj)
 		config, err = LoadConfig(testObjJSON)
 		if err == nil {
-			err = config.Commit()
+			err = config.Commit(false)
 		}
 		suite.NotNil(err, "JSON with one of our optional fields with the wrong type should fail: %s", field)
 	}
@@ -205,14 +205,14 @@ func (suite *ConfigTestSuite) Test_LoadConfig_GoodJson() {
 	testObjJSON, _ = json.Marshal(testObj)
 	config, err := LoadConfig(testObjJSON)
 	if err == nil {
-		err = config.Commit()
+		err = config.Commit(false)
 	}
 	suite.Nil(err, "JSON with good values for our required fields but no optional fields should succeed")
 
 	// Has all of our required fields, and all optional fields
 	config, err = LoadConfig(suite.confStubBlob)
 	if err == nil {
-		err = config.Commit()
+		err = config.Commit(false)
 	}
 	suite.Nil(err, "JSON with all good values for required and optional fields should succeed")
 
@@ -224,7 +224,7 @@ func (suite *ConfigTestSuite) Test_LoadConfig_GoodJson() {
 	testObjJSON, _ = json.Marshal(testObj)
 	config, err = LoadConfig(testObjJSON)
 	if err == nil {
-		err = config.Commit()
+		err = config.Commit(false)
 	}
 	suite.Nil(err, "JSON with null for optional values should succeed")
 }
@@ -402,7 +402,7 @@ func LoadConfigMigrateTest(oslDirChildrenPreMigration []FileTree, oslDirChildren
 	}
 
 	// Commit config, this is where file migration happens
-	err = config.Commit()
+	err = config.Commit(true)
 	if err != nil {
 		suite.T().Fatal("Error committing config:", err)
 		return

psiphon/controller_test.go

@@ -561,7 +561,7 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 	}
 
 	// All config fields should be set before calling Commit.
-	err = config.Commit()
+	err = config.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}

psiphon/dataStoreRecovery_test.go

@@ -66,7 +66,7 @@ func TestBoltResiliency(t *testing.T) {
 	if err != nil {
 		t.Fatalf("LoadConfig failed: %s", err)
 	}
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("Commit failed: %s", err)
 	}

psiphon/dialParameters_test.go

@@ -71,7 +71,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 		NetworkIDGetter:      new(testNetworkGetter),
 	}
 
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}

psiphon/exchange_test.go

@@ -82,7 +82,7 @@ func TestServerEntryExchange(t *testing.T) {
 	if err != nil {
 		t.Fatalf("LoadConfig failed: %s", err)
 	}
-	err = config.Commit()
+	err = config.Commit(false)
 	if err != nil {
 		t.Fatalf("Commit failed: %s", err)
 	}

psiphon/feedback.go

@@ -107,7 +107,7 @@ func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer
 	if err != nil {
 		return errors.Trace(err)
 	}
-	err = config.Commit()
+	err = config.Commit(true)
 	if err != nil {
 		return errors.Trace(err)
 	}

psiphon/limitProtocols_test.go

@@ -109,7 +109,7 @@ func TestLimitTunnelProtocols(t *testing.T) {
 
 	clientConfig.DataRootDirectory = testDataDirName
 
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}

psiphon/memory_test/memory_test.go

@@ -115,7 +115,7 @@ func runMemoryTest(t *testing.T, testMode int) {
 	if err != nil {
 		t.Fatalf("error processing configuration file: %s", err)
 	}
-	err = config.Commit()
+	err = config.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}

psiphon/remoteServerList_test.go

@@ -204,7 +204,7 @@ func testObfuscatedRemoteServerLists(t *testing.T, omitMD5Sums bool) {
 		DataRootDirectory:    testDataDirName,
 		PropagationChannelId: "0",
 		SponsorId:            "0"}
-	err = config.Commit()
+	err = config.Commit(false)
 	if err != nil {
 		t.Fatalf("Error initializing config: %s", err)
 	}
@@ -403,7 +403,7 @@ func testObfuscatedRemoteServerLists(t *testing.T, omitMD5Sums bool) {
 	if err != nil {
 		t.Fatalf("error processing configuration file: %s", err)
 	}
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}

psiphon/server/server_test.go

@@ -853,7 +853,7 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 		clientConfig.Authorizations = []string{clientAuthorization}
 	}
 
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}
@@ -2023,7 +2023,7 @@ func storePruneServerEntriesTest(
 		// working directory.
 		DataRootDirectory: testDataDirName,
 	}
-	err := clientConfig.Commit()
+	err := clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("Commit failed: %s", err)
 	}
@@ -2100,7 +2100,7 @@ func checkPruneServerEntriesTest(
 		// working directory.
 		DataRootDirectory: testDataDirName,
 	}
-	err := clientConfig.Commit()
+	err := clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("Commit failed: %s", err)
 	}

psiphon/server/sessionID_test.go

@@ -139,7 +139,7 @@ func TestDuplicateSessionID(t *testing.T) {
 	if err != nil {
 		t.Fatalf("LoadConfig failed: %s", err)
 	}
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("Commit failed: %s", err)
 	}

psiphon/userAgent_test.go

@@ -209,7 +209,7 @@ func attemptConnectionsWithUserAgent(
 	clientConfig.TunnelProtocol = tunnelProtocol
 	clientConfig.DataRootDirectory = testDataDirName
 
-	err = clientConfig.Commit()
+	err = clientConfig.Commit(false)
 	if err != nil {
 		t.Fatalf("error committing configuration file: %s", err)
 	}