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.

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

fix(region_config): fix configure endpoint bug in getRegionPrefix #129

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented Aug 15, 2024

Summary

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

local function generateRegionPrefix(region)
if not region then
return nil, "no region given"
end
local parts = split(region, "-", true)
if #parts < 3 then
return nil, "not a valid region, only 2 parts; "..region
end
parts[#parts] = "*"
return table.concat(parts, "-")
end

This code originates from the JS code here:

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

@windmgc windmgc requested a review from Tieske August 15, 2024 08:52
Copy link

github-actions bot commented Aug 15, 2024

Luacheck Report

2 tests   2 ✅  0s ⏱️
1 suites  0 💤
1 files    0 ❌

Results for commit 93d3c6b.

♻️ This comment has been updated with latest results.

src/resty/aws/init.lua Outdated Show resolved Hide resolved
@windmgc windmgc merged commit 5bc6766 into main Aug 21, 2024
9 checks passed
@windmgc windmgc deleted the fix-cn-region-sts-endpoint branch August 21, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants