From 5bc67661f2896ae357fa1b31e6693ff22e1e8d97 Mon Sep 17 00:00:00 2001 From: Keery Nie Date: Wed, 21 Aug 2024 16:01:17 +0800 Subject: [PATCH] fix(region_config): fix configure endpoint bug in getRegionPrefix (#129) ## 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` --- spec/04-services/05-sts_spec.lua | 120 +++++++++++++++++++++++++++++++ src/resty/aws/init.lua | 13 ++-- 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/spec/04-services/05-sts_spec.lua b/spec/04-services/05-sts_spec.lua index cb7660e..2c318db 100644 --- a/spec/04-services/05-sts_spec.lua +++ b/spec/04-services/05-sts_spec.lua @@ -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) diff --git a/src/resty/aws/init.lua b/src/resty/aws/init.lua index 9d3b9a1..7894c7d 100644 --- a/src/resty/aws/init.lua +++ b/src/resty/aws/init.lua @@ -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" @@ -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 @@ -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", -- "*/*", -- }