From 264215ae01bf6df5cf559ec89b95eda8df67f929 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 12:13:05 +0100 Subject: [PATCH 1/8] resolve proxy to inject using http package --- .../inject_proxy_component_modifier.go | 36 +++++-------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier.go b/internal/pkg/agent/application/inject_proxy_component_modifier.go index 0ac945e77e8..b841112964e 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier.go @@ -5,8 +5,7 @@ package application import ( - "os" - "strings" + "net/http" "github.com/elastic/elastic-agent-client/v7/pkg/client" "github.com/elastic/elastic-agent/internal/pkg/agent/application/coordinator" @@ -59,9 +58,8 @@ func InjectProxyEndpointModifier() coordinator.ComponentsModifier { // injectProxyURL will inject the a proxy_url into the passed map if there is no existing key and there is an appropriate proxy through defined as an env var. // -// The 1st item of the passed hosts list is checked to see if it starts with https or http and the corresponding proxy var is used. -// Nothing is injected if the *_PROXY env var is empty, the map contains proxy_url: "", or the map has proxy_disable: true. -// If no hosts are passed, then the HTTPS_PROXY value is used over the HTTP_PROXY value if it's defined. +// Go http client is used to determine the proxy URL, to ensure consistent behavior across all components. +// Traffic through proxy is preferred if the proxy is defined for any of the hosts. func injectProxyURL(m map[string]interface{}, hosts []string) { if m == nil { return @@ -79,28 +77,12 @@ func injectProxyURL(m map[string]interface{}, hosts []string) { } } - var proxyURL string - matched := false - // If hosts are specified, check the 1st to see if HTTPS or HTTP is used to determine proxy - if len(hosts) > 0 { - if strings.HasPrefix(hosts[0], "https://") { - matched = true - proxyURL = os.Getenv("HTTPS_PROXY") - } else if strings.HasPrefix(hosts[0], "http://") { - matched = true - proxyURL = os.Getenv("HTTP_PROXY") - } - } - // if no hosts are specified, or it could not match a host prefix prefer HTTPS_PROXY over HTTP_PROXY - if proxyURL == "" && !matched { - proxyURL = os.Getenv("HTTPS_PROXY") - if proxyURL == "" { - proxyURL = os.Getenv("HTTP_PROXY") + // Check if a proxy is defined for the hosts + for _, host := range hosts { + proxyURL, _ := http.ProxyFromEnvironment(&http.Request{Host: host}) + if proxyURL.String() != "" { + m["proxy_url"] = proxyURL.String() + return } } - // No proxy defined - if proxyURL == "" { - return - } - m["proxy_url"] = proxyURL } From c38c7ed213e045806162eee6f79afb181a6589a7 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:03:59 +0100 Subject: [PATCH 2/8] unit tests --- .../inject_proxy_component_modifier.go | 11 +++++-- .../inject_proxy_component_modifier_test.go | 33 ++++++------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier.go b/internal/pkg/agent/application/inject_proxy_component_modifier.go index b841112964e..9ae2b624ebd 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier.go @@ -79,8 +79,15 @@ func injectProxyURL(m map[string]interface{}, hosts []string) { // Check if a proxy is defined for the hosts for _, host := range hosts { - proxyURL, _ := http.ProxyFromEnvironment(&http.Request{Host: host}) - if proxyURL.String() != "" { + request, err := http.NewRequest("GET", host, nil) + if err != nil { + continue + } + proxyURL, err := http.ProxyFromEnvironment(request) + if err != nil { + continue + } + if proxyURL != nil && proxyURL.String() != "" { m["proxy_url"] = proxyURL.String() return } diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go index a285673aa71..ce0ee7f60c8 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go @@ -320,6 +320,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { func Test_injectProxyURL(t *testing.T) { t.Setenv("HTTPS_PROXY", "https://localhost:8080") t.Setenv("HTTP_PROXY", "http://localhost:8081") + t.Setenv("NO_PROXY", "do.not.inject.proxy.for.me") tests := []struct { name string @@ -341,41 +342,27 @@ func Test_injectProxyURL(t *testing.T) { hosts: nil, expect: map[string]interface{}{"key": "value", "proxy_disable": true}, }, { - name: "no hosts uses HTTPS_PROXY", + name: "http hosts uses HTTP_PROXY", m: map[string]interface{}{"key": "value"}, - hosts: nil, - expect: map[string]interface{}{"key": "value", "proxy_url": "https://localhost:8080"}, + hosts: []string{"http://example:80"}, + expect: map[string]interface{}{"key": "value", "proxy_url": "http://localhost:8081"}, }, { name: "https hosts uses HTTPS_PROXY", m: map[string]interface{}{"key": "value"}, hosts: []string{"https://example:443"}, expect: map[string]interface{}{"key": "value", "proxy_url": "https://localhost:8080"}, - }, { - name: "http host uses HTTP_PROXY", + }, + { + name: "host skipped by NO_PROXY", m: map[string]interface{}{"key": "value"}, - hosts: []string{"http://example:80"}, - expect: map[string]interface{}{"key": "value", "proxy_url": "http://localhost:8081"}, + hosts: []string{"https://do.not.inject.proxy.for.me", "https://do.not.inject.proxy.for.me:8080", "really.do.not.inject.proxy.for.me"}, + expect: map[string]interface{}{"key": "value"}, }} + for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { injectProxyURL(tc.m, tc.hosts) require.Equal(t, tc.expect, tc.m) }) } - - t.Run("no hosts or HTTPS_PROXY uses HTTP_PROXY", func(t *testing.T) { - t.Setenv("HTTPS_PROXY", "") - t.Setenv("HTTP_PROXY", "http://localhost:8081") - - m := map[string]interface{}{"key": "value"} - injectProxyURL(m, nil) - require.Equal(t, map[string]interface{}{"key": "value", "proxy_url": "http://localhost:8081"}, m) - }) - t.Run("no env vars", func(t *testing.T) { - t.Setenv("HTTPS_PROXY", "") - t.Setenv("HTTP_PROXY", "") - m := map[string]interface{}{"key": "value"} - injectProxyURL(m, nil) - require.Equal(t, map[string]interface{}{"key": "value"}, m) - }) } From d3bedfc7a7819c351e3fab213cdc04b2ef0e1bf2 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:11:44 +0100 Subject: [PATCH 3/8] golint --- .../pkg/agent/application/inject_proxy_component_modifier.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier.go b/internal/pkg/agent/application/inject_proxy_component_modifier.go index 9ae2b624ebd..b2bde4f8d51 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier.go @@ -79,6 +79,7 @@ func injectProxyURL(m map[string]interface{}, hosts []string) { // Check if a proxy is defined for the hosts for _, host := range hosts { + // nolint:noctx // this request will not be executed request, err := http.NewRequest("GET", host, nil) if err != nil { continue From 5949aa0f5f801decf5a4ddc9926427d66bea898b Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:14:38 +0100 Subject: [PATCH 4/8] golint --- .../pkg/agent/application/inject_proxy_component_modifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier.go b/internal/pkg/agent/application/inject_proxy_component_modifier.go index b2bde4f8d51..d8eb9f1e989 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier.go @@ -79,7 +79,7 @@ func injectProxyURL(m map[string]interface{}, hosts []string) { // Check if a proxy is defined for the hosts for _, host := range hosts { - // nolint:noctx // this request will not be executed + //nolint:noctx // this request will not be executed request, err := http.NewRequest("GET", host, nil) if err != nil { continue From d0339f2c20b6ae2a8703bd9c8139a1f6a7fba387 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:27:30 +0100 Subject: [PATCH 5/8] changelog fragment --- ...29703-environment-proxy-injection-fix.yaml | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 changelog/fragments/1738329703-environment-proxy-injection-fix.yaml diff --git a/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml b/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml new file mode 100644 index 00000000000..bb866a538d2 --- /dev/null +++ b/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml @@ -0,0 +1,33 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: environment-proxy-injection-fix + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +description: +When injecting environment proxy into Elastic Defend config the Agent should use http library to resolve the proxy settings to ensure consistency across all components. + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/elastic/elastic-agent/pull/6675 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: https://github.com/elastic/elastic-agent/issues/6209 From fb8b21b3ae27b09ed6d705ebea1dc0dc1064688f Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:30:16 +0100 Subject: [PATCH 6/8] changelog fragment --- .../fragments/1738329703-environment-proxy-injection-fix.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml b/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml index bb866a538d2..4933176b4a7 100644 --- a/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml +++ b/changelog/fragments/1738329703-environment-proxy-injection-fix.yaml @@ -16,8 +16,7 @@ summary: environment-proxy-injection-fix # Long description; in case the summary is not enough to describe the change # this field accommodate a description without length limits. # NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. -description: -When injecting environment proxy into Elastic Defend config the Agent should use http library to resolve the proxy settings to ensure consistency across all components. +description: When injecting environment proxy into Elastic Defend config the Agent should use http library to resolve the proxy settings to ensure consistency across all components. # Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. component: elastic-agent From 96dc6cc04a14c489353d1e711317da3409535149 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:54:35 +0100 Subject: [PATCH 7/8] move environment manipulation, trying to fix test run in CI --- .../application/inject_proxy_component_modifier_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go index ce0ee7f60c8..2355e9c366b 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go @@ -318,10 +318,6 @@ func TestInjectProxyEndpointModifier(t *testing.T) { } func Test_injectProxyURL(t *testing.T) { - t.Setenv("HTTPS_PROXY", "https://localhost:8080") - t.Setenv("HTTP_PROXY", "http://localhost:8081") - t.Setenv("NO_PROXY", "do.not.inject.proxy.for.me") - tests := []struct { name string m map[string]interface{} @@ -361,6 +357,10 @@ func Test_injectProxyURL(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + t.Setenv("HTTPS_PROXY", "https://localhost:8080") + t.Setenv("HTTP_PROXY", "http://localhost:8081") + t.Setenv("NO_PROXY", "do.not.inject.proxy.for.me") + injectProxyURL(tc.m, tc.hosts) require.Equal(t, tc.expect, tc.m) }) From 3fb97d65e7370b01dc168912afd5baad6bb81f37 Mon Sep 17 00:00:00 2001 From: Leszek Kubik <39905449+intxgo@users.noreply.github.com> Date: Fri, 31 Jan 2025 19:55:25 +0100 Subject: [PATCH 8/8] fix unit test, localhost never goes through environment proxy --- .../inject_proxy_component_modifier_test.go | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go index 2355e9c366b..1f40b317b2b 100644 --- a/internal/pkg/agent/application/inject_proxy_component_modifier_test.go +++ b/internal/pkg/agent/application/inject_proxy_component_modifier_test.go @@ -43,7 +43,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"]}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"]}`)) require.NoError(t, err) return &source }(), @@ -64,7 +64,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"]}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"]}`)) require.NoError(t, err) return &source }(), @@ -88,7 +88,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_url":"https://proxy:8080"}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_url":"https://proxy:8080"}`)) require.NoError(t, err) return &source }(), @@ -109,7 +109,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_url":"https://proxy:8080"}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_url":"https://proxy:8080"}`)) require.NoError(t, err) return &source }(), @@ -133,7 +133,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_url":""}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_url":""}`)) require.NoError(t, err) return &source }(), @@ -154,7 +154,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_url":""}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_url":""}`)) require.NoError(t, err) return &source }(), @@ -178,7 +178,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_disable":true}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_disable":true}`)) require.NoError(t, err) return &source }(), @@ -199,7 +199,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_disable":true}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_disable":true}`)) require.NoError(t, err) return &source }(), @@ -223,7 +223,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"]}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"]}`)) require.NoError(t, err) return &source }(), @@ -244,7 +244,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://localhost:9200"],"proxy_url":"https://localhost:8080"}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["https://example.output:9200"],"proxy_url":"https://localhost:8080"}`)) require.NoError(t, err) return &source }(), @@ -268,7 +268,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["http://localhost:9200"]}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["http://example.output:9200"]}`)) require.NoError(t, err) return &source }(), @@ -289,7 +289,7 @@ func TestInjectProxyEndpointModifier(t *testing.T) { Type: elasticsearch, Source: func() *structpb.Struct { var source structpb.Struct - err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["http://localhost:9200"],"proxy_url":"http://localhost:8081"}`)) + err := source.UnmarshalJSON([]byte(`{"type":"elasticsearch","hosts":["http://example.output:9200"],"proxy_url":"http://localhost:8081"}`)) require.NoError(t, err) return &source }(), @@ -347,8 +347,7 @@ func Test_injectProxyURL(t *testing.T) { m: map[string]interface{}{"key": "value"}, hosts: []string{"https://example:443"}, expect: map[string]interface{}{"key": "value", "proxy_url": "https://localhost:8080"}, - }, - { + }, { name: "host skipped by NO_PROXY", m: map[string]interface{}{"key": "value"}, hosts: []string{"https://do.not.inject.proxy.for.me", "https://do.not.inject.proxy.for.me:8080", "really.do.not.inject.proxy.for.me"},