Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

Improve region endpoint resolver #432

Merged
merged 9 commits into from
Oct 23, 2020
Merged

Improve region endpoint resolver #432

merged 9 commits into from
Oct 23, 2020

Conversation

pingles
Copy link
Contributor

@pingles pingles commented Oct 19, 2020

Pull out into separate type and add some more tests about handling of gov, fips and chinese regions.

Additionally removes comparison against SDK regions and instead relies on DNS resolution to verify.

This touches on, but doesn't necessarily fix #410 #368 #344 yet.

Pull out into separate type and add some more tests about handling of gov, fips and chinese regions.

Additionally removes comparison against SDK regions and instead relies on DNS resolution to verify.
@pingles
Copy link
Contributor Author

pingles commented Oct 19, 2020

@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #432 into master will increase coverage by 7.31%.
The diff coverage is 50.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   46.40%   53.72%   +7.31%     
==========================================
  Files          26       30       +4     
  Lines        1058     1115      +57     
==========================================
+ Hits          491      599     +108     
+ Misses        516      462      -54     
- Partials       51       54       +3     
Impacted Files Coverage Δ
pkg/aws/sts/gateway.go 0.00% <0.00%> (-51.03%) ⬇️
pkg/aws/sts/resolver_detect_arn.go 22.22% <0.00%> (ø)
pkg/server/gateway.go 9.37% <ø> (+9.37%) ⬆️
pkg/server/server.go 76.92% <33.33%> (+55.18%) ⬆️
pkg/server/server_builder.go 36.63% <36.63%> (ø)
pkg/aws/sts/cache.go 57.81% <50.00%> (-4.16%) ⬇️
pkg/server/gateway_builder.go 56.92% <56.92%> (ø)
pkg/aws/sts/kiam_configuration_builder.go 71.42% <71.42%> (ø)
pkg/aws/sts/aws_endpoint_resolver.go 82.60% <82.60%> (ø)
pkg/aws/sts/metrics.go 100.00% <100.00%> (ø)
... and 8 more

* Removed most of the region config setup from sts.DefaultGateway into a configBuilder, added more tests around configBuilder to confirm behaviour
* Changed server to request server credentials with the server assume role after configuring for region, should address #368
* Regional endpoint adds a us-iso prefix to handle airgapped regions addressing #410
* Updated version of AWS SDK to 1.35
Make helper funcs hidden
Rename opts to cmd to be consistent
Add some tests for GetPodRole
@pingles pingles marked this pull request as ready for review October 21, 2020 13:21
* Created a Kiam Server integration test to make it easier to test
* Extracted server and gateway into builders, tidies complicated construction and makes it easier to test
* Move the cacheSize metric from a counter to a gauge and define in metrics.go and outside of DefaultCache func, removes duplicate panic in tests
* KiamGatewayBuilder adds WithMaxRetries to configure the gRPC retry behaviour (by default, doesn't retry operations) and potentially helps address #425
@pingles
Copy link
Contributor Author

pingles commented Oct 22, 2020

Latest, this definitely fixes #410.

It should also fix #368 which would have accessed the global endpoint while retrieving server credentials (and then using regional endpoints for pod credentials). The tests show it using a regional resolver and how it resolves the endpoints.

Copy link
Contributor

@Joseph-Irving Joseph-Irving left a comment

Choose a reason for hiding this comment

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

lgtm

@pingles pingles merged commit 1da27f1 into master Oct 23, 2020
@pingles pingles deleted the region-resolution branch October 23, 2020 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kiam fails to resolve regional sts endpoint in us-iso-east-1 and us-isob-east-1
2 participants