Initial commit of RubyV2 test server#5
Conversation
|
Requesting Copilot review since I can't Ruby |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an initial implementation of a Ruby V2 test server for S3 Encryption Client compatibility testing. The server provides a REST API wrapper around the AWS S3 Encryption Client v2 to enable cross-language compatibility testing alongside existing Java and Python implementations.
Key changes include:
- Complete Ruby server implementation with Sinatra-based REST API
- Client management system with caching for S3 encryption client instances
- Metadata serialization utilities and error handling to match Smithy model specifications
- Updated Java test cases to accommodate Ruby-specific error message variations
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test-server/ruby-v2-server/app.rb | Main Sinatra application implementing REST endpoints for client creation and S3 operations |
| test-server/ruby-v2-server/lib/client_manager.rb | Manages S3 encryption client instances with thread-safe caching |
| test-server/ruby-v2-server/lib/metadata_utils.rb | Utility class for metadata string/hash conversion matching Java/Python format |
| test-server/ruby-v2-server/lib/error_handlers.rb | Error response utilities matching Smithy model types |
| test-server/ruby-v2-server/lib/logger.rb | Centralized logging utility with structured request/response logging |
| test-server/ruby-v2-server/Gemfile | Ruby dependency specification |
| test-server/ruby-v2-server/config.ru | Rack configuration file |
| test-server/ruby-v2-server/README.md | Documentation for setup and usage |
| test-server/java-tests/src/it/java/software/amazon/encryption/s3/RoundTripTests.java | Updated test cases to handle Ruby-specific error messages |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
333d4a9 to
baca64a
Compare
| kms_key_id = config.dig('keyMaterial', 'kmsKeyId') | ||
| enable_legacy_wrapping = config['enableLegacyWrappingAlgorithms'] || false | ||
|
|
||
| raise 'KMS Key ID is required' if kms_key_id.nil? || kms_key_id.empty? |
There was a problem hiding this comment.
eventually we'll need RSA and AES keys as well but this is fine for now.
kessplas
left a comment
There was a problem hiding this comment.
only the comments about asserting Ruby when the message is the Ruby message are blocking
ShubhamChaturvedi7
left a comment
There was a problem hiding this comment.
LGTM. We might want to refactor out some of the repeated modules such as logger etc but it's a test server so not high priority.
710c9af to
0419eea
Compare
V2 and v3 pending changes
7b4dc23 to
11c92bc
Compare
765cb8c to
52ef9aa
Compare
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.