Skip to content

S3 storage support #14

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.

Sign up for GitHub

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

Jump to bottom
Closed
arkavo-com wants to merge 15 commits into from
Closed

S3 storage support #14

arkavo-com wants to merge 15 commits into from

Conversation

@arkavo-com
Copy link
Contributor

@arkavo-com arkavo-com commented Jan 12, 2025 •
edited
Loading

Summary

  • Add S3 storage support for storing and retrieving events
  • Implement AWS SDK integration with proper configuration for both real AWS and LocalStack
  • Add comprehensive test suite with both unit tests and integration tests

Implementation Details

  • Add S3 client initialization with proper configuration handling
  • Support both production AWS and LocalStack environments
  • Implement fallback mechanisms for LocalStack compatibility
  • Add environment detection for tests

Testing

  • Unit tests with mocks for core S3 functionality
  • Integration tests that run with LocalStack in CI
  • Tests can also run against real AWS with proper credentials

CI/CD

  • GitHub Actions workflow runs integration tests with LocalStack
  • Tests handle differences between AWS SDK and LocalStack

Documentation

  • Added usage examples in tests
  • Included configuration details in test files

🤖 Generated with Claude Code

arkavo-com and others added 8 commits January 11, 2025 21:54
@arkavo-com
Add S3 storage support for events and initialize S3 client
6e76cc8 
Integrated event storage functionality via AWS S3, adding a new action to store event payloads. Updated the ServerState structure and initialization to include the S3 client. Adjusted configuration and documentation to support S3, including new environment variables and library dependencies.
@arkavo-com @claude
Add S3 integration tests and GitHub Actions workflow
f42b5a7 
- Created tests for S3 storage functionality
- Added event storage integration tests
- Set up GitHub Actions workflow with LocalStack for S3 testing
- Updated README with testing instructions

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix GitHub Actions workflow for LocalStack integration
c4c6b81 
- Remove volume mount that was causing errors
- Fix environment variables for better compatibility

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix AWS CLI installation in workflow
90e118c 
- Use existing AWS CLI in GitHub runner
- Remove unnecessary installation steps

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix S3 integration tests for LocalStack compatibility
b8ff53f 
- Use AWS CLI instead of SDK for LocalStack operations
- Add fallback mechanisms for different environments
- Improve test resilience

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix S3 integration test compile errors
d7c995b 
- Use 'ref' pattern to avoid moving endpoint_url
- Remove unused import

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix event storage integration test compile error
9fb3c44 
- Use 'ref' pattern to avoid moving endpoint_url in pattern matching
- Same fix as applied to the S3 integration test

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix formatting and remove unit tests from integration workflow
836e13e 
- Format code with rustfmt
- Remove unit tests from integration-tests workflow

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com
Copy link
Contributor Author

arkavo-com commented Mar 13, 2025

S3 Storage Implementation Review

As a senior developer reviewing this PR, I'm impressed with the overall structure and thoroughness. The implementation adds robust S3 storage capabilities with proper error handling and testing. Here's my detailed review:

Strengths

  • Environment Flexibility: Excellent handling of both real AWS and LocalStack environments. The conditional code branching is clean and maintainable.
  • Testing Strategy: Great mix of unit tests with mocks and integration tests with LocalStack. The tests are thoughtful and cover key scenarios.
  • Error Handling: Good attention to error handling throughout the S3 operations.
  • LocalStack Workarounds: Smart approach using AWS CLI fallbacks for operations that don't work with the SDK in LocalStack.
  • CI Integration: Well-implemented GitHub Actions workflow for running tests in a controlled environment.

Areas for Improvement

  1. Dependency Management:

    • Consider using a specific version constraint for aws-sdk-s3 rather than a floating one, to prevent unexpected breaking changes.

  2. Code Organization:

    • The S3 client initialization in ServerState::new() is getting complex. Consider extracting S3-specific setup to a separate function for better maintainability.

  3. Security Considerations:

    • AWS credentials handling is good, but add a comment reminding users never to hardcode these values in production.

  4. DevOps Efficiency:

    • LocalStack configuration is duplicated in both workflow and test files. Consider centralizing this configuration.

  5. Minor Issues:

    • .DS_Store file was accidentally committed. Add this to .gitignore.
    • There are still some warnings in the test files (unused variables, etc.) that should be addressed.

Conclusion

This is a solid implementation that achieves its goals and includes proper testing. The PR is ready for review by other team members.

Recommended tasks before merging:

  1. Add .DS_Store to .gitignore
  2. Fix the warnings in test files
  3. Consider the version constraint suggestion

Overall, this is high-quality work that follows best practices for cloud storage integration.

arkavo-com and others added 2 commits March 12, 2025 21:45
@arkavo-com @claude
Add .DS_Store to .gitignore
a7e4c6f 
- Address review feedback by ignoring macOS system files

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com
Delete src/bin/.DS_Store
0b98b30
@arkavo-com arkavo-com changed the title Add S3 storage support and initialize S3 client S3 storage support Mar 13, 2025
arkavo-com and others added 5 commits May 25, 2025 10:47
@arkavo-com @claude
Add support for nanoTDF v13 magic bytes
ff6915a 
Update handle_nats function to recognize both v12 (L1L) and v13 (L1M/TDFN) magic bytes when determining message type. This ensures v13 nanoTDF messages are correctly categorized as NATS messages (0x05) instead of being misclassified as events (0x06).

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Add comprehensive logging for rewrap request debugging
9f1c088 
Add detailed logging throughout the rewrap request flow to help diagnose why iOS clients aren't receiving 0x04 responses. Logs now track when requests are received, ephemeral keys processed, and responses sent through WebSocket. Also fixed clippy lint error.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Add v13 nanoTDF policy type support
c3fdf83 
Update policy field parsing to handle v13 policy types (0x02, 0x03) while maintaining v12 compatibility. Pass version byte to read_policy_field to enable version-specific parsing. This fixes InvalidPolicy errors when processing v13 nanoTDF headers.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Add detailed logging for v13 nanoTDF debugging
c7d93df 
Add logging to show magic bytes, version, and first bytes of rewrap payload to help diagnose v13 parsing issues.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@arkavo-com @claude
Fix v13 nanoTDF KAS field parsing
0d7c533 
Add support for v13's extended KAS structure which includes KAS Key Curve Enum and KAS Public Key fields after the KAS Endpoint Locator. This fixes the InvalidPolicy errors by correctly parsing the v13 header structure.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 25, 2025

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@arkavo-com arkavo-com closed this Oct 6, 2025
@arkavo-com arkavo-com deleted the feature/profile-image branch October 25, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@arkavo-com