Repository navigation
fix: fix S3 Data Lake connector empty role_arn validation - #67005
Merged
Davin Chia (davinchia) merged 12 commits intoOct 7, 2025
Merged
Conversation
Contributor
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Helpful Resources
PR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
|
Contributor
|
Davin Chia (davinchia)
force-pushed
the
fix/s3-data-lake-empty-role-arn-validation
branch
2 times, most recently
from
October 2, 2025 04:53
28c32d0 to
b0181e3
Compare
Benoit Moriceau (benmoriceau)
approved these changes
Oct 2, 2025
Edward Gao (edgao)
approved these changes
Oct 2, 2025
Edward Gao (edgao)
left a comment
Edward Gao (edgao)
left a comment
Contributor
There was a problem hiding this comment.
one comment about logic, otherwise lgtm
| // Validate that either both credentials are provided or neither (for instance profiles) | ||
| if ((awsAccessKeyId == null) != (awsSecretAccessKey == null)) { | ||
| throw IllegalArgumentException( | ||
| "Both access_key_id and secret_access_key must be provided together, or neither (to use instance profiles)" |
Contributor
There was a problem hiding this comment.
in assume role mode, we always use the key+secret
- Change role_arn validation to check for both null and blank strings - Empty string role_arn is now treated as null and uses key-based auth - Refactor tests for better conciseness with helper function - Use consistent Kotlin patterns matching existing codebase style Fixes airbytehq/airbyte-internal-issues#14643
Davin Chia (davinchia)
force-pushed
the
fix/s3-data-lake-empty-role-arn-validation
branch
from
October 2, 2025 16:05
b0181e3 to
be9142a
Compare
- Update version from 0.3.36 to 0.3.37 - Add changelog entry for empty string role_arn fix
Davin Chia (davinchia)
marked this pull request as ready for review
October 2, 2025 16:23
Contributor
Author
|
/format-fix
|
Contributor
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 3caf38d. |
Edward Gao (edgao)
approved these changes
Oct 3, 2025
Use UUID-based table names for check operation test tables to prevent conflicts with: - Stale metadata from previous check runs - Concurrent check operations - User tables named airbyte_test_table This fixes the integration test failure caused by corrupted Glue catalog metadata pointing to non-existent S3 files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ata-lake-empty-role-arn-validation
Davin Chia (davinchia)
force-pushed
the
fix/s3-data-lake-empty-role-arn-validation
branch
from
October 7, 2025 00:41
f465b5a to
109a3dd
Compare
…tables Wrap test table creation in try-finally to ensure cleanup always occurs, even if validation logic is added in the future or if the operation fails. This prevents accumulation of orphaned test tables in Glue catalog and S3.
…ata-lake-empty-role-arn-validation
Update version and changelog for empty role_arn validation fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The configUpdater is not needed for check tests as they use actual config values directly, not test infrastructure placeholders. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Davin Chia (davinchia)
enabled auto-merge (squash)
October 7, 2025 18:31
Davin Chia (davinchia)
deleted the
fix/s3-data-lake-empty-role-arn-validation
branch
October 7, 2025 18:38
Matteo Palarchio (matteogp)
pushed a commit
that referenced
this pull request
Oct 10, 2025
- Change role_arn validation to check for both null and blank strings
- Add validation for mismatched AWS credentials (only one of
access_key_id or secret_access_key provided)
- Add comprehensive tests for empty string role_arn, null role_arn, and
credential validation
- Improve error messages for better debugging
Fixes airbytehq/airbyte-internal-issues#14643
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes airbytehq/airbyte-internal-issues#14643
What
How
Review guide
User Impact
Can this PR be safely reverted and rolled back?
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.