From 10440ac12d6771ab80469adf551d2cac8d3461e6 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 30 Aug 2022 11:23:52 -0400 Subject: [PATCH] 17344: Remove client code setting X-External-Client header. It doesn't do anything: Nginx replaces it with 0 or 1 based on remote IP address. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- sdk/go/arvadosclient/arvadosclient.go | 20 +++---------------- sdk/python/arvados/api.py | 3 --- sdk/python/arvados/keep.py | 1 - sdk/python/tests/run_test_server.py | 1 - sdk/python/tests/test_keep_client.py | 1 - .../crunch-dispatch-local.go | 1 - .../crunch-dispatch-slurm.go | 1 - services/keepproxy/keepproxy_test.go | 1 - tools/keep-block-check/keep-block-check.go | 4 ---- .../keep-block-check/keep-block-check_test.go | 2 -- tools/keep-rsync/keep-rsync.go | 4 ---- tools/keep-rsync/keep-rsync_test.go | 3 --- 12 files changed, 3 insertions(+), 39 deletions(-) diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go index 2044df6337..13b3a30ac4 100644 --- a/sdk/go/arvadosclient/arvadosclient.go +++ b/sdk/go/arvadosclient/arvadosclient.go @@ -103,10 +103,6 @@ type ArvadosClient struct { // Client object shared by client requests. Supports HTTP KeepAlive. Client *http.Client - // If true, sets the X-External-Client header to indicate - // the client is outside the cluster. - External bool - // Base URIs of Keep services, e.g., {"https://host1:8443", // "https://host2:8443"}. If this is nil, Keep clients will // use the arvados.v1.keep_services.accessible API to discover @@ -180,7 +176,6 @@ func New(c *arvados.Client) (*ArvadosClient, error) { ApiToken: c.AuthToken, ApiInsecure: c.Insecure, Client: hc, - External: false, Retries: 2, KeepServiceURIs: c.KeepServiceURIs, lastClosedIdlesAt: time.Now(), @@ -191,15 +186,9 @@ func New(c *arvados.Client) (*ArvadosClient, error) { // MakeArvadosClient creates a new ArvadosClient using the standard // environment variables ARVADOS_API_HOST, ARVADOS_API_TOKEN, -// ARVADOS_API_HOST_INSECURE, ARVADOS_EXTERNAL_CLIENT, and -// ARVADOS_KEEP_SERVICES. -func MakeArvadosClient() (ac *ArvadosClient, err error) { - ac, err = New(arvados.NewClientFromEnv()) - if err != nil { - return - } - ac.External = StringBool(os.Getenv("ARVADOS_EXTERNAL_CLIENT")) - return +// ARVADOS_API_HOST_INSECURE, and ARVADOS_KEEP_SERVICES. +func MakeArvadosClient() (*ArvadosClient, error) { + return New(arvados.NewClientFromEnv()) } // CallRaw is the same as Call() but returns a Reader that reads the @@ -280,9 +269,6 @@ func (c *ArvadosClient) CallRaw(method string, resourceType string, uuid string, if c.RequestID != "" { req.Header.Add("X-Request-Id", c.RequestID) } - if c.External { - req.Header.Add("X-External-Client", "1") - } resp, err = c.Client.Do(req) if err != nil { diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py index db1d0f4e12..85d4197588 100644 --- a/sdk/python/arvados/api.py +++ b/sdk/python/arvados/api.py @@ -66,9 +66,6 @@ def _intercept_http_request(self, uri, method="GET", headers={}, **kwargs): self.max_request_size < len(kwargs['body'])): raise apiclient_errors.MediaUploadSizeError("Request size %i bytes exceeds published limit of %i bytes" % (len(kwargs['body']), self.max_request_size)) - if config.get("ARVADOS_EXTERNAL_CLIENT", "") == "true": - headers['X-External-Client'] = '1' - headers['Authorization'] = 'OAuth2 %s' % self.arvados_api_token retryable = method in [ diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py index 7c05cc0a6a..44e9157767 100644 --- a/sdk/python/arvados/keep.py +++ b/sdk/python/arvados/keep.py @@ -159,7 +159,6 @@ class Keep(object): config.get('ARVADOS_API_TOKEN'), config.flag_is_true('ARVADOS_API_HOST_INSECURE'), config.get('ARVADOS_KEEP_PROXY'), - config.get('ARVADOS_EXTERNAL_CLIENT') == 'true', os.environ.get('KEEP_LOCAL_STORE')) if (global_client_object is None) or (cls._last_key != key): global_client_object = KeepClient() diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py index 7147c7aa8f..e5d1d8fa38 100644 --- a/sdk/python/tests/run_test_server.py +++ b/sdk/python/tests/run_test_server.py @@ -926,7 +926,6 @@ class TestCaseWithServers(unittest.TestCase): cls._orig_config = arvados.config.settings().copy() cls._cleanup_funcs = [] os.environ.pop('ARVADOS_KEEP_SERVICES', None) - os.environ.pop('ARVADOS_EXTERNAL_CLIENT', None) for server_kwargs, start_func, stop_func in ( (cls.MAIN_SERVER, run, reset), (cls.WS_SERVER, run_ws, stop_ws), diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py index 722b70b58b..87e4eefb29 100644 --- a/sdk/python/tests/test_keep_client.py +++ b/sdk/python/tests/test_keep_client.py @@ -184,7 +184,6 @@ class KeepProxyTestCase(run_test_server.TestCaseWithServers): cls.api_client = arvados.api('v1') def tearDown(self): - arvados.config.settings().pop('ARVADOS_EXTERNAL_CLIENT', None) super(KeepProxyTestCase, self).tearDown() def test_KeepProxyTest1(self): diff --git a/services/crunch-dispatch-local/crunch-dispatch-local.go b/services/crunch-dispatch-local/crunch-dispatch-local.go index c33c2358ca..e455981891 100644 --- a/services/crunch-dispatch-local/crunch-dispatch-local.go +++ b/services/crunch-dispatch-local/crunch-dispatch-local.go @@ -102,7 +102,6 @@ func main() { if client.Insecure { os.Setenv("ARVADOS_API_HOST_INSECURE", "1") } - os.Setenv("ARVADOS_EXTERNAL_CLIENT", "") } else { logger.Warnf("Client credentials missing from config, so falling back on environment variables (deprecated).") } diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go index c774584d68..ac394e1149 100644 --- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go +++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go @@ -101,7 +101,6 @@ func (disp *Dispatcher) configure() error { if disp.Client.Insecure { os.Setenv("ARVADOS_API_HOST_INSECURE", "1") } - os.Setenv("ARVADOS_EXTERNAL_CLIENT", "") for k, v := range disp.cluster.Containers.SLURM.SbatchEnvironmentVariables { os.Setenv(k, v) } diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go index 8242f5b2b5..2eaea27816 100644 --- a/services/keepproxy/keepproxy_test.go +++ b/services/keepproxy/keepproxy_test.go @@ -146,7 +146,6 @@ func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool, kp *ar TestProxyUUID: "http://" + srv.Addr, } kc.SetServiceRoots(sr, sr, sr) - kc.Arvados.External = true return srv, kc, logbuf } diff --git a/tools/keep-block-check/keep-block-check.go b/tools/keep-block-check/keep-block-check.go index 995a1fd559..6127485835 100644 --- a/tools/keep-block-check/keep-block-check.go +++ b/tools/keep-block-check/keep-block-check.go @@ -108,7 +108,6 @@ type apiConfig struct { APIToken string APIHost string APIHostInsecure bool - ExternalClient bool } // Load config from given file @@ -152,8 +151,6 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin config.APIHost = value case "ARVADOS_API_HOST_INSECURE": config.APIHostInsecure = arvadosclient.StringBool(value) - case "ARVADOS_EXTERNAL_CLIENT": - config.ExternalClient = arvadosclient.StringBool(value) case "ARVADOS_BLOB_SIGNING_KEY": blobSigningKey = value } @@ -171,7 +168,6 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, blobSignatureTTL ApiInsecure: config.APIHostInsecure, Client: &http.Client{Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: config.APIHostInsecure}}}, - External: config.ExternalClient, } // If keepServicesJSON is provided, use it instead of service discovery diff --git a/tools/keep-block-check/keep-block-check_test.go b/tools/keep-block-check/keep-block-check_test.go index d973e06027..4dcb47a8da 100644 --- a/tools/keep-block-check/keep-block-check_test.go +++ b/tools/keep-block-check/keep-block-check_test.go @@ -118,7 +118,6 @@ func setupConfigFile(c *C, fileName string) string { fileContent += "ARVADOS_API_TOKEN=" + arvadostest.DataManagerToken + "\n" fileContent += "\n" fileContent += "ARVADOS_API_HOST_INSECURE=" + os.Getenv("ARVADOS_API_HOST_INSECURE") + "\n" - fileContent += " ARVADOS_EXTERNAL_CLIENT = false \n" fileContent += " NotANameValuePairAndShouldGetIgnored \n" fileContent += "ARVADOS_BLOB_SIGNING_KEY=abcdefg\n" @@ -291,7 +290,6 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) { c.Assert(config.APIHost, Equals, os.Getenv("ARVADOS_API_HOST")) c.Assert(config.APIToken, Equals, arvadostest.DataManagerToken) c.Assert(config.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))) - c.Assert(config.ExternalClient, Equals, false) c.Assert(blobSigningKey, Equals, "abcdefg") } diff --git a/tools/keep-rsync/keep-rsync.go b/tools/keep-rsync/keep-rsync.go index 98c9609cb3..7e519f775b 100644 --- a/tools/keep-rsync/keep-rsync.go +++ b/tools/keep-rsync/keep-rsync.go @@ -118,7 +118,6 @@ type apiConfig struct { APIToken string APIHost string APIHostInsecure bool - ExternalClient bool } // Load src and dst config from given files @@ -164,8 +163,6 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin config.APIHost = value case "ARVADOS_API_HOST_INSECURE": config.APIHostInsecure = arvadosclient.StringBool(value) - case "ARVADOS_EXTERNAL_CLIENT": - config.ExternalClient = arvadosclient.StringBool(value) case "ARVADOS_BLOB_SIGNING_KEY": blobSigningKey = value } @@ -181,7 +178,6 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, repl ApiInsecure: config.APIHostInsecure, Client: &http.Client{Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: config.APIHostInsecure}}}, - External: config.ExternalClient, } // If keepServicesJSON is provided, use it instead of service discovery diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go index 45ed3f67f5..dc5b957125 100644 --- a/tools/keep-rsync/keep-rsync_test.go +++ b/tools/keep-rsync/keep-rsync_test.go @@ -367,7 +367,6 @@ func (s *ServerNotRequiredSuite) TestLoadConfig(c *C) { c.Assert(srcConfig.APIHost, Equals, os.Getenv("ARVADOS_API_HOST")) c.Assert(srcConfig.APIToken, Equals, arvadostest.SystemRootToken) c.Assert(srcConfig.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))) - c.Assert(srcConfig.ExternalClient, Equals, false) dstConfig, _, err := loadConfig(dstConfigFile) c.Check(err, IsNil) @@ -375,7 +374,6 @@ func (s *ServerNotRequiredSuite) TestLoadConfig(c *C) { c.Assert(dstConfig.APIHost, Equals, os.Getenv("ARVADOS_API_HOST")) c.Assert(dstConfig.APIToken, Equals, arvadostest.SystemRootToken) c.Assert(dstConfig.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))) - c.Assert(dstConfig.ExternalClient, Equals, false) c.Assert(srcBlobSigningKey, Equals, "abcdefg") } @@ -412,7 +410,6 @@ func setupConfigFile(c *C, name string) *os.File { fileContent := "ARVADOS_API_HOST=" + os.Getenv("ARVADOS_API_HOST") + "\n" fileContent += "ARVADOS_API_TOKEN=" + arvadostest.SystemRootToken + "\n" fileContent += "ARVADOS_API_HOST_INSECURE=" + os.Getenv("ARVADOS_API_HOST_INSECURE") + "\n" - fileContent += "ARVADOS_EXTERNAL_CLIENT=false\n" fileContent += "ARVADOS_BLOB_SIGNING_KEY=abcdefg" _, err = file.Write([]byte(fileContent)) -- 2.30.2