From ab1aaefe06c9a5aaf2c8d712cd23f9cff55106b9 Mon Sep 17 00:00:00 2001 From: shimako55 Date: Tue, 20 Feb 2024 20:26:32 +0900 Subject: [PATCH 1/3] feat: Add proxy support to Argo CLI API client Signed-off-by: shimako55 --- cmd/argo/commands/client/conn.go | 14 +++++++++++++- pkg/apiclient/apiclient.go | 5 ++++- pkg/apiclient/http1-client.go | 6 +++--- pkg/apiclient/http1/facade.go | 15 +++++++++++++-- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/cmd/argo/commands/client/conn.go b/cmd/argo/commands/client/conn.go index cdf7f4783f04..68c8443b29c0 100644 --- a/cmd/argo/commands/client/conn.go +++ b/cmd/argo/commands/client/conn.go @@ -3,6 +3,8 @@ package client import ( "context" "fmt" + "net/http" + "net/url" "os" log "github.com/sirupsen/logrus" @@ -55,6 +57,14 @@ func AddAPIClientFlagsToCmd(cmd *cobra.Command) { } func NewAPIClient(ctx context.Context) (context.Context, apiclient.Client, error) { + var proxy func(*http.Request) (*url.URL, error) + if overrides.ClusterInfo.ProxyURL != "" { + proxyURL, err := url.Parse(overrides.ClusterInfo.ProxyURL) + if err != nil { + return nil, nil, err + } + proxy = http.ProxyURL(proxyURL) + } return apiclient.NewClientFromOpts( apiclient.Opts{ ArgoServerOpts: ArgoServerOpts, @@ -70,7 +80,9 @@ func NewAPIClient(ctx context.Context) (context.Context, apiclient.Client, error Offline: Offline, OfflineFiles: OfflineFiles, Context: ctx, - }) + Proxy: proxy, + } + ) } func Namespace() string { diff --git a/pkg/apiclient/apiclient.go b/pkg/apiclient/apiclient.go index edd3de24f1d1..fba035ffa38d 100644 --- a/pkg/apiclient/apiclient.go +++ b/pkg/apiclient/apiclient.go @@ -3,6 +3,8 @@ package apiclient import ( "context" "fmt" + "net/http" + "net/url" log "github.com/sirupsen/logrus" "k8s.io/client-go/tools/clientcmd" @@ -35,6 +37,7 @@ type Opts struct { Offline bool OfflineFiles []string Context context.Context + Proxy func(*http.Request) (*url.URL, error) } func (o Opts) String() string { @@ -73,7 +76,7 @@ func NewClientFromOpts(opts Opts) (context.Context, Client, error) { if opts.AuthSupplier == nil { return nil, nil, fmt.Errorf("AuthSupplier cannot be empty when connecting to Argo Server") } - return newHTTP1Client(opts.ArgoServerOpts.GetURL(), opts.AuthSupplier(), opts.ArgoServerOpts.InsecureSkipVerify, opts.ArgoServerOpts.Headers, opts.ArgoServerOpts.HTTP1Client) + return newHTTP1Client(opts.ArgoServerOpts.GetURL(), opts.AuthSupplier(), opts.ArgoServerOpts.InsecureSkipVerify, opts.ArgoServerOpts.Headers, opts.ArgoServerOpts.HTTP1Client, opts.Proxy) } else if opts.ArgoServerOpts.URL != "" { if opts.AuthSupplier == nil { return nil, nil, fmt.Errorf("AuthSupplier cannot be empty when connecting to Argo Server") diff --git a/pkg/apiclient/http1-client.go b/pkg/apiclient/http1-client.go index 518733bb4be7..a0504b61ccd5 100644 --- a/pkg/apiclient/http1-client.go +++ b/pkg/apiclient/http1-client.go @@ -3,7 +3,7 @@ package apiclient import ( "context" "net/http" - + "net/url" "github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate" cronworkflowpkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/cronworkflow" "github.com/argoproj/argo-workflows/v3/pkg/apiclient/http1" @@ -41,6 +41,6 @@ func (h httpClient) NewInfoServiceClient() (infopkg.InfoServiceClient, error) { return http1.InfoServiceClient(h), nil } -func newHTTP1Client(baseUrl string, auth string, insecureSkipVerify bool, headers []string, customHttpClient *http.Client) (context.Context, Client, error) { - return context.Background(), httpClient(http1.NewFacade(baseUrl, auth, insecureSkipVerify, headers, customHttpClient)), nil +func newHTTP1Client(baseUrl string, auth string, insecureSkipVerify bool, headers []string, customHttpClient *http.Client, proxy func(*http.Request) (*url.URL, error)) (context.Context, Client, error) { + return context.Background(), httpClient(http1.NewFacade(baseUrl, auth, insecureSkipVerify, headers, customHttpClient, proxy)), nil } diff --git a/pkg/apiclient/http1/facade.go b/pkg/apiclient/http1/facade.go index 91b6468d3074..3b74b98c062c 100644 --- a/pkg/apiclient/http1/facade.go +++ b/pkg/apiclient/http1/facade.go @@ -27,10 +27,11 @@ type Facade struct { insecureSkipVerify bool headers []string httpClient *http.Client + proxy func(*http.Request) (*url.URL, error) } -func NewFacade(baseUrl, authorization string, insecureSkipVerify bool, headers []string, httpClient *http.Client) Facade { - return Facade{baseUrl, authorization, insecureSkipVerify, headers, httpClient} +func NewFacade(baseUrl, authorization string, insecureSkipVerify bool, headers []string, httpClient *http.Client, proxy func(*http.Request) (*url.URL, error)) Facade { + return Facade{baseUrl, authorization, insecureSkipVerify, headers, httpClient, proxy} } func (h Facade) Get(ctx context.Context, in, out interface{}, path string) error { @@ -67,10 +68,15 @@ func (h Facade) EventStreamReader(ctx context.Context, in interface{}, path stri req.Header.Set("Accept", "text/event-stream") req.Header.Set("Authorization", h.authorization) log.Debugf("curl -H 'Accept: text/event-stream' -H 'Authorization: ******' '%v'", u) + proxyURL, err := h.proxyFunc()(req) + if err != nil { + return nil, err + } client := h.httpClient if h.httpClient == nil { client = &http.Client{ Transport: &http.Transport{ + Proxy: http.ProxyURL(proxyURL), TLSClientConfig: &tls.Config{ InsecureSkipVerify: h.insecureSkipVerify, }, @@ -113,10 +119,15 @@ func (h Facade) do(ctx context.Context, in interface{}, out interface{}, method req.Header = headers req.Header.Set("Authorization", h.authorization) log.Debugf("curl -X %s -H 'Authorization: ******' -d '%s' '%v'", method, string(data), u) + proxyURL, err := h.proxyFunc()(req) + if err != nil { + return err + } client := h.httpClient if h.httpClient == nil { client = &http.Client{ Transport: &http.Transport{ + Proxy: http.ProxyURL(proxyURL), TLSClientConfig: &tls.Config{ InsecureSkipVerify: h.insecureSkipVerify, }, From b1226a703cb9528543560a120035d8c407f11a04 Mon Sep 17 00:00:00 2001 From: shimako55 Date: Tue, 20 Feb 2024 20:27:12 +0900 Subject: [PATCH 2/3] Add tests for `Facade.proxyFunc` to ensure correct proxy function is returned based on configuration Signed-off-by: shimako55 --- pkg/apiclient/http1/facade.go | 7 ++++++ pkg/apiclient/http1/facade_test.go | 34 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/pkg/apiclient/http1/facade.go b/pkg/apiclient/http1/facade.go index 3b74b98c062c..9c92142fec95 100644 --- a/pkg/apiclient/http1/facade.go +++ b/pkg/apiclient/http1/facade.go @@ -191,3 +191,10 @@ func parseHeaders(headerStrings []string) (http.Header, error) { } return headers, nil } + +func (h *Facade) proxyFunc() func(*http.Request) (*url.URL, error) { + if h.proxy != nil { + return h.proxy + } + return http.ProxyFromEnvironment +} diff --git a/pkg/apiclient/http1/facade_test.go b/pkg/apiclient/http1/facade_test.go index 0340661e64fc..6dd238ce5233 100644 --- a/pkg/apiclient/http1/facade_test.go +++ b/pkg/apiclient/http1/facade_test.go @@ -1,6 +1,9 @@ package http1 import ( + "net/http" + "net/url" + "reflect" "testing" "github.com/stretchr/testify/assert" @@ -14,3 +17,34 @@ func TestFacade_do(t *testing.T) { require.NoError(t, err) assert.Equal(t, "http://my-url/my-ns/?labels.foo=1", u.String()) } + +func TestFacade_proxyFunc(t *testing.T) { + proxyFunc := func(_ *http.Request) (*url.URL, error) { + return nil, nil + } + tests := []struct { + name string + proxy func(*http.Request) (*url.URL, error) + want func(*http.Request) (*url.URL, error) + }{ + { + name: "use proxy settings from environment variables", + proxy: nil, + want: http.ProxyFromEnvironment, + }, + { + name: "use specific proxy", + proxy: proxyFunc, + want: proxyFunc, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := Facade{proxy: tt.want} + got := f.proxyFunc() + if reflect.ValueOf(got).Pointer() != reflect.ValueOf(tt.want).Pointer() { + t.Errorf("Facade.proxyURL() = %p, want %p", got, tt.want) + } + }) + } +} From cac0ae3a7969f06f98c4dc571e0a0d87964bf419 Mon Sep 17 00:00:00 2001 From: shimako55 Date: Tue, 29 Oct 2024 18:45:12 +0900 Subject: [PATCH 3/3] fix: Correct syntax Signed-off-by: shimako55 --- cmd/argo/commands/client/conn.go | 3 +-- pkg/apiclient/http1-client.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/argo/commands/client/conn.go b/cmd/argo/commands/client/conn.go index 68c8443b29c0..a677f109a3e3 100644 --- a/cmd/argo/commands/client/conn.go +++ b/cmd/argo/commands/client/conn.go @@ -81,8 +81,7 @@ func NewAPIClient(ctx context.Context) (context.Context, apiclient.Client, error OfflineFiles: OfflineFiles, Context: ctx, Proxy: proxy, - } - ) + }) } func Namespace() string { diff --git a/pkg/apiclient/http1-client.go b/pkg/apiclient/http1-client.go index a0504b61ccd5..6092f5e6f4d6 100644 --- a/pkg/apiclient/http1-client.go +++ b/pkg/apiclient/http1-client.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/url" + "github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate" cronworkflowpkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/cronworkflow" "github.com/argoproj/argo-workflows/v3/pkg/apiclient/http1"