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.

Sign up for GitHub

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

Jump to bottom

Use AWS_DEFAULT_REGION not AWS_REGION #2103

Closed
delitescere opened this issue Aug 15, 2018 · 5 comments
Closed

Use AWS_DEFAULT_REGION not AWS_REGION #2103

delitescere opened this issue Aug 15, 2018 · 5 comments

Comments

@delitescere
Copy link

aws-sdk-go/aws/defaults/defaults.go

Line 57 in f2f8437

WithRegion(os.Getenv("AWS_REGION")).

According to https://docs.aws.amazon.com/cli/latest/userguide/cli-environment.html there is now a "standard" (unfortunately many API clients do indeed use AWS_REGION).

Suggest only using AWS_REGION if AWS_DEFAULT_REGION is not blank.
@diehlaws diehlaws self-assigned this Aug 15, 2018
@diehlaws diehlaws added the feature-request A feature should be added or improved. label Aug 15, 2018
@xibz xibz removed the feature-request A feature should be added or improved. label Aug 16, 2018
@xibz
Copy link
Contributor

xibz commented Aug 16, 2018

Hello @delitescere, thank you for reaching out to us. This would be a breaking change for the SDK and something we would not be able to support without introducing another environment variable. This is just an inconsistency of how SDKs handle environment variable. In addition, AWS_DEFAULT_REGION was introduced later, which is why the behavior is the way that it is. Since this is a breaking change, I will go ahead and close this. I suggest using AWS_REGION for both the CLI and the SDK. If you have any other issues, please let us know!

@xibz xibz closed this as completed Aug 16, 2018
@delitescere
Copy link
Author

Thanks @xibz! Yes I definitely know the annoyance that the names of these configuration variables causes - I have several variations of the same values depending on what the client expects.

Although, I don't know that my suggestion makes it a breaking change. Falling back to AWS_REGION if AWS_DEFAULT_REGION is blank seems like nice backwards compatibility and pushing towards eventual uniformity?

@jasdel
Copy link
Contributor

jasdel commented Aug 22, 2018

@delitescere thanks for the feedback. The reason the change in behavior is the SDK looks at AWS_REGION first and only AWS_DEFAULT_REGION next if the SDK's support for the shared config is enabled via the session or AWS_SDK_LOAD_CONFIG=1 env is set.

The SDK requires an option support for the shared config files since its feature were added after the SDK initial released. Automatically supporting the shared config would of potentially change the behavior of user applications that were running in the same environment as the AWS CLI.

@aws aws deleted a comment Sep 10, 2018
@pwaller
Copy link
Contributor

pwaller commented Dec 11, 2018 •
edited

By the way, it seems the CLI has dropped support for AWS_REGION now, which lead to some confusion since I was expecting both to work. After a quick search I actually can't find anything mentioning this (changelogs, git commits) in either boto3 or the awscli repository. I just observe that AWS_REGION isn't mentioned in the aws-cli documentation and doesn't work anymore.

FWIW I would like to see these tools behave as consistently as possible, though I understand the pressure to remain backwards compatible. Perhaps pressure should be applied to awscli to re-support AWS_REGION?

xoen added a commit to ministryofjustice/analytics-platform-config that referenced this issue Dec 18, 2018
@xoen
kube2iam: Updated config for official helm chart
94b56d1 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
@xoen xoen mentioned this issue Dec 18, 2018
Merged
@akutz akutz mentioned this issue Feb 22, 2019
Merged
@harishkolanu235
Copy link

hello can anyone send get the all instances from all regions using go.

@trivikr trivikr mentioned this issue Nov 4, 2019
Closed
3 tasks
@fogfish fogfish mentioned this issue Dec 24, 2019
Closed
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 6, 2020
@xoen
kube2iam: Updated config for official helm chart
4b45da1 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 6, 2020
@xoen
kube2iam: Updated config for official helm chart
e59b4c0 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 8, 2020
@xoen
kube2iam: Updated config for official helm chart
06ec602 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 8, 2020
@xoen
kube2iam: Updated config for official helm chart
638d8d1 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 8, 2020
@xoen
kube2iam: Updated config for official helm chart
a49c9ba 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
dhirajnarwanimoj pushed a commit to ministryofjustice/analytics-platform-config that referenced this issue Apr 9, 2020
@xoen
kube2iam: Updated config for official helm chart
14a7990 
We used to have our own kube2iam helm chart because there wasn't an helm chart before.
There is now a stable official helm chart [1] so no reason to keep ours.

The structure of the values changed to comply with what the helm chart expects.

NOTE: I'm also overriding the image tag as the helm chart uses a slightly older version
by default (`0.10.0` vs the newer `0.10.4`)

NOTE: I'm setting the `AWS_DEFAULT_REGION` environment variable even if the helm chart
already sets `AWS_REGION` [2].
Documentation for `--use-regional-sts-endpoint` [3] talks about the `AWS_REGION`
environment variable and it seems there is some confusion [4] at the moment
around which environment variable takes precedence and/or is used by the AWS SDK.

[1]: https://github.com/helm/charts/tree/master/stable/kube2iam
[2]: https://github.com/helm/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L67
[3]: https://github.com/jtblin/kube2iam/blob/master/README.md#aws-sts-endpoint-and-regions
[4]: aws/aws-sdk-go#2103
@UpGado UpGado mentioned this issue Jul 24, 2020
Merged
@jqmichael jqmichael mentioned this issue Jul 25, 2020
Closed
@james-jory james-jory mentioned this issue Aug 20, 2020
Merged
@diehlaws diehlaws removed their assignment Aug 26, 2020
@jared-gs jared-gs mentioned this issue Apr 29, 2022
Merged
@bwagner5 bwagner5 mentioned this issue Jul 22, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@delitescere @pwaller @jasdel @xibz @harishkolanu235 @diehlaws