Skip to content

Fix for functionnames where functionname A startwith functionname B #162

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

Merged
merged 5 commits into from
May 10, 2019

Conversation

MartinRisk
Copy link
Member

.find would find wrong versionname and aliasname

Example:
Function A: GetAccount
Function B: GetAccountByName

For function GetAccountByName the versionname and aliasname would be from the GetAccount function

.find would find wrong versionname and aliasname
@Enase Enase added the bug label May 9, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…function name B

- test stack updated with case that covers the issue
@Enase
Copy link
Contributor

Enase commented May 9, 2019

I've added commit with:

  • fix for SNS (the same issue as was fixed for API Gateway by @MartinRisk)
  • stack and alias fixtures updated with function name overlaps in order to cover current issue (related tests updated)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@Enase
Copy link
Contributor

Enase commented May 9, 2019

@HyperBrain pls review

const versionName = _.find(_.keys(versions), version => _.startsWith(version, functionName));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, functionName));
const isExternalRef = isExternalRefAuthorizerPredicate(permission.Properties.FunctionName);
const versionName = _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));

Choose a reason for hiding this comment

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

this will work, but you essentially not checking for _.startsWith anymore but for string equality. Shouldn't then a simple === suffice?
May be to be more robust this should be _startsWith(version

Copy link
Contributor

Choose a reason for hiding this comment

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

getFunctionVersionName and getAliasVersionName functions added to utils

Copy link

@aleksdikanski aleksdikanski left a comment

Choose a reason for hiding this comment

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

the way to retrieve the version name and the alias name is repeated multiple times. Maybe this could be move to utility functions and defined only once. e.g.

function getVersionName(versions, functionName) {
    return _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));
}

similar for aliasName

- getFunctionVersionName and getAliasVersionName function added to utils
@Enase Enase merged commit dd1dbba into master May 10, 2019
@Enase Enase deleted the fix-for-naming-issue branch May 10, 2019 16:17
defnotrobbie pushed a commit to 1stdibs/serverless-aws-alias that referenced this pull request Nov 14, 2022
…or-naming-issue

Fix for functionnames where functionname A startwith functionname B
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants