Add Proxy settings to AWS Common

[legoguy1000]

What does this PR do?

Adds the ability to set a proxy to the AWS SDK config.

Why is it important?

This is useful when the Beats outbound connections need to go through a proxy but don't want it to affect other inputs/outputs. When using the HTTP_PROXY env variable, this also affects the Beats output to Elasticsearch. Having a dedicated setting allows only the AWS requests to be proxied and not affect any other input/output.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

• [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-07-27T04:49:55.574+0000

• Duration: 133 min 56 sec

• Commit: a0a5bcb

Test stats 🧪

Test	Results
Failed	0
Passed	49346
Skipped	5308
Total	54654

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	49346
Skipped	5308
Total	54654

[P1llus]

@kaiyan-sheng any thoughts?

[kaiyan-sheng]

@legoguy1000 Thanks for the contribution! @P1llus This is definitely useful addition! I will assign this PR to myself and take care of the review and backports.

[legoguy1000]

@legoguy1000 Thanks for the contribution! @P1llus This is definitely useful addition! I will assign this PR to myself and take care of the review and backports.

Cool. At first I tried to use the httpcommon package to not duplicate work, but the TLS settings weren't needed and there was an incompatibility from it not having a Roundtrip method.

[elasticmachine]

Pinging @elastic/integrations (Team:Integrations)

[kaiyan-sheng]

/test

[kaiyan-sheng]

Do you think it's beneficial to merge function EnrichAWSConfigWithEndpoint and EnrichAWSConfigWithProxy together?

[legoguy1000]

Do you think it's beneficial to merge function EnrichAWSConfigWithEndpoint and EnrichAWSConfigWithProxy together?

I had thought about that originally and I wasn't sure if I was implementing this in the best way to begin with so I wanted to reduce the modifications of existing code and just add new. It shouldn't be an issue to combine the functions and just add another parameter to the function to pass the proxy config. I'll do whatever you guys think is best.

func EnrichAWSConfigWithEndpoint(endpoint string, serviceName string, regionName string, proxyUrl *url.URL, awsConfig awssdk.Config) awssdk.Config {
	if endpoint != "" {
		if regionName == "" {
			awsConfig.EndpointResolver = awssdk.ResolveWithEndpointURL("https://" + serviceName + "." + endpoint)
		} else {
			awsConfig.EndpointResolver = awssdk.ResolveWithEndpointURL("https://" + serviceName + "." + regionName + "." + endpoint)
		}
	}
	if proxyUrl != nil {
		httpClient := &http.Client{
			Transport: &http.Transport{
				Proxy: http.ProxyURL(proxyUrl),
			},
		}
		awsConfig.HTTPClient = httpClient
	}
	return awsConfig
}

[kaiyan-sheng]

/test

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-proxy upstream/aws-proxy
git merge upstream/master
git push upstream aws-proxy

[legoguy1000]

Do you think it's beneficial to merge function EnrichAWSConfigWithEndpoint and EnrichAWSConfigWithProxy together?

I had thought about that originally and I wasn't sure if I was implementing this in the best way to begin with so I wanted to reduce the modifications of existing code and just add new. It shouldn't be an issue to combine the functions and just add another parameter to the function to pass the proxy config. I'll do whatever you guys think is best.

func EnrichAWSConfigWithEndpoint(endpoint string, serviceName string, regionName string, proxyUrl *url.URL, awsConfig awssdk.Config) awssdk.Config {
	if endpoint != "" {
		if regionName == "" {
			awsConfig.EndpointResolver = awssdk.ResolveWithEndpointURL("https://" + serviceName + "." + endpoint)
		} else {
			awsConfig.EndpointResolver = awssdk.ResolveWithEndpointURL("https://" + serviceName + "." + regionName + "." + endpoint)
		}
	}
	if proxyUrl != nil {
		httpClient := &http.Client{
			Transport: &http.Transport{
				Proxy: http.ProxyURL(proxyUrl),
			},
		}
		awsConfig.HTTPClient = httpClient
	}
	return awsConfig
}

@kaiyan-sheng Did you want me to change the code to the above to combine it with the EnrichAWSConfigWithEndpoint function or leave it separate?

[kaiyan-sheng]

@legoguy1000 Let's merge two into one function :) Thank you!!

[legoguy1000]

@legoguy1000 Let's merge two into one function :) Thank you!!

@kaiyan-sheng So I started to consolidate the function with the EnrichAWSConfigWithEndpoint function and found that it is called in all sorts of places, including where the AWS config isn't being passed. Because it really only needs to be called once when the AWS SDK config object is setup i created a new function that encapsulates the GetAWSCredentials. Thoughts?

[kaiyan-sheng]

@legoguy1000 Agree, I wonder if we should combine EnrichAWSConfigWithEndpoint into InitializeAWSConfig at some point as well. It doesn't have to be in this same PR.

[kaiyan-sheng]

/test

[legoguy1000]

@legoguy1000 Agree, I wonder if we should combine EnrichAWSConfigWithEndpoint into InitializeAWSConfig at some point as well. It doesn't have to be in this same PR.

I don't know that i would because the way that function is being used, the InitializeAWSConfig only needs to be called once, the EnrichAWSConfigWithEndpoint is called multiple times with different services in different inputs/modules... So I think keeping it separate for now is best.

[legoguy1000]

@kaiyan-sheng can u take a look and rerun tests?

[kaiyan-sheng]

/test

[legoguy1000]

@kaiyan-sheng looks like everything passed. Did you have any other comments/changes you think should be made?

[kaiyan-sheng]

@legoguy1000 LGTM, thank you again for your contribution!!

[kaiyan-sheng]

7.x backport is done in #27077