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

skip service validation to get the default regions endpoint #733

Merged
merged 3 commits into from
Jul 11, 2024
Merged

skip service validation to get the default regions endpoint #733

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd50db2
skip service validation to get the default regions endpoint
kmala Jul 10, 2024
b1b2f45
Add unit test
kmala Jul 11, 2024
be03600
update the go version to 1.22.5
kmala Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions pkg/token/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,18 @@ type tokenVerifier struct {
validSTShostnames map[string]bool
}

func getDefaultHostNameForRegion(partition *endpoints.Partition, region string) (string, error) {
rep, err := partition.EndpointFor(stsServiceID, region, endpoints.STSRegionalEndpointOption, endpoints.ResolveUnknownServiceOption)
Copy link
Contributor

Choose a reason for hiding this comment

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

qs for familiarity: Looks like endpoints.ResolveUnknownServiceOption will return an error [here[(https://github.com/aws/aws-sdk-go/blob/fbe26bf118cf10a5c6aef4a105e2a5609c617b1a/aws/endpoints/v3model.go#L218-L221). What was the current behavior of partition.EndpointFor when endpoints.ResolveUnknownServiceOption was false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently it would throw service not found https://github.com/kubernetes-sigs/aws-iam-authenticator/blob/master/pkg/token/token.go#L412-L413 as the services are not yet updated in the sdk go

Copy link
Contributor

Choose a reason for hiding this comment

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

discussed offline, will add an unit test here

if err != nil {
return "", fmt.Errorf("Error resolving endpoint for %s in partition %s. err: %v", region, partition.ID(), err)
}
parsedURL, err := url.Parse(rep.URL)
if err != nil {
return "", fmt.Errorf("Error parsing STS URL %s. err: %v", rep.URL, err)
}
return parsedURL.Hostname(), nil
}

func stsHostsForPartition(partitionID, region string) map[string]bool {
validSTShostnames := map[string]bool{}

Expand All @@ -410,6 +422,14 @@ func stsHostsForPartition(partitionID, region string) map[string]bool {
stsSvc, ok := partition.Services()[stsServiceID]
if !ok {
logrus.Errorf("STS service not found in partition %s", partitionID)
// Add the host of the current instances region if the service doesn't already exists in the partition
// so we don't fail if the service is not present in the go sdk but matches the instances region.
stsHostName, err := getDefaultHostNameForRegion(partition, region)
if err != nil {
logrus.WithError(err).Error("Error getting default hostname")
} else {
validSTShostnames[stsHostName] = true
Copy link
Contributor

Choose a reason for hiding this comment

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

qs for familiarity: can you describe how this helps? when the sdk does not have the partition, the else block will not be taken. How does the rest of the flow succeed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the sdk doesn't have partition we fail as we are now aware of what a default dns hostname should be. This would help where there is partition but the services are not yet defined under the partition.

}
return validSTShostnames
}
stsSvcEndPoints := stsSvc.Endpoints()
Expand All @@ -430,17 +450,12 @@ func stsHostsForPartition(partitionID, region string) map[string]bool {
// Add the host of the current instances region if not already exists so we don't fail if the region is not
// present in the go sdk but matches the instances region.
if _, ok := stsSvcEndPoints[region]; !ok {
rep, err := partition.EndpointFor(stsServiceID, region, endpoints.STSRegionalEndpointOption)
stsHostName, err := getDefaultHostNameForRegion(partition, region)
if err != nil {
logrus.WithError(err).Errorf("Error resolving endpoint for %s in partition %s", region, partitionID)
logrus.WithError(err).Error("Error getting default hostname")
return validSTShostnames
}
parsedURL, err := url.Parse(rep.URL)
if err != nil {
logrus.WithError(err).Errorf("Error parsing STS URL %s", rep.URL)
return validSTShostnames
}
validSTShostnames[parsedURL.Hostname()] = true
validSTShostnames[stsHostName] = true
}

return validSTShostnames
Expand Down