-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add regions names #171
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
base: main
Are you sure you want to change the base?
Add regions names #171
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,17 +5,92 @@ import ( | |||||||||||||||||||||
| "errors" | ||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||
| "sort" | ||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||
| "time" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| "github.com/aws/aws-sdk-go-v2/config" | ||||||||||||||||||||||
| "github.com/aws/aws-sdk-go-v2/service/ec2" | ||||||||||||||||||||||
| "github.com/aws/aws-sdk-go-v2/service/ssm" | ||||||||||||||||||||||
| "github.com/charmbracelet/huh" | ||||||||||||||||||||||
| tslocal "tailscale.com/client/local" | ||||||||||||||||||||||
| tsapi "tailscale.com/client/tailscale/v2" | ||||||||||||||||||||||
| "tailscale.com/ipn" | ||||||||||||||||||||||
| "tailscale.com/tailcfg" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| func getBroadRegion(region string) string { | ||||||||||||||||||||||
| switch { | ||||||||||||||||||||||
| case strings.HasPrefix(region, "us-gov-"): | ||||||||||||||||||||||
| return "US GovCloud" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "us-"): | ||||||||||||||||||||||
| return "United States" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "eu-"): | ||||||||||||||||||||||
| return "Europe" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "ap-"): | ||||||||||||||||||||||
| return "Asia Pacific" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "sa-"): | ||||||||||||||||||||||
| return "South America" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "ca-"): | ||||||||||||||||||||||
| return "Canada" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "af-"): | ||||||||||||||||||||||
| return "Africa" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "me-"): | ||||||||||||||||||||||
| return "Middle East" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "cn-"): | ||||||||||||||||||||||
| return "China" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "il-"): | ||||||||||||||||||||||
| return "Israel" | ||||||||||||||||||||||
| case strings.HasPrefix(region, "mx-"): | ||||||||||||||||||||||
| return "Mexico" | ||||||||||||||||||||||
| default: | ||||||||||||||||||||||
| return "Other" | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // getRegionDisplayNames fetches human-readable names for the given region codes | ||||||||||||||||||||||
| // from the AWS SSM Parameter Store global infrastructure parameters. | ||||||||||||||||||||||
| func getRegionDisplayNames(ctx context.Context, regionCodes []string) (map[string]string, error) { | ||||||||||||||||||||||
| cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-east-1")) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return nil, fmt.Errorf("failed to load config: %w", err) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| ssmSvc := ssm.NewFromConfig(cfg) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| names := make(map[string]string, len(regionCodes)) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // SSM GetParameters accepts at most 10 names per call. | ||||||||||||||||||||||
| for i := 0; i < len(regionCodes); i += 10 { | ||||||||||||||||||||||
| end := i + 10 | ||||||||||||||||||||||
| if end > len(regionCodes) { | ||||||||||||||||||||||
| end = len(regionCodes) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| batch := regionCodes[i:end] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| paths := make([]string, len(batch)) | ||||||||||||||||||||||
| for j, code := range batch { | ||||||||||||||||||||||
| paths[j] = "/aws/service/global-infrastructure/regions/" + code + "/longName" | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| output, err := ssmSvc.GetParameters(ctx, &ssm.GetParametersInput{Names: paths}) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return nil, fmt.Errorf("failed to get region names from SSM: %w", err) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for _, param := range output.Parameters { | ||||||||||||||||||||||
| // Path: /aws/service/global-infrastructure/regions/{code}/longName | ||||||||||||||||||||||
| if param.Name == nil || param.Value == nil { | ||||||||||||||||||||||
| continue | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| parts := strings.Split(*param.Name, "/") | ||||||||||||||||||||||
| if len(parts) >= 7 { | ||||||||||||||||||||||
| names[parts[5]] = *param.Value | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+79
to
+87
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return names, nil | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| func GetRegions(ctx context.Context) ([]string, error) { | ||||||||||||||||||||||
| cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-east-1")) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
|
|
@@ -40,28 +115,81 @@ func GetRegions(ctx context.Context) ([]string, error) { | |||||||||||||||||||||
| return regionNames, nil | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Function that uses huh to return an AWS region fetched from the aws sdk. | ||||||||||||||||||||||
| // SelectRegion uses a two-step huh form to first pick a broad geographic area | ||||||||||||||||||||||
| // and then a specific region (shown with its human-readable name). | ||||||||||||||||||||||
| // Both steps are in a single form so the user can navigate back to correct a | ||||||||||||||||||||||
| // wrong broad-area selection. | ||||||||||||||||||||||
| func SelectRegion(ctx context.Context) (string, error) { | ||||||||||||||||||||||
| regionNames, err := GetRegions(ctx) | ||||||||||||||||||||||
| regionCodes, err := GetRegions(ctx) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| if ctx.Err() != nil { | ||||||||||||||||||||||
| return "", fmt.Errorf("operation canceled: %w", ctx.Err()) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return "", fmt.Errorf("failed to retrieve regions: %w", err) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| var selectedRegion string | ||||||||||||||||||||||
| // Group region codes by broad geographic area. | ||||||||||||||||||||||
| broadMap := map[string][]string{} | ||||||||||||||||||||||
| for _, code := range regionCodes { | ||||||||||||||||||||||
| broad := getBroadRegion(code) | ||||||||||||||||||||||
| broadMap[broad] = append(broadMap[broad], code) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| broadRegions := make([]string, 0, len(broadMap)) | ||||||||||||||||||||||
| for broad := range broadMap { | ||||||||||||||||||||||
| broadRegions = append(broadRegions, broad) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| sort.Strings(broadRegions) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Fetch all display names upfront so OptionsFunc stays synchronous. | ||||||||||||||||||||||
| displayNames, err := getRegionDisplayNames(ctx, regionCodes) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| if ctx.Err() != nil { | ||||||||||||||||||||||
| return "", fmt.Errorf("region selection canceled: %w", ctx.Err()) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return "", fmt.Errorf("failed to get region names: %w", err) | ||||||||||||||||||||||
|
Comment on lines
+147
to
+150
|
||||||||||||||||||||||
| if ctx.Err() != nil { | |
| return "", fmt.Errorf("region selection canceled: %w", ctx.Err()) | |
| } | |
| return "", fmt.Errorf("failed to get region names: %w", err) | |
| // If the context was canceled, respect that and abort. | |
| if ctx.Err() != nil { | |
| return "", fmt.Errorf("region selection canceled: %w", ctx.Err()) | |
| } | |
| // Otherwise, treat display-name lookup as best-effort: fall back to codes only. | |
| displayNames = map[string]string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the file and examine the broader context
find . -name "common.go" -type fRepository: lucacome/tailout
Length of output: 80
🏁 Script executed:
# Search for getRegionDisplayNames function definition
rg "func.*getRegionDisplayNames" --context 5Repository: lucacome/tailout
Length of output: 712
🏁 Script executed:
# Look at how displayNames is used after the error handling
rg -A 20 "getRegionDisplayNames" internal/common.goRepository: lucacome/tailout
Length of output: 1586
🏁 Script executed:
# Get more context on how displayNames is used after the error handling
rg -A 50 "displayNames, err := getRegionDisplayNames" internal/common.go | head -80Repository: lucacome/tailout
Length of output: 1691
🏁 Script executed:
# Search for where displayNames is referenced in the region selection logic
rg "displayNames" internal/common.go -B 2 -A 2Repository: lucacome/tailout
Length of output: 401
Don't make region selection fail when SSM display-name lookup fails.
Line 148 currently turns display-name enrichment into a hard dependency. If SSM access is denied/unavailable, users can't select any region even though region codes are available. The downstream OptionsFunc already handles missing display names by falling back to raw region codes, so the SSM failure should not be fatal.
Suggested fix
displayNames, err := getRegionDisplayNames(ctx, regionCodes)
if err != nil {
if ctx.Err() != nil {
return "", fmt.Errorf("region selection canceled: %w", ctx.Err())
}
- return "", fmt.Errorf("failed to get region names: %w", err)
+ // Keep selection functional with raw codes when display-name enrichment fails.
+ displayNames = map[string]string{}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/common.go` around lines 142 - 148, The call to getRegionDisplayNames
makes region selection fail when SSM lookup errors; change the error handling in
internal/common.go so that if getRegionDisplayNames returns an error you only
return if ctx.Err() != nil (preserve cancellation), otherwise log or swallow the
error and continue (do not return the error) so displayNames can be nil/empty
and the downstream OptionsFunc can fall back to raw region codes; update the
block around getRegionDisplayNames(ctx, regionCodes) accordingly and ensure
logging includes the error for observability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SelectRegionloads AWS config twice (once inGetRegions, again ingetRegionDisplayNames). If this runs on every interactive create, it adds extra latency and repeated config/credential resolution. Consider reusing the loadedcfg(or passing anssm.Client/aws.Configinto helpers) to avoid duplicate loads.