From a9061e5453b7ea718ce9cc26007a7d3111b64038 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Sat, 9 May 2020 11:27:27 +0800 Subject: [PATCH 1/3] cloudprovider: add methods for getting http client Prepare for following changes to avoid using and changing default client settings --- pkg/cloudprovider/cloudprovider.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index 46b38ac7c8..1795182f4c 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -17,6 +17,8 @@ package cloudprovider import ( "context" "fmt" + "net/http" + "time" "yunion.io/x/jsonutils" "yunion.io/x/log" @@ -155,6 +157,18 @@ type ProviderConfig struct { ProxyFunc httputils.TransportProxyFunc } +func (cp *ProviderConfig) HttpClient() *http.Client { + client := httputils.GetClient(true, 15*time.Second) + httputils.SetClientProxyFunc(client, cp.ProxyFunc) + return client +} + +func (cp *ProviderConfig) AdaptiveTimeoutHttpClient() *http.Client { + client := httputils.GetAdaptiveTimeoutClient() + httputils.SetClientProxyFunc(client, cp.ProxyFunc) + return client +} + type ICloudProviderFactory interface { GetProvider(cfg ProviderConfig) (ICloudProvider, error) From fd3c9e5cd810fdc2487566d872ab0028e4f1fcab Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Sat, 9 May 2020 11:28:10 +0800 Subject: [PATCH 2/3] multicloud: avoid changing transport of global default http client --- pkg/multicloud/aliyun/aliyun.go | 3 +-- pkg/multicloud/aws/aws.go | 4 +--- pkg/multicloud/azure/azure.go | 4 +--- pkg/multicloud/ctyun/ctyun.go | 3 +-- pkg/multicloud/google/google.go | 3 +-- pkg/multicloud/huawei/huawei.go | 7 ++----- pkg/multicloud/qcloud/qcloud.go | 4 +--- pkg/multicloud/ucloud/ucloud.go | 5 +---- pkg/multicloud/zstack/zstack.go | 4 +--- 9 files changed, 10 insertions(+), 27 deletions(-) diff --git a/pkg/multicloud/aliyun/aliyun.go b/pkg/multicloud/aliyun/aliyun.go index 4740c6e250..cc67c191b6 100644 --- a/pkg/multicloud/aliyun/aliyun.go +++ b/pkg/multicloud/aliyun/aliyun.go @@ -276,8 +276,7 @@ func (client *SAliyunClient) getOssClient(regionId string) (*oss.Client, error) // which can be used to whitelist ips, domains from http_proxy, // https_proxy setting // oss use no timeout client so as to send/download large files - httpClient := httputils.GetAdaptiveTimeoutClient() - httputils.SetClientProxyFunc(httpClient, client.cpcfg.ProxyFunc) + httpClient := client.cpcfg.AdaptiveTimeoutHttpClient() cliOpts := []oss.ClientOption{ oss.HTTPClient(httpClient), } diff --git a/pkg/multicloud/aws/aws.go b/pkg/multicloud/aws/aws.go index 41d3d421e8..3dafd114d8 100644 --- a/pkg/multicloud/aws/aws.go +++ b/pkg/multicloud/aws/aws.go @@ -29,7 +29,6 @@ import ( api "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudprovider" - "yunion.io/x/onecloud/pkg/util/httputils" ) const ( @@ -189,8 +188,7 @@ func (self *SAwsClient) fetchRegions() error { } func (client *SAwsClient) getAwsSession(regionId string) (*session.Session, error) { - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, client.cpcfg.ProxyFunc) + httpClient := client.cpcfg.HttpClient() return session.NewSession(&sdk.Config{ Region: sdk.String(regionId), Credentials: credentials.NewStaticCredentials( diff --git a/pkg/multicloud/azure/azure.go b/pkg/multicloud/azure/azure.go index 5bf7a8f8ee..ca7670e3a3 100644 --- a/pkg/multicloud/azure/azure.go +++ b/pkg/multicloud/azure/azure.go @@ -33,7 +33,6 @@ import ( api "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudprovider" - "yunion.io/x/onecloud/pkg/util/httputils" ) const ( @@ -153,8 +152,7 @@ func (self *SAzureClient) getDefaultClient() (*autorest.Client, error) { return nil, err } - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, self.cpcfg.ProxyFunc) + httpClient := self.cpcfg.HttpClient() client.Sender = httpClient self.env = env diff --git a/pkg/multicloud/ctyun/ctyun.go b/pkg/multicloud/ctyun/ctyun.go index 5edfc32970..6b9dc1cb88 100644 --- a/pkg/multicloud/ctyun/ctyun.go +++ b/pkg/multicloud/ctyun/ctyun.go @@ -85,8 +85,7 @@ type SCtyunClient struct { } func NewSCtyunClient(cfg *CtyunClientConfig) (*SCtyunClient, error) { - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, cfg.cpcfg.ProxyFunc) + httpClient := cfg.cpcfg.HttpClient() client := &SCtyunClient{ CtyunClientConfig: cfg, httpClient: httpClient, diff --git a/pkg/multicloud/google/google.go b/pkg/multicloud/google/google.go index e2753bde39..41fa1e70f8 100644 --- a/pkg/multicloud/google/google.go +++ b/pkg/multicloud/google/google.go @@ -136,8 +136,7 @@ func NewGoogleClient(cfg *GoogleClientConfig) (*SGoogleClient, error) { TokenURL: google.JWTTokenURL, } - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, cfg.cpcfg.ProxyFunc) + httpClient := cfg.cpcfg.HttpClient() ctx := context.Background() ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient) diff --git a/pkg/multicloud/huawei/huawei.go b/pkg/multicloud/huawei/huawei.go index a96b2c34e8..338649bbbf 100644 --- a/pkg/multicloud/huawei/huawei.go +++ b/pkg/multicloud/huawei/huawei.go @@ -27,7 +27,6 @@ import ( "yunion.io/x/onecloud/pkg/multicloud/huawei/client/auth" "yunion.io/x/onecloud/pkg/multicloud/huawei/client/auth/credentials" "yunion.io/x/onecloud/pkg/multicloud/huawei/obs" - "yunion.io/x/onecloud/pkg/util/httputils" ) /* @@ -143,8 +142,7 @@ func (self *SHuaweiClient) newRegionAPIClient(regionId string) (*client.Client, return nil, err } - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, self.cpcfg.ProxyFunc) + httpClient := self.cpcfg.HttpClient() cli.SetHttpClient(httpClient) return cli, nil @@ -156,8 +154,7 @@ func (self *SHuaweiClient) newGeneralAPIClient() (*client.Client, error) { return nil, err } - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, self.cpcfg.ProxyFunc) + httpClient := self.cpcfg.HttpClient() cli.SetHttpClient(httpClient) return cli, nil diff --git a/pkg/multicloud/qcloud/qcloud.go b/pkg/multicloud/qcloud/qcloud.go index 91b44f9411..0380bc3147 100644 --- a/pkg/multicloud/qcloud/qcloud.go +++ b/pkg/multicloud/qcloud/qcloud.go @@ -429,13 +429,11 @@ func (client *SQcloudClient) GetRegions() []SRegion { } func (client *SQcloudClient) getDefaultClient() (*common.Client, error) { - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, client.cpcfg.ProxyFunc) - cli, err := common.NewClientWithSecretId(client.secretId, client.secretKey, QCLOUD_DEFAULT_REGION) if err != nil { return nil, err } + httpClient := client.cpcfg.HttpClient() cli.WithHttpTransport(httpClient.Transport) return cli, nil } diff --git a/pkg/multicloud/ucloud/ucloud.go b/pkg/multicloud/ucloud/ucloud.go index 98f26e6d5a..b3a07968c4 100644 --- a/pkg/multicloud/ucloud/ucloud.go +++ b/pkg/multicloud/ucloud/ucloud.go @@ -24,7 +24,6 @@ import ( api "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudprovider" - "yunion.io/x/onecloud/pkg/util/httputils" ) /* @@ -102,9 +101,7 @@ type SUcloudClient struct { // 进行资源操作时参数account 对应数据库cloudprovider表中的account字段,由accessKey和projectID两部分组成，通过"/"分割。 // 初次导入Subaccount时，参数account对应cloudaccounts表中的account字段，即accesskey。此时projectID为空，只能进行同步子账号（项目）、查询region列表等projectId无关的操作。 func NewUcloudClient(cfg *UcloudClientConfig) (*SUcloudClient, error) { - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, cfg.cpcfg.ProxyFunc) - + httpClient := cfg.cpcfg.HttpClient() client := SUcloudClient{ UcloudClientConfig: cfg, httpClient: httpClient, diff --git a/pkg/multicloud/zstack/zstack.go b/pkg/multicloud/zstack/zstack.go index c2967aa880..b71a175846 100644 --- a/pkg/multicloud/zstack/zstack.go +++ b/pkg/multicloud/zstack/zstack.go @@ -103,9 +103,7 @@ func getSignUrl(uri string) (string, error) { } func NewZStackClient(cfg *ZstackClientConfig) (*SZStackClient, error) { - httpClient := httputils.GetDefaultClient() - httputils.SetClientProxyFunc(httpClient, cfg.cpcfg.ProxyFunc) - + httpClient := cfg.cpcfg.HttpClient() cli := &SZStackClient{ ZstackClientConfig: cfg, httpClient: httpClient, From e711ae7648571b74b30bb03d2d14eb5bdb66fd10 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Sat, 9 May 2020 11:37:39 +0800 Subject: [PATCH 3/3] multicloud: loader: guard against changing proxyFunc of default client --- pkg/multicloud/loader/proxyfunc_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/multicloud/loader/proxyfunc_test.go b/pkg/multicloud/loader/proxyfunc_test.go index fb128e4b1c..b5f4975065 100644 --- a/pkg/multicloud/loader/proxyfunc_test.go +++ b/pkg/multicloud/loader/proxyfunc_test.go @@ -21,6 +21,7 @@ import ( "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudprovider" + "yunion.io/x/onecloud/pkg/util/httputils" ) func TestProxyFunc(t *testing.T) { @@ -39,6 +40,14 @@ func TestProxyFunc(t *testing.T) { } t.Errorf("vendor %s: proxyFunc not working", vendor) } + t.Run("default client no proxy", func(t *testing.T) { + proxied = false + client := httputils.GetDefaultClient() + client.Get("http://default-client-no-proxy.TestProxyFunc." + vendor + "/") + if proxied { + t.Errorf("%s: default client proxy changed", vendor) + } + }) } t.Parallel()