Skip to content

Conversation

@ianwow
Copy link
Contributor

@ianwow ianwow commented Mar 17, 2025

🔧 Type of changes

  • new bid adapter
  • bid adapter update
  • new feature
  • new analytics adapter
  • new module
  • module update
  • bugfix
  • documentation
  • configuration
  • dependency update
  • tech debt (test coverage, refactorings, etc.)

✨ What's the context?

Currently, the S3 integration for prebid-server-java requires AWS credentials to be explicitly defined in the config yaml file. The SettingsConfiguration class in PBS Java uses AwsBasicCredentials to authenticate S3 requests, making credential storage in the YAML file mandatory. However, this is not good practice for credential management. This PR adds another credentials provider that will look for credentials in this order:

  • Java System Properties
  • Environment Variables
  • Web Identity Token
  • AWS credentials file (~/.aws/credentials)
  • ECS container credentials
  • EC2 instance profile

🧠 Rationale behind the change

So that S3 integration can be used with the recommended practices for AWS credential management.

🧪 Test plan

  • If accessKeyId and secretAccessKey are provided in the config yaml file, then verify that they are still used.
  • If they are not provided in the config yaml file, then verify that AWS credentials are found in the order described above.

🏎 Quality check

  • [ x ] Are your changes following our code style guidelines?
  • [ no ] Are there any breaking changes in your code?
  • [ yes ] Does your test coverage exceed 90%?
  • [ no ] Are there any erroneous console logs, debuggers or leftover code in your changes?

muuki88
muuki88 previously approved these changes Mar 21, 2025
Copy link
Contributor

@muuki88 muuki88 left a comment

Choose a reason for hiding this comment

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

This is great. Thanks for improving this feature 😊

osulzhenko
osulzhenko previously approved these changes Mar 25, 2025
@ianwow ianwow dismissed stale reviews from muuki88 and osulzhenko via 7c5f309 April 2, 2025 22:48
@ianwow ianwow requested a review from CTMBNara April 3, 2025 04:35
ianwow added 9 commits April 3, 2025 09:08
The "HookStageExecutorTest#shouldExecuteEntrypointHooksToleratingTimeoutAndFailedFuture" test has been failing intermittently due to strict time boundaries (50-70ms) 
for hook execution. This commit increases the acceptable range to account for slight variations in execution time across different environments.

- Previous range: 50-70ms
- New range: 50-80ms
Adopting feedback from PR. prebid#3842 (comment)
@CTMBNara CTMBNara merged commit c191cd5 into prebid:master Apr 25, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants