-
Notifications
You must be signed in to change notification settings - Fork 423
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
Abstract region discovery #757
base: master
Are you sure you want to change the base?
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 |
---|---|---|
|
@@ -189,6 +189,20 @@ type Config struct { | |
ReservedPrefixConfig map[string]ReservedPrefixConfig | ||
// Dynamic File Path for BackendMode | ||
DynamicBackendModePath string | ||
|
||
// EndpointDiscovererMode is a method for determining how to validate STS domain names in presigned URLs. | ||
// Must be one of "Legacy" (default until June 2025), "API", or "File". | ||
// | ||
// Legacy uses the AWS SDK Go v1 endpoint list to validate that the STS region in an incoming token is valid | ||
// for a given partition. | ||
// | ||
// API makes the server invoke the AWS API call `account:ListRegions` to find a list of valid regions. | ||
// | ||
// File has the server read a file containing a JSON list of strings with valid STS endpoints. | ||
EndpointValidationMode string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Would it be worthwhile to define this using an enum/const generator, instead of string? eg:
It would save us from requiring defensive checks in the switch/case statements, but I also understand if the overhead of dealing with enums in Go isn't worth the hassle. |
||
|
||
// EndpointValidationFile the file path when EndpointValidationMode's value is "File" | ||
EndpointValidationFile string | ||
} | ||
|
||
type ReservedPrefixConfig struct { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package regions | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/aws/aws-sdk-go-v2/aws" | ||
"github.com/aws/aws-sdk-go-v2/service/account" | ||
) | ||
|
||
// NewAPIDiscoverer returns a new Discoverer for the given partition. Hostname | ||
// discovery uses the `account:ListRegions` API call to identifiy | ||
// | ||
// The partition is used to determine the correct hostnames for the STS endpoints | ||
// The supported partitions are "aws", "aws-us-gov", and "aws-cn". Future partitions | ||
// will need to be added manually, or use a different Discoverer | ||
func NewAPIDiscoverer(c account.ListRegionsAPIClient, partition string) Discoverer { | ||
return &apiDiscoverer{ | ||
client: c, | ||
partition: partition, | ||
} | ||
} | ||
|
||
type apiDiscoverer struct { | ||
client account.ListRegionsAPIClient | ||
partition string | ||
} | ||
|
||
func (d *apiDiscoverer) Find(ctx context.Context) (map[string]bool, error) { | ||
stsHostnames := map[string]bool{} | ||
|
||
var tlds []string | ||
serviceNames := []string{"sts", "sts-fips"} | ||
switch d.partition { | ||
case "aws": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to my enums question above: is it worthwhile to use enums here? |
||
tlds = []string{"amazonaws.com", "api.aws"} | ||
stsHostnames["sts.amazonaws.com"] = true | ||
case "aws-us-gov": | ||
tlds = []string{"amazonaws.com", "api.aws"} | ||
case "aws-cn": | ||
serviceNames = []string{"sts"} | ||
tlds = []string{"amazonaws.com.cn"} | ||
case "aws-iso": | ||
tlds = []string{"c2s.ic.gov"} | ||
case "aws-iso-b": | ||
tlds = []string{"sc2s.sgov.gov"} | ||
case "aws-iso-e": | ||
tlds = []string{"cloud.adc-e.uk"} | ||
case "aws-iso-f": | ||
tlds = []string{"csp.hci.ic.gov"} | ||
default: | ||
return nil, fmt.Errorf("unrecognized partition %s", d.partition) | ||
} | ||
|
||
result, err := d.client.ListRegions(ctx, nil) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get regions: %w", err) | ||
} | ||
|
||
for _, region := range result.Regions { | ||
// TODO: We could assess if a region is opted in, but we may need to | ||
// increase the frequency of Verifier's ticker or make it configurable | ||
// so clients don't have to wait up to 24h to trust tokens from a | ||
// recently opted-in region. | ||
for _, serviceName := range serviceNames { | ||
for _, tld := range tlds { | ||
hostname := fmt.Sprintf("%s.%s.%s", serviceName, aws.ToString(region.RegionName), tld) | ||
stsHostnames[hostname] = true | ||
} | ||
} | ||
} | ||
|
||
return stsHostnames, nil | ||
} |
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.
nit: should we return an error instead of calling
Fatal
? Seems likegetConfig
is expected to propagate errors