Conversation
Why are these changes being introduced: * Lambda needed role assumption capabilities * Clients were doing the same thing two different ways Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-680 How does this address that need: * Normalized the configuration for Lambda and OpenSearch clients to use consistent role assumption and credential handling by implementing a common AWS authentication abstraction.
Contributor
There was a problem hiding this comment.
Pull request overview
Normalizes AWS credential and role-assumption handling for Lambda and OpenSearch clients.
Changes:
- Adds shared AWS authentication and configuration validation.
- Updates Lambda and OpenSearch initialization.
- Renames AOSS role configuration to
AWS_ROLE_ARN. - Updates documentation and tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Summary |
|---|---|
test/initializers/lambda_config_test.rb |
Tests Lambda credential configuration. |
test/initializers/aws_config_validator_test.rb |
Updates shared configuration validation tests. |
README.md |
Documents AWS authentication settings; credential precedence requires clarification. |
lib/opensearch_config_validator.rb |
Removes the superseded validator. |
lib/aws_config_validator.rb |
Centralizes validation; migration support for AWS_AOSS_ROLE_ARN is required. |
lib/aws_auth.rb |
Provides shared credential and role-assumption helpers. |
config/initializers/opensearch.rb |
Uses shared OpenSearch authentication. |
config/initializers/lambda.rb |
Adds Lambda validation and role-based credentials. |
Suppressed comments (1)
config/initializers/lambda.rb:12
- When both
AWS_ROLE_ARNandAWS_SESSION_TOKENare set, Lambda takes this role-assumption branch, whileaws_aoss_clienttakes the session-token branch and uses the original credentials. The shared configuration can therefore make the two clients authenticate as different principals, contrary to the stated normalization (and the generic README text that a session token is used directly). Define one precedence rule—or reject the combination—and apply it consistently.
if AwsAuth.role_arn_present?
Rails.logger.debug 'Configuring Lambda client with assumed role credentials'
return AwsAuth.assume_role_credentials(role_session_name: 'timdex-lambda')
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+215
to
+219
| - `AWS_ROLE_ARN`: IAM role ARN to assume when using role-based AWS authentication. | ||
| Used by both OpenSearch (AOSS) and Lambda when `AWS_SESSION_TOKEN` is not set. | ||
| - `AWS_SESSION_TOKEN`: (Optional) AWS session token for temporary credentials when using expiring AWS credentials. | ||
| Use this with temporary AWS credentials for AWS-based OpenSearch access and Lambda. | ||
| For AOSS, when this is set, temporary credentials are used directly and `AWS_AOSS_ROLE_ARN` is not needed. | ||
| When this is set, temporary credentials are used directly and `AWS_ROLE_ARN` is not needed. |
| } | ||
|
|
||
| # Required only when AWS_SESSION_TOKEN is not present (using role assumption) | ||
| required_vars['AWS_ROLE_ARN'] = ENV.fetch('AWS_ROLE_ARN', nil) if ENV['AWS_SESSION_TOKEN'].blank? |
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
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.
Why are these changes being introduced:
Relevant ticket(s):
How does this address that need:
Developer
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Code Reviewer
(not just this pull request message)
Requires database migrations?
NO
Includes new or updated dependencies?
NO