Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LoadDefaultConfig not programmatically setting region, need AWS Region set as environment variable #2260

Closed
colinbut opened this issue Aug 30, 2023 · 10 comments
Assignees
Labels
bug This issue is a bug. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@colinbut
Copy link

Describe the bug

Trying to set the aws region programmatically by passing in to config.LoadDefaultConfig using WithRegion as demonstrated by here:

https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/#specify-region-programmatically

However, get the following error saying aws region is not found.

operation error CloudWatch Logs: DescribeLogGroups, failed to resolve service endpoint, an AWS region is required, but was not found

Expected Behavior

cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion(". ")) to be working without needing to set AWS Region env variable.

Current Behavior

need to set env variable AWS Region

Reproduction Steps

check out - https://github.com/influxdata/telegraf/blob/master/plugins/common/aws/credentials.go#L27-L37

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

v1.21.0

Compiler and Version used

1.21

Operating System and version

Linux RedHat Distros

@colinbut colinbut added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 30, 2023
@colinbut colinbut changed the title not programmatically setting region in LoadDefaultConfig, need AWS Region set as environment variable LoadDefaultConfig not programmatically setting region, need AWS Region set as environment variable Aug 30, 2023
@colinbut
Copy link
Author

We found this issue in the telegraf project - https://github.com/influxdata/telegraf

@powersj - please correct any mistake info or add any additional information that i might be missing, afterall, I'm not the maintainer of telegraf project so don't know the code as much as you do. 💡

@RanVaknin
Copy link
Contributor

HI @colinbut ,

I'm a bit confused as the region being passed here is invalid:

- cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion(".   "))
+ cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1")) // or any other valid region name

Thanks,
Ran~

@RanVaknin RanVaknin self-assigned this Sep 1, 2023
@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Sep 1, 2023
@colinbut
Copy link
Author

colinbut commented Sep 1, 2023

@RanVaknin apologies that's just a typo on my end.

but yes, my point is something like config.WithRegion("us-east-1")) doesn't work...

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 2, 2023
@RanVaknin
Copy link
Contributor

RanVaknin commented Sep 2, 2023

Hi @colinbut ,

Im not sure what you mean by "doesn't work". Can you please add an entire code snippet?

LoadDefaultConfig with region works as expected:

package main

import (
	"context"
	"fmt"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs"
	"log"
)

func main() {
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithClientLogMode(aws.LogRequestWithBody))
	if err != nil {
		log.Fatalf("unable to load SDK config, %v", err)
	}

	client := cloudwatchlogs.NewFromConfig(cfg)

	out, err := client.DescribeLogGroups(context.TODO(), &cloudwatchlogs.DescribeLogGroupsInput{})
	if err != nil {
		panic(err)
	}

	fmt.Printf("There are %d log groups.", len(out.LogGroups))
}

Output:

SDK 2023/09/01 18:49:00 DEBUG Request
POST / HTTP/1.1
Host: logs.us-east-1.amazonaws.com
User-Agent: aws-sdk-go-v2/1.21.0 os/macos lang/go#1.19.1 md/GOOS#darwin md/GOARCH#arm64 api/cloudwatchlogs#1.23.5
Content-Length: 2
Amz-Sdk-Invocation-Id: REDACTED
Amz-Sdk-Request: attempt=1; max=3
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20230902/us-east-1/logs/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date;x-amz-target, Signature=REDACTED
Content-Type: application/x-amz-json-1.1
X-Amz-Date: 20230902T014900Z
X-Amz-Target: Logs_20140328.DescribeLogGroups
Accept-Encoding: gzip

{}
There are 49 log groups.

This request is routed to the correct service endpoint as seen in Host: logs.us-east-1.amazonaws.com and the valid results.
Im not familiar with Telegraf, but I suggest you open this bug report with their repo.

Thanks,
Ran~

@RanVaknin RanVaknin added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 2, 2023
@colinbut
Copy link
Author

colinbut commented Sep 2, 2023

Hi @RanVaknin - thanks for your response, i'm not too sure actually.

There's an ongoing issue against the telegraf project which I've also referenced here as well and you can see the initial problem i faced in the telegraf project - influxdata/telegraf#11963

The telegraf project maintainer @powersj asked me to raise an issue here as he believes there's issue with the way the region is set programmatically hence feels the problem lies in the aws sdk go itself rather than the telegraf project.

@RanVaknin - probably best if you liaise with @powersj to take this further.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 3, 2023
@powersj
Copy link

powersj commented Sep 5, 2023

Hi @RanVaknin

Telegraf maintainer here. I asked @colinbut to file this upstream issue after we confirmed that the region was in fact passed correctly in telegraf. Yet passing the region as an environmental variable seems to resolve the issue. As such I am asking for guidance on why that might be case. Why is the behavior different?

Thanks!

@RanVaknin
Copy link
Contributor

Hi @powersj,

The upstream issue only appears to confirm that the region ends up unset within the context of Telegraf code configuring an SDK client. It doesn't necessarily prove that the SDK itself is failing to apply config.WithRegion.

Setting the region programmatically via config.WithRegion and subsequently passing the result to NewFromConfig works correctly as demonstrated above.

Is there potentially a disconnect between the configuration being loaded and it being passed in during creation of an SDK client?

Thanks,
Ran~

@RanVaknin RanVaknin added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 5, 2023
@powersj
Copy link

powersj commented Sep 5, 2023

The upstream issue only appears to confirm that the region ends up unset within the context of Telegraf code configuring an SDK client.

I do not disagree that there may be a disconnect, but it is not clear to me what to even look at :) It is not clear why this works when setting AWS_REGION, but not when set in the config.

The debug output that we had @colinbut collect prints out what the SDK client thinks the region is set to before any actions and after loading the configuration. It reports the expected region and then the user immediately gets the error:

env AWS_REGION is ""
setting region to: eu-west-1
loading with root credentials
loaded config is using region: eu-west-1
2023-08-26T09:13:22Z E! [telegraf] Error running agent: connecting output outputs.cloudwatch_logs: error connecting to output "outputs.cloudwatch_logs": operation error CloudWatch Logs: DescribeLogGroups, failed to resolve service endpoint, an AWS region is required, but was not found

@RanVaknin
Copy link
Contributor

Hi @powersj,

It is not clear why this works when setting AWS_REGION, but not when set in the config.

If I had to guess this is probably an oversight with how this got implemented in the Telegraf credentials utility. The extent of our support is for the SDK itself, and not the SDK vended through 3rd party applications like Telegraf. The SDK team is not familiar with your codebase or your product to make sensible recommendations on how to fix this.

env AWS_REGION is ""
setting region to: eu-west-1
loading with root credentials
loaded config is using region: eu-west-1
2023-08-26T09:13:22Z E! [telegraf] Error running agent: connecting output outputs.cloudwatch_logs: error connecting to output "outputs.cloudwatch_logs": operation error CloudWatch Logs: DescribeLogGroups, failed to resolve service endpoint, an AWS region is required, but was not found

The SDK does not have this logging utility, so the logs provided here probably represent how Telegraf sets the region, rather then how the region is set in the SDK. This doesn't rule out the possibility that somewhere later in the code Telegraf sets the region incorrectly or at all for the SDK.

My suggestion would be to put a breakpoint in your codebase where the region is supplied explicitly by the user, and then slowly but surely move through each line of code to see where this data is overwritten / removed.

Since we have confirmed that this is not an SDK issue, and is not actionable by the SDK team I'm inclined to close this.
This needs to be investigated by the Telegraf team.

Thanks,
Ran~

@RanVaknin RanVaknin closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2023
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

3 participants