From f553dd496de6b30bf7263424c17e4be4452b593e Mon Sep 17 00:00:00 2001 From: radhika Date: Tue, 13 Oct 2015 17:01:46 -0400 Subject: [PATCH] 7167: Convert blobSigningKey also into local variable and make necessary changes to accommodate this change. Remove the New method added in arvadosclient.go and revert MakeArvadosClient to what it was before. --- sdk/go/arvadosclient/arvadosclient.go | 29 +++++-------- tools/keep-rsync/keep-rsync.go | 62 ++++++++++++++------------- tools/keep-rsync/keep-rsync_test.go | 41 +++++++++--------- 3 files changed, 63 insertions(+), 69 deletions(-) diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go index af8bce4d4a..fae26c1019 100644 --- a/sdk/go/arvadosclient/arvadosclient.go +++ b/sdk/go/arvadosclient/arvadosclient.go @@ -90,30 +90,21 @@ type APIConfig struct { ExternalClient bool } -// Create a new ArvadosClient, initialized with standard Arvados environment variables -// ARVADOS_API_HOST, ARVADOS_API_TOKEN, ARVADOS_API_HOST_INSECURE, ARVADOS_EXTERNAL_CLIENT. +// Create a new ArvadosClient, initialized with standard Arvados environment +// variables ARVADOS_API_HOST, ARVADOS_API_TOKEN, and (optionally) +// ARVADOS_API_HOST_INSECURE. func MakeArvadosClient() (ac ArvadosClient, err error) { - var config APIConfig - config.APIToken = os.Getenv("ARVADOS_API_TOKEN") - config.APIHost = os.Getenv("ARVADOS_API_HOST") - var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$") + insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE")) + external := matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT")) - config.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE")) - config.ExternalClient = matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT")) - - return New(config) -} - -// Create a new ArvadosClient, using the given input parameters. -func New(config APIConfig) (ac ArvadosClient, err error) { ac = ArvadosClient{ - ApiServer: config.APIHost, - ApiToken: config.APIToken, - ApiInsecure: config.APIHostInsecure, + ApiServer: os.Getenv("ARVADOS_API_HOST"), + ApiToken: os.Getenv("ARVADOS_API_TOKEN"), + ApiInsecure: insecure, Client: &http.Client{Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: config.APIHostInsecure}}}, - External: config.ExternalClient} + TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}}}, + External: external} if ac.ApiServer == "" { return ac, MissingArvadosApiHost diff --git a/tools/keep-rsync/keep-rsync.go b/tools/keep-rsync/keep-rsync.go index e848de8b8d..bdfcb276c1 100644 --- a/tools/keep-rsync/keep-rsync.go +++ b/tools/keep-rsync/keep-rsync.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "crypto/tls" "errors" "flag" "fmt" @@ -9,19 +10,16 @@ import ( "git.curoverse.com/arvados.git/sdk/go/keepclient" "io/ioutil" "log" + "net/http" "regexp" "strings" "time" ) -// keep-rsync arguments -var ( - blobSigningKey string -) - func main() { var srcConfigFile, dstConfigFile, srcKeepServicesJSON, dstKeepServicesJSON, prefix string var replications int + var srcBlobSigningKey string flag.StringVar( &srcConfigFile, @@ -67,7 +65,7 @@ func main() { flag.Parse() - srcConfig, dstConfig, err := loadConfig(srcConfigFile, dstConfigFile) + srcConfig, dstConfig, srcBlobSigningKey, _, err := loadConfig(srcConfigFile, dstConfigFile) if err != nil { log.Fatalf("Error loading configuration from files: %s", err.Error()) } @@ -79,43 +77,41 @@ func main() { } // Copy blocks not found in dst from src - err = performKeepRsync(kcSrc, kcDst, prefix) + err = performKeepRsync(kcSrc, kcDst, srcBlobSigningKey, prefix) if err != nil { log.Fatalf("Error while syncing data: %s", err.Error()) } } // Load src and dst config from given files -func loadConfig(srcConfigFile, dstConfigFile string) (srcConfig, dstConfig arvadosclient.APIConfig, err error) { +func loadConfig(srcConfigFile, dstConfigFile string) (srcConfig, dstConfig arvadosclient.APIConfig, srcBlobSigningKey, dstBlobSigningKey string, err error) { if srcConfigFile == "" { - return srcConfig, dstConfig, errors.New("-src-config-file must be specified") + return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, errors.New("-src-config-file must be specified") } - srcConfig, err = readConfigFromFile(srcConfigFile) + srcConfig, srcBlobSigningKey, err = readConfigFromFile(srcConfigFile) if err != nil { - return srcConfig, dstConfig, fmt.Errorf("Error reading source configuration: %v", err) + return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, fmt.Errorf("Error reading source configuration: %v", err) } if dstConfigFile == "" { - return srcConfig, dstConfig, errors.New("-dst-config-file must be specified") + return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, errors.New("-dst-config-file must be specified") } - dstConfig, err = readConfigFromFile(dstConfigFile) + dstConfig, dstBlobSigningKey, err = readConfigFromFile(dstConfigFile) if err != nil { - return srcConfig, dstConfig, fmt.Errorf("Error reading destination configuration: %v", err) + return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, fmt.Errorf("Error reading destination configuration: %v", err) } - return srcConfig, dstConfig, err + return } var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$") // Read config from file -func readConfigFromFile(filename string) (arvadosclient.APIConfig, error) { - var config arvadosclient.APIConfig - +func readConfigFromFile(filename string) (config arvadosclient.APIConfig, blobSigningKey string, err error) { content, err := ioutil.ReadFile(filename) if err != nil { - return config, err + return config, "", err } lines := strings.Split(string(content), "\n") @@ -141,21 +137,29 @@ func readConfigFromFile(filename string) (arvadosclient.APIConfig, error) { blobSigningKey = value } } - return config, nil + return } // Initializes keep-rsync using the config provided func setupKeepClients(srcConfig, dstConfig arvadosclient.APIConfig, srcKeepServicesJSON, dstKeepServicesJSON string, replications int) (kcSrc, kcDst *keepclient.KeepClient, err error) { // arvSrc from srcConfig - arvSrc, err := arvadosclient.New(srcConfig) - if err != nil { - return kcSrc, kcDst, err + arvSrc := arvadosclient.ArvadosClient{ + ApiToken: srcConfig.APIToken, + ApiServer: srcConfig.APIHost, + ApiInsecure: srcConfig.APIHostInsecure, + Client: &http.Client{Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: srcConfig.APIHostInsecure}}}, + External: srcConfig.ExternalClient, } // arvDst from dstConfig - arvDst, err := arvadosclient.New(dstConfig) - if err != nil { - return kcSrc, kcDst, err + arvDst := arvadosclient.ArvadosClient{ + ApiToken: dstConfig.APIToken, + ApiServer: dstConfig.APIHost, + ApiInsecure: dstConfig.APIHostInsecure, + Client: &http.Client{Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: dstConfig.APIHostInsecure}}}, + External: dstConfig.ExternalClient, } // Get default replications value from destination, if it is not already provided @@ -200,7 +204,7 @@ func setupKeepClients(srcConfig, dstConfig arvadosclient.APIConfig, srcKeepServi // Get unique block locators from src and dst // Copy any blocks missing in dst -func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, prefix string) error { +func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, blobSigningKey, prefix string) error { // Get unique locators from src srcIndex, err := getUniqueLocators(kcSrc, prefix) if err != nil { @@ -217,7 +221,7 @@ func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, prefix string) error toBeCopied := getMissingLocators(srcIndex, dstIndex) // Copy each missing block to dst - err = copyBlocksToDst(toBeCopied, kcSrc, kcDst) + err = copyBlocksToDst(toBeCopied, kcSrc, kcDst, blobSigningKey) return err } @@ -253,7 +257,7 @@ func getMissingLocators(srcLocators, dstLocators map[string]bool) []string { } // Copy blocks from src to dst; only those that are missing in dst are copied -func copyBlocksToDst(toBeCopied []string, kcSrc, kcDst *keepclient.KeepClient) error { +func copyBlocksToDst(toBeCopied []string, kcSrc, kcDst *keepclient.KeepClient, blobSigningKey string) error { done := 0 total := len(toBeCopied) diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go index a1091b8f8a..0194f4c175 100644 --- a/tools/keep-rsync/keep-rsync_test.go +++ b/tools/keep-rsync/keep-rsync_test.go @@ -30,6 +30,9 @@ type ServerRequiredSuite struct{} func (s *ServerRequiredSuite) SetUpSuite(c *C) { } +var kcSrc, kcDst *keepclient.KeepClient +var srcKeepServicesJSON, dstKeepServicesJSON, blobSigningKey string + func (s *ServerRequiredSuite) SetUpTest(c *C) { arvadostest.ResetEnv() @@ -46,11 +49,6 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) { arvadostest.StopAPI() } -var kcSrc *keepclient.KeepClient -var kcDst *keepclient.KeepClient -var srcKeepServicesJSON string -var dstKeepServicesJSON string - var testKeepServicesJSON = "{ \"kind\":\"arvados#keepServiceList\", \"etag\":\"\", \"self_link\":\"\", \"offset\":null, \"limit\":null, \"items\":[ { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012340\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012340\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25107, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false }, { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012341\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012341\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25108, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false } ], \"items_available\":2 }" // Testing keep-rsync needs two sets of keep services: src and dst. @@ -272,9 +270,9 @@ func testKeepRsync(c *C, enforcePermissions bool, prefix string) { setupRsync(c, enforcePermissions, true, 1) // setupTestData - setupTestData(c, enforcePermissions, prefix) + setupTestData(c, prefix) - err := performKeepRsync(kcSrc, kcDst, prefix) + err := performKeepRsync(kcSrc, kcDst, blobSigningKey, prefix) c.Check(err, IsNil) // Now GetIndex from dst and verify that all 5 from src and the 2 extra blocks are found @@ -309,7 +307,8 @@ func testKeepRsync(c *C, enforcePermissions bool, prefix string) { // Setup test data in src and dst. var srcLocators, srcLocatorsMatchingPrefix, dstLocators, extraDstLocators []string -func setupTestData(c *C, enforcePermissions bool, indexPrefix string) { + +func setupTestData(c *C, indexPrefix string) { srcLocators = []string{} srcLocatorsMatchingPrefix = []string{} dstLocators = []string{} @@ -349,7 +348,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeSrcKeepservers(c *C) { setupRsync(c, false, false, 1) - err := performKeepRsync(kcSrc, kcDst, "") + err := performKeepRsync(kcSrc, kcDst, "", "") c.Check(strings.HasSuffix(err.Error(), "no such host"), Equals, true) } @@ -360,7 +359,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeDstKeepservers(c *C) { setupRsync(c, false, false, 1) - err := performKeepRsync(kcSrc, kcDst, "") + err := performKeepRsync(kcSrc, kcDst, "", "") c.Check(strings.HasSuffix(err.Error(), "no such host"), Equals, true) } @@ -369,12 +368,12 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorGettingBlockFromSrc(c *C setupRsync(c, true, true, 1) // put some blocks in src and dst - setupTestData(c, true, "") + setupTestData(c, "") // Change blob signing key to a fake key, so that Get from src fails - blobSigningKey = "123456789012345678901234yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc" + blobSigningKey = "thisisfakeblobsigningkey" - err := performKeepRsync(kcSrc, kcDst, "") + err := performKeepRsync(kcSrc, kcDst, blobSigningKey, "") c.Check(strings.HasSuffix(err.Error(), "Block not found"), Equals, true) } @@ -383,12 +382,12 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorPuttingBlockInDst(c *C) setupRsync(c, false, true, 1) // put some blocks in src and dst - setupTestData(c, true, "") + setupTestData(c, "") // Increase Want_replicas on dst to result in insufficient replicas error during Put kcDst.Want_replicas = 2 - err := performKeepRsync(kcSrc, kcDst, "") + err := performKeepRsync(kcSrc, kcDst, blobSigningKey, "") c.Check(strings.HasSuffix(err.Error(), "Could not write sufficient replicas"), Equals, true) } @@ -405,7 +404,7 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) { dstConfigFile := dstFile.Name() // load configuration from those files - srcConfig, dstConfig, err := loadConfig(srcConfigFile, dstConfigFile) + srcConfig, dstConfig, srcBlobSigningKey, _, err := loadConfig(srcConfigFile, dstConfigFile) c.Check(err, IsNil) c.Assert(srcConfig.APIHost, Equals, "testhost") @@ -418,18 +417,18 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) { c.Assert(dstConfig.APIHostInsecure, Equals, true) c.Assert(dstConfig.ExternalClient, Equals, false) - c.Assert(blobSigningKey, Equals, "abcdefg") + c.Assert(srcBlobSigningKey, Equals, "abcdefg") } // Test loadConfig func without setting up the config files func (s *ServerRequiredSuite) TestLoadConfig_MissingSrcConfig(c *C) { - _, _, err := loadConfig("", "") + _, _, _, _, err := loadConfig("", "") c.Assert(err.Error(), Equals, "-src-config-file must be specified") } // Test loadConfig func - error reading src config func (s *ServerRequiredSuite) TestLoadConfig_ErrorLoadingSrcConfig(c *C) { - _, _, err := loadConfig("no-such-config-file", "") + _, _, _, _, err := loadConfig("no-such-config-file", "") c.Assert(strings.HasSuffix(err.Error(), "no such file or directory"), Equals, true) } @@ -441,7 +440,7 @@ func (s *ServerRequiredSuite) TestLoadConfig_MissingDstConfig(c *C) { srcConfigFile := srcFile.Name() // load configuration - _, _, err := loadConfig(srcConfigFile, "") + _, _, _, _, err := loadConfig(srcConfigFile, "") c.Assert(err.Error(), Equals, "-dst-config-file must be specified") } @@ -453,7 +452,7 @@ func (s *ServerRequiredSuite) TestLoadConfig_ErrorLoadingDstConfig(c *C) { srcConfigFile := srcFile.Name() // load configuration - _, _, err := loadConfig(srcConfigFile, "no-such-config-file") + _, _, _, _, err := loadConfig(srcConfigFile, "no-such-config-file") c.Assert(strings.HasSuffix(err.Error(), "no such file or directory"), Equals, true) } -- 2.30.2