From 7f0c951d032ce2eb1097aaf680e4955dae264969 Mon Sep 17 00:00:00 2001 From: mirokuratczyk Date: Fri, 24 Jan 2020 11:13:52 -0500 Subject: [PATCH] Make config migration optional - Allows tests to skip migration attempts --- ClientLibrary/clientlib/clientlib.go | 2 +- ConsoleClient/main.go | 2 +- MobileLibrary/psi/psi.go | 2 +- psiphon/config.go | 60 +++++++++++++++------------- psiphon/config_test.go | 22 +++++----- psiphon/controller_test.go | 2 +- psiphon/dataStoreRecovery_test.go | 2 +- psiphon/dialParameters_test.go | 2 +- psiphon/exchange_test.go | 2 +- psiphon/feedback.go | 2 +- psiphon/limitProtocols_test.go | 2 +- psiphon/memory_test/memory_test.go | 2 +- psiphon/remoteServerList_test.go | 4 +- psiphon/server/server_test.go | 6 +-- psiphon/server/sessionID_test.go | 2 +- psiphon/userAgent_test.go | 2 +- 16 files changed, 60 insertions(+), 56 deletions(-) diff --git a/ClientLibrary/clientlib/clientlib.go b/ClientLibrary/clientlib/clientlib.go index 1577de0d3..8b8f9a6af 100644 --- a/ClientLibrary/clientlib/clientlib.go +++ b/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") } diff --git a/ConsoleClient/main.go b/ConsoleClient/main.go index 509890066..451714fea 100644 --- a/ConsoleClient/main.go +++ b/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) diff --git a/MobileLibrary/psi/psi.go b/MobileLibrary/psi/psi.go index 3a78d19d9..1a4bff7a7 100644 --- a/MobileLibrary/psi/psi.go +++ b/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) } diff --git a/psiphon/config.go b/psiphon/config.go index 6ab7fd9da..9905fa033 100755 --- a/psiphon/config.go +++ b/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. diff --git a/psiphon/config_test.go b/psiphon/config_test.go index 5ecaa29a7..01974dfb4 100644 --- a/psiphon/config_test.go +++ b/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 diff --git a/psiphon/controller_test.go b/psiphon/controller_test.go index 8f762753e..26c2fdf9a 100644 --- a/psiphon/controller_test.go +++ b/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) } diff --git a/psiphon/dataStoreRecovery_test.go b/psiphon/dataStoreRecovery_test.go index b8a36a089..7231a511d 100644 --- a/psiphon/dataStoreRecovery_test.go +++ b/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) } diff --git a/psiphon/dialParameters_test.go b/psiphon/dialParameters_test.go index 27570bfd3..f9ab87364 100644 --- a/psiphon/dialParameters_test.go +++ b/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) } diff --git a/psiphon/exchange_test.go b/psiphon/exchange_test.go index 36fc8641d..bc6bd501f 100644 --- a/psiphon/exchange_test.go +++ b/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) } diff --git a/psiphon/feedback.go b/psiphon/feedback.go index c70dbd1f4..9d25edbed 100644 --- a/psiphon/feedback.go +++ b/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) } diff --git a/psiphon/limitProtocols_test.go b/psiphon/limitProtocols_test.go index bd74660b9..fb943b733 100644 --- a/psiphon/limitProtocols_test.go +++ b/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) } diff --git a/psiphon/memory_test/memory_test.go b/psiphon/memory_test/memory_test.go index 57df4644b..670bd0da2 100644 --- a/psiphon/memory_test/memory_test.go +++ b/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) } diff --git a/psiphon/remoteServerList_test.go b/psiphon/remoteServerList_test.go index 5bb7ab0c2..45cfe27fe 100644 --- a/psiphon/remoteServerList_test.go +++ b/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) } diff --git a/psiphon/server/server_test.go b/psiphon/server/server_test.go index be9752f62..c0e443236 100644 --- a/psiphon/server/server_test.go +++ b/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) } diff --git a/psiphon/server/sessionID_test.go b/psiphon/server/sessionID_test.go index 46d6d509c..df97fecb0 100644 --- a/psiphon/server/sessionID_test.go +++ b/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) } diff --git a/psiphon/userAgent_test.go b/psiphon/userAgent_test.go index 3fb72f553..21354c684 100644 --- a/psiphon/userAgent_test.go +++ b/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) }