Skip to content

Events create connection test fix #11904

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 2 commits into from
Nov 26, 2024

Conversation

zaingz
Copy link
Contributor

@zaingz zaingz commented Nov 22, 2024

Motivation

The current EventBridge connection validation in LocalStack lacks error handling and doesn't fully align with AWS specifications. This leads to:

  1. Incomplete validation feedback - errors are reported individually rather than collectively
  2. Inconsistent error messaging compared to AWS
  3. Early termination on first validation error instead of reporting all validation issues
  4. Potential confusion for users due to different validation behavior from AWS

Changes

This PR enhances the EventBridge connection validation mechanism to more closely mirror AWS behavior:

Validation Logic

  • Modified validation methods to collect and return multiple errors simultaneously
  • Implemented comprehensive validation for connection names:
    • Length constraints (1-64 characters)
    • Character set validation (alphanumeric, periods, hyphens, underscores)
    • Pattern matching using regex ^[\\.\\-_A-Za-z0-9]+$
  • Enhanced authorization type validation to support:
    • BASIC
    • OAUTH_CLIENT_CREDENTIALS
    • API_KEY

Error Handling

  • Updated validation methods to return lists of validation errors instead of raising immediate exceptions
  • Implemented combined error reporting in create_connection()
  • Improved error message formatting to match AWS specifications
  • Added proper pluralization for error messages based on error count

Copy link

github-actions bot commented Nov 22, 2024

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 3m 1s ⏱️ - 39m 45s
2 029 tests  - 1 539  1 914 ✅  - 1 321  115 💤  - 218  0 ❌ ±0 
2 031 runs   - 1 539  1 914 ✅  - 1 321  117 💤  - 218  0 ❌ ±0 

Results for commit 8aa78cf. ± Comparison against base commit 7c2d1e3.

This pull request removes 1539 tests.
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]
…

♻️ This comment has been updated with latest results.

@zaingz zaingz self-assigned this Nov 22, 2024
@zaingz zaingz added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases aws:events Amazon EventBridge semver: patch Non-breaking changes which can be included in patch releases and removed semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Nov 22, 2024
@zaingz zaingz marked this pull request as ready for review November 25, 2024 16:39
Copy link
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

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

Well-validated and thoroughly described PR :)
The bump type fits well 👍

Let's adjust the PR title before merging 🚀

@zaingz zaingz changed the title Test fix Events create connection test fix Nov 26, 2024
@zaingz zaingz merged commit 5f19fbc into master Nov 26, 2024
48 of 50 checks passed
@zaingz zaingz deleted the events/test_create_connection_validations branch November 26, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:events Amazon EventBridge semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants