Skip to content
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.

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

feat: support bucket region for all routes #35

Merged
merged 56 commits into from
Jan 2, 2025
Merged

Conversation

Juiced66
Copy link
Contributor

@Juiced66 Juiced66 commented Nov 25, 2024

Modularized Controllers, Bucket Controller, and Testing Migration

Summary

This PR introduces significant structural changes and enhancements to the kuzzle-plugin-s3 project. It modularizes controllers, adds a dedicated bucket controller with full CRUD capabilities, migrates tests to jest, and enforces stricter configuration requirements.

Key Changes

  1. Controller Refactoring:

    • Split controllers into separate modules for improved maintainability and scalability.
    • Modularized design ensures cleaner separation of concerns and easier future enhancements.
  2. Bucket Controller:

    • Implements CRUD operations for S3 buckets:
      • Create: Create buckets with region-specific configurations.
      • Read: Check bucket existence.
      • Update: Update policies or public access settings.
      • Delete: Safely remove empty buckets.
      • Empty: Empties a bucket.
    • Allows bucket configuration management (e.g., CORS, policies).
  3. Testing Framework Migration:

    • Migrated all tests to jest, replacing the existing framework.
    • Added comprehensive unit and integration tests to ensure reliability of the new features.
  4. Breaking Changes:

    • Mandatory Parameters:
      • bucketName and bucketRegion are now required for all applicable API calls.
    • Configuration Format:
      • The configuration file format has changed to support multi-region setups and enhanced flexibility.

Migration Notes

  • API Clients:
    • Ensure all requests include bucketName and bucketRegion where required.
  • Configuration Update:
    • Update your kuzzlerc and other configuration files to match the new format. Example configurations for multiple regions are provided in the updated documentation.

Benefits

  • Improved modularity and maintainability.
  • Comprehensive bucket management capabilities.
  • Enhanced test coverage for robust validation.
  • Compatibility with multi-region setups.

Testing and Validation

  • Verified functionality through extensive tests using jest.

Additional Notes

This is a breaking change. Ensure that configurations and client integrations are updated accordingly.

For a detailed view of changes, refer to the diff view.

Mathis Gardon and others added 17 commits November 21, 2024 13:43
…PublicAccess)

- also enable default CORS rules on bucket creation
- support API param dynamic change of bucket name
- fix fileGetUrl support expire time and bucket region
upgrade to uuid v9 was not taken into account in code use
stack trace is insufficient in plugin calling point to identify which step is causing a crash when creating a bucket
…ucket request params

- this is not handled at the moment because we need to disable block public request before setting a bucket policy that makes access public.
- it is then currently advided to first create the bucket, then disable blockPublicAccess, then put bucket policies in order to make sure bucket permissions are set in the right order.
- it could be a feature to support proper/automatic disable of block public access based on bucket policies ? see roadmap for this plugin
@Juiced66 Juiced66 self-assigned this Nov 25, 2024
@Juiced66 Juiced66 marked this pull request as draft November 26, 2024 15:24
Copy link
Member

@sebtiz13 sebtiz13 left a comment

Choose a reason for hiding this comment

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

Globally, why not use typescript in this plugin ?

tests/setup/jest.setup.js Outdated Show resolved Hide resolved
@Juiced66
Copy link
Contributor Author

Globally, why not use typescript in this plugin ?

I 100% agree it should, unfortunately, this PR is required ASAP and I cannot take time to do so

@Juiced66 Juiced66 mentioned this pull request Jan 2, 2025
@Juiced66 Juiced66 changed the base branch from master to beta January 2, 2025 13:10
@Juiced66 Juiced66 merged commit d2301b8 into beta Jan 2, 2025
8 checks passed
@kuzzle
Copy link

kuzzle commented Jan 2, 2025

🎉 This PR is included in version 2.2.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sebtiz13 sebtiz13 deleted the bucket-controller branch January 9, 2025 14:54
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