Skip to content

Commit

Permalink
fix(region_config): fix configure endpoint bug in getRegionPrefix (#129)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes a bug inside `generateRegionPrefix` that caused some service endpoint in specific region is not getting configured correctly.

https://github.com/Kong/lua-resty-aws/blob/50d9f0849eb03d4c2a3d7cc8a8cd10344a14df12/src/resty/aws/init.lua#L138-L149

This code originates from the [JS code here](https://github.com/aws/aws-sdk-js/blob/c0ec9d31057748cda57eac863273f5ef5a695782/lib/region_config.js#L4-L9):
```
function generateRegionPrefix(region) {
  if (!region) return null;
  var parts = region.split('-');
  if (parts.length < 3) return null;
  return parts.slice(0, parts.length - 2).join('-') + '-*';
}
```

There is a bug in our Lua code, that `parts.slice(0, parts.length - 2).join('-') + '-*'` is fetching the `[0, #parts-2)` items from the region parts and concatenating with another asterisk. But our SDK is just replacing the last item in the array with an asterisk, which equals fetching `[0, #parts-2]` and concatenating with another asterisk. (Here I'm using an index starting with 0 to clarify the difference). This caused different results when we generated region prefixes: a region `cn-north-1` will result in `cn-*` in the JS function and `cn-north-*` in the Lua function.

The PR fixes it and lets the `region_config_data` apply correctly.


## Issue

FTI-6159 mentioned an issue caused by this bug, which happens inside `cn-north-1` that is expected to apply the `cn-*/*` endpoint config. The bug caused a mismatch and endpoint config cannot be applied correctly. This bug would also influence other regions like `us-isob-east-1`
  • Loading branch information
windmgc authored Aug 21, 2024
1 parent 50d9f08 commit 5bc6766
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 4 deletions.
120 changes: 120 additions & 0 deletions spec/04-services/05-sts_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,124 @@ describe("STS service", function()
end)
end)
end

-- CN Region check, the STS endpoint will be suffixed with ".com.cn"
-- For CN Region there will be no region injections since globalEndpoint
-- is not defined for "cn-*/*" in region_config_data.lua
for _, region in ipairs({"cn-north-1", "cn-northwest-1"}) do
describe("In Region #" .. region, function ()
-- before_each(function()
-- aws.config.region = region
-- end)

it("AWS_STS_REGIONAL_ENDPOINT==regional with default endpoint", function ()
local config = {
region = region,
stsRegionalEndpoints = "regional",
dry_run = true,
}

local sts = aws:STS(config)
local request = sts:assumeRole({
RoleArn = test_assume_role_arn,
RoleSessionName = test_role_session_name,
})

assert.same(sts.config.stsRegionalEndpoints, "regional")
assert.is_nil(sts.config.signingRegion)
assert.falsy(sts.config._regionalEndpointInjected)
-- Check the endpoint has not been injected
assert.same(sts.config.endpoint, "sts." .. region .. ".amazonaws.com.cn")
assert.not_nil(request.headers.Authorization:find(region, 1, true))
end)

describe("AWS_STS_REGIONAL_ENDPOINT==regional with non-default endpoint", function()
it("and endpoint is regional domain", function ()
local config = {
region = region,
stsRegionalEndpoints = "regional",
endpoint = "https://sts." .. region .. ".amazonaws.com.cn",
dry_run = true,
}

local sts = aws:STS(config)
local request = sts:assumeRole({
RoleArn = test_assume_role_arn,
RoleSessionName = test_role_session_name,
})

assert.same(sts.config.stsRegionalEndpoints, "regional")
assert.is_nil(sts.config.signingRegion)
assert.falsy(sts.config._regionalEndpointInjected)
-- Check thes endpoint has not been injected
assert.same(sts.config.endpoint, config.endpoint)
assert.not_nil(request.headers.Authorization:find(region, 1, true))
end)

it("and endpoint is region VPC endpoint", function ()
local config = {
region = region,
stsRegionalEndpoints = "regional",
endpoint = "https://vpce-1234567-abcdefg.sts." .. region .. ".vpce.amazonaws.com",
dry_run = true,
}

local sts = aws:STS(config)
local request = sts:assumeRole({
RoleArn = test_assume_role_arn,
RoleSessionName = test_role_session_name,
})

assert.same(sts.config.stsRegionalEndpoints, "regional")
assert.is_nil(sts.config.signingRegion)
assert.falsy(sts.config._regionalEndpointInjected)
-- Check the endpoint has not been injected when endpoint is a vpc endpoint
assert.same(sts.config.endpoint, config.endpoint)
assert.not_nil(request.headers.Authorization:find(region, 1, true))
end)

it("and endpoint is AZ VPC endpoint", function ()
local config = {
region = region,
stsRegionalEndpoints = "regional",
endpoint = "https://vpce-1234567-abcdefg-" .. region .. "c" .. ".sts." .. region .. ".vpce.amazonaws.com",
dry_run = true,
}

local sts = aws:STS(config)
local request = sts:assumeRole({
RoleArn = test_assume_role_arn,
RoleSessionName = test_role_session_name,
})

assert.same(sts.config.stsRegionalEndpoints, "regional")
assert.is_nil(sts.config.signingRegion)
assert.falsy(sts.config._regionalEndpointInjected)
-- Check the endpoint has not been injected when endpoint is a vpc endpoint
assert.same(sts.config.endpoint, config.endpoint)
assert.not_nil(request.headers.Authorization:find(region, 1, true))
end)
end)

it("AWS_STS_REGIONAL_ENDPOINT==legacy with default endpoint", function ()
local config = {
region = region,
stsRegionalEndpoints = "legacy",
dry_run = true,
}

local sts = aws:STS(config)
local request = sts:assumeRole({
RoleArn = test_assume_role_arn,
RoleSessionName = test_role_session_name,
})

assert.same(sts.config.stsRegionalEndpoints, "legacy")
assert.is_nil(sts.config.signingRegion)
assert.falsy(sts.config._regionalEndpointInjected)
assert.same(sts.config.endpoint, "sts." .. region .. ".amazonaws.com.cn")
assert.not_nil(request.headers.Authorization:find(region, 1, true))
end)
end)
end
end)
13 changes: 9 additions & 4 deletions src/resty/aws/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,10 @@ end


do
-- https://github.com/aws/aws-sdk-js/blob/c0ec9d31057748cda57eac863273f5ef5a695782/lib/region_config.js#L4
-- returns the region with the last element replaced by "*"
-- "us-east-1" --> "us-east-*"
-- "us-east-1" --> "us-*"
-- "us-isob-west-1" --> "us-isob-*"
local function generateRegionPrefix(region)
if not region then
return nil, "no region given"
Expand All @@ -144,7 +146,10 @@ do
if #parts < 3 then
return nil, "not a valid region, only 2 parts; "..region
end
parts[#parts] = "*"

local n_parts = #parts
parts[n_parts] = nil
parts[n_parts - 1] = "*"
return table.concat(parts, "-")
end

Expand All @@ -159,9 +164,9 @@ do
-- 'sts' configured for region 'us-west-2';
-- {
-- "us-west-2/sts",
-- "us-west-*/sts",
-- "us-*/sts",
-- "us-west-2/*",
-- "us-west-*/*",
-- "us-*/*",
-- "*/sts",
-- "*/*",
-- }
Expand Down

0 comments on commit 5bc6766

Please sign in to comment.