Skip to content

StepFunctions: Improvements to Timestamp Validation #12074

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 4 commits into from
Jan 20, 2025

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Dec 27, 2024

Motivation

The Step Functions (SFN) v2 interpreter has certain limitations in validating timestamp strings for the Timestamp field. This field can be set statically using string literals or dynamically, such as via JSONata expressions. While validation exists for TimestampPath fields, similar logic was not applied to Timestamp fields. Consequently, some timestamps failed validation during state machine creation, as described in #11997.

These changes enhance the validation logic to ensure both statically provided and dynamically computed timestamp values are checked for proper format and a valid date. Additionally, a series of positive and negative test cases have been added to verify timestamp handling.

It is worth mentioning that dynamically computed invalid timestamps via JSONata expressions result in a generic execution failure rather than a new evaluation failure. This discrepancy is limited to the error type, while the failure description correctly conveys the issue. Improvements to this behaviour are planned as we introduce validation patterns for JSONata expression outcomes.

Changes

  • Enhanced timestamp validation logic for both static and dynamically computed values
  • Added positive and negative snapshot tests for Timestamp* fields

@MEPalma MEPalma added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Dec 27, 2024
@MEPalma MEPalma added this to the 4.1 milestone Dec 27, 2024
@MEPalma MEPalma self-assigned this Dec 27, 2024
Copy link

github-actions bot commented Dec 27, 2024

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   34m 10s ⏱️ - 1h 16m 57s
1 389 tests  - 2 560  1 318 ✅  - 2 322  71 💤  - 238  0 ❌ ±0 
1 391 runs   - 2 560  1 318 ✅  - 2 322  73 💤  - 238  0 ❌ ±0 

Results for commit a6f3688. ± Comparison against base commit 630b2a1.

This pull request removes 2582 and adds 22 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.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp[NANOSECONDS]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp[SECONDS]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[INVALID_DATE]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[INVALID_ISO]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[INVALID_TIME]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[JSONATA]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[NO_T]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_invalid[NO_Z]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[INVALID_DATE]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[INVALID_ISO]
…
This pull request removes 243 skipped tests and adds 5 skipped tests. Note that renamed tests count towards both.
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_api_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_invalid_desiredstate
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_double_create_with_client_token
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_lifecycle
…
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[INVALID_DATE]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[INVALID_ISO]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[INVALID_TIME]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[NO_T]
tests.aws.services.stepfunctions.v2.scenarios.test_base_scenarios.TestBaseScenarios ‑ test_wait_timestamp_jsonata[NO_Z]

♻️ This comment has been updated with latest results.

@tiurin tiurin self-requested a review January 20, 2025 10:08
Copy link
Contributor

@tiurin tiurin left a comment

Choose a reason for hiding this comment

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

Great to see improved test coverage! 👍

@@ -60,31 +72,34 @@ def _create_failure_event(self, env: Environment, timestamp_str: str) -> Failure
event_details=EventDetails(
executionFailedEventDetails=ExecutionFailedEventDetails(
error=StatesErrorNameType.StatesRuntime.to_name(),
cause=f"The TimestampPath parameter does not reference a valid ISO-8601 extended offset date-time format string: {self.string.literal_value} == {timestamp_str}",
cause="The Timestamp parameter does not reference a valid ISO-8601 "
f"extended offset date-time format string: {self.string.literal_value} == {timestamp_str}",
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why do we need to print both literal value of object property and timestamp_str parameter? Also, what == here means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This error message is composed in an effort to match AWS's error message in these circumstances (example).
The message lists how the expression provided for sampling the timestamp {self.string.literal_value} evaluates to the invalid timestamp value {timestamp_str}. The error message may look like this: The TimestampPath parameter does not reference a valid ISO-8601 extended offset date-time format string: $.TimestampValue == 2016-12-05 21:29:29Z"

@MEPalma MEPalma merged commit 5bbacec into master Jan 20, 2025
31 checks passed
@MEPalma MEPalma deleted the MEP-sfn-wait_timestamp_validation branch January 20, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants