Skip to content

[SFN] Activity Name and Arn Validation Logic #11924

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 27, 2024
Merged

[SFN] Activity Name and Arn Validation Logic #11924

merged 1 commit into from
Nov 27, 2024

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Nov 25, 2024

Motivation

Currently, the SFN v2 provider applies faulty activity arn and name validation logics, which can result is InvalidName exceptions during activity creation and especially during DescribeActivity api actions. This is espiecially true for activity names that contain punctuations, as described in #11916. This is due to the activity arn validation not being aligned with AWS SFN and the activity name validation (used on creation). These changes address both issues and add positive and negative snapshot tests for the creation deletion and description of activity names and arns.

Changes

  • positive and negative activity name/arn tests
  • updated activity arn validation rule
  • updated activity name validation logic to check for maximum name lenght

@MEPalma MEPalma added the semver: patch Non-breaking changes which can be included in patch releases label Nov 25, 2024
@MEPalma MEPalma self-assigned this Nov 25, 2024
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   34m 41s ⏱️ - 1h 14m 21s
1 264 tests  - 2 463  1 170 ✅  - 2 210  94 💤  - 253  0 ❌ ±0 
1 266 runs   - 2 463  1 170 ✅  - 2 210  96 💤  - 253  0 ❌ ±0 

Results for commit 7598132. ± Comparison against base commit 4ff3829.

This pull request removes 2498 and adds 35 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[\x00activity]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity"name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity#name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity$name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity%name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity&name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity*name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity,name]
tests.aws.services.stepfunctions.v2.test_sfn_api_activities.TestSnfApiActivities ‑ test_create_activity_invalid_name[activity/name]
…

dominikschubert
dominikschubert approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this so quickly 🚀

tests/aws/services/stepfunctions/v2/test_sfn_api_activities.py
@@ -48,13 +63,40 @@ def test_create_describe_delete_activity(
sfn_snapshot.match("delete_activity_response_2", delete_activity_response_2)

@markers.aws.validated
@pytest.mark.parametrize(
Copy link
Member

Choose a reason for hiding this comment

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

praise: very thorough 👍

@MEPalma MEPalma merged commit cf4b124 into master Nov 27, 2024
36 checks passed
@MEPalma MEPalma deleted the MEP-sfn-activity_punctuation branch November 27, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominikschubert dominikschubert dominikschubert approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev is a code owner

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman is a code owner

Assignees

@MEPalma MEPalma

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MEPalma @dominikschubert