serverless-operations / serverless-operations/serverless-step-functions

Allow additions to generated Permissions; use more stable resource naming

Open
#420 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area: iam enhancement
Dominant language
JavaScript
Stars
1k
Forks
220
PR merge metrics
No merged PRs in 30d

Description

This is a Feature Proposal

Description

The permissions generated, for example with a Lambda triggered by a CloudWatch Notification on a State Machine status change, are generated with static content other than the Lambda they reference. It would be nice to be able to fold in further restrictions (like a SourceAccount) to help restrict that permission.

Alternatively, a user is able to modify the generated resource after the fact via a Resources section, but that poses a second challenge: the resource name is generated as an MD5 hash of the index of the notification, and the resource object itself. This means that if the underlying code used to generate the Event changes (like to introduce the above feature), this permission's suffix is going to change which could break existing customizations done in this way.

For the future, it seems like it could be beneficial to simplify it to generating the name based off something more stable/predictable like the Lambda's name. That way the only method to cause a change to the generated Permission name is by taking an action yourself, and not due to a future code change to the plugin.

The only conflict that this could create as currently written is that the generator currently blindly creates the permissions, regardless of if they exist or not, meaning you end up with duplicates. For instance, if you have an error handler Lambda that is triggered by both a FAILED and a TIME_OUT state change, you will end up with two identical Lambda Permission policies in the generated CFT (currently with different names due to the index being part of the hash). If instead, the permission generation checked for duplicates and just created a singular one, the index could be removed as a part of this function and it seems like it could rely safely on Name alone (and you wouldn't end up with the duplicate permission).

Unfortunately I think at this point it might have to be considered a breaking change though, as altering the way the name is generated, or modifying the generated permission code, would likely cause the resource names to change on people (although it may be possible to fold in additional things like SourceAccount after the name generating hash function has run to avoid the name change...)

Anyways -- just a thought on an improvement! Really appreciate this plugin.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/deploy/stepFunctions/compileNotifications.js around the referenced generated-permission code. Review how notification permissions are named and created, then define how additional restrictions, duplicate permissions, and stable Lambda-based names should work while accounting for the possible breaking change.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, javascript
Domain
cloud
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.