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

Check for empty region before invoking API in AWS SDK #7523

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

a2ush
Copy link
Contributor

@a2ush a2ush commented Feb 2, 2024

Description

Currently, you will receive the following error if you forget to set the AWS region code.

$ eksctl create cluster
Error: checking AWS STS access – cannot get role ARN for current session: operation error STS: GetCallerIdentity, https response error StatusCode: 0, RequestID: , request send failed, Post "https://sts..amazonaws.com/": Bad Request

When you look closely, you can notice that there is something wrong with this endpoint "https://sts..amazonaws.com/".
However, it is difficult to immediately resolve the problem from this error message.

This PR provides a more understandable error message if you run into the same situation.
I think it helps to navigate the next action to solve the problem.

(demo)
$ ./eksctl create cluster
Error: no region code: please set the AWS region to ~/.aws/config file or environment variables.

note:
The Installation document already has "prerequisite" description about aws configs.
https://eksctl.io/installation/#prerequisite

You can use ~/.aws/credentials file or environment variables.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

@@ -187,6 +187,9 @@ func newAWSProvider(spec *api.ProviderConfig, configurationLoader AWSConfigurati
}

if spec.Region == "" {
if cfg.Region == "" {
return nil, fmt.Errorf("no region code: please set the AWS region to ~/.aws/config file or environment variables")
}
spec.Region = cfg.Region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eksctl/pkg/eks/apiv2.go

Lines 48 to 50 in 4e3d0e4

if pc.Region != "" {
options = append(options, config.WithRegion(pc.Region))
}

Looks like cfg.Region is set from spec.Region in newV2Config() on L184.

We should just move this check on spec.Region to before L184 and error if it's empty then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your suggestion.

I understood the current logic as the following;

  • if Spec.Region is not empty, cfg.Region is set from spec.Region in newV2Config()
  • if Spec.Region is empty, cfg.Region is set from ~/.aws/config or AWS_REGION in newV2Config()

So, I think we should check the value of cfg.Region after L184.
If cfg.Region is empty, we can determine AWS region is not defined.

//note

	cfg, err := newV2Config(spec, credentialsCacheFilePath, configurationLoader)
	if err != nil {
		return nil, err
	}

	fmt.Printf("spec.Region : %v\n", spec.Region)
	fmt.Printf("cfg.Region : %v\n", cfg.Region)

	if cfg.Region == "" {
		return nil, fmt.Errorf("AWS Region must be set, please set the AWS Region in AWS config file or as AWS_REGION environment variable")
	}

[result]

$ ./eksctl create cluster --dry-run
spec.Region : 
cfg.Region : ap-northeast-1

$ rm ~/.aws/config
$ ./eksctl create cluster --dry-run 
spec.Region : 
cfg.Region : 
Error: AWS Region must be set, please set the AWS Region in AWS config file or as AWS_REGION environment variable

$ export AWS_REGION=us-east-1      
$ ./eksctl create cluster --dry-run
spec.Region : 
cfg.Region : us-east-1

$ ./eksctl create cluster -f sample.yaml --dry-run
spec.Region : ap-northeast-1
cfg.Region : ap-northeast-1

pkg/eks/api.go Outdated Show resolved Hide resolved
@yuxiang-zhang yuxiang-zhang changed the title Changed the error to more understandable when the region code isn't set Check for empty region before invoking API in AWS SDK Feb 12, 2024
@yuxiang-zhang yuxiang-zhang merged commit fade871 into eksctl-io:main Feb 12, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants