7167: Convert blobSigningKey also into local variable and make necessary changes...

Remove the New method added in arvadosclient.go and revert MakeArvadosClient to what it was before.

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index af8bce4d4a7336cac0c66fb07292aaa203b4b0e6..fae26c101958dfd235ca0e7129b98ad20afac593 100644 (file)
--- 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 e848de8b8de425cdf2f50bdc2025966e3f6db084..bdfcb276c10bb3a3a45eac5165b256885d24fad7 100644 (file)
--- 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 a1091b8f8a601e94767dc696da39fb3805446d60..0194f4c175df78d9195e90a5934b2dd35d8074d6 100644 (file)
--- 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)
 }