Skip to content

Minio and proper ssl - #1318

Open
tagantank wants to merge 2 commits into
BoldGrid:masterfrom
tagantank:minio-and-proper-ssl
Open

Minio and proper ssl#1318
tagantank wants to merge 2 commits into
BoldGrid:masterfrom
tagantank:minio-and-proper-ssl

Conversation

@tagantank

Copy link
Copy Markdown

When trying to upload files to MinIO using the S3 Compatible engine, I experienced:

HTTP 400/404 errors during file uploads
SSL not working even when configured
Incorrect URL formatting for path-style endpoints

Root Causes

Hardcoded SSL: The CdnEngine_S3_Compatible class hardcodes SSL to false,
preventing HTTPS connections.
URL Style Issues: The plugin always tries virtual-hosted-style URLs when possible,
but MinIO requires path-style URLs.
Port Handling: When endpoints include a port (e.g., minio.example.com:443),
the port gets included in the Host header, causing HTTP 400 errors.
No Manual Override: There’s no way to explicitly choose between storage types
when auto-detection fails.

Proposed Solution

I’ve created a patch that adds:

Storage Type Selection: New cdn.s3_compatible.storage_type config option:

auto (default) – backward compatible auto-detection
aws – force virtual-hosted-style URLs
minio – force path-style URLs

UI Enhancement: Dropdown field in CDN settings for easy selection

Proper SSL Detection:

Respects cdn.s3.ssl setting
Auto-detects SSL based on port 443
Passes correct SSL setting to S3Compatible

Port Extraction:

Separates port from hostname for proper Host header
Includes port in URL while excluding from Host header

Patch Details

Files Modified: 5

ConfigKeys.php – adds storage_type config key
inc/options/cdn/s3_compatible.php – adds UI dropdown
Cdn_Core.php – passes storage_type to engine
CdnEngine_S3_Compatible.php – implements SSL logic and storage type handling
lib/S3Compatible.php – adds port extraction and storage type support

Backward Compatibility: Fully compatible – defaults to ‘auto’ mode

Tested With:

MinIO server (path-style)
AWS S3 compatible services (virtual-hosted-style)
HTTPS endpoints with port 443
HTTP endpoints

…Total Cache

- Configuration key `cdn.s3_compatible.storage_type` in `ConfigKeys.php`
- UI dropdown for storage type selection (Auto/AWS/MinIO) in CDN settings
- Support for path-style URLs (MinIO) and virtual-hosted-style URLs (AWS S3)
- Automatic SSL detection based on port 443 in endpoint
- Separate port handling in S3 API requests

- Modified `Cdn_Core.php` to pass `storage_type` to engine configuration
- Updated `CdnEngine_S3_Compatible.php`:
  - Proper SSL detection from configuration
  - Automatic SSL detection by port 443
  - Pass `storage_type` parameter to S3Compatible class
- Updated `lib/S3Compatible.php`:
  - Store port separately from hostname (`$endpointPort` static property)
  - Store storage type (`$storageType` static property)
  - URL style determination based on `storage_type`:
    - `aws`: virtual-hosted-style (bucket.host/path)
    - `minio`: path-style (host:port/bucket/path)
    - `auto`: automatic detection by endpoint domain
  - Proper port handling in URL construction (without duplication)
  - Proper Host header formation (without port for virtual-hosted-style)

- Fixed 404 error when uploading to MinIO (incorrect URL style)
- Fixed 400 error when uploading to MinIO (incorrect Host header and path construction)
- Fixed parse error in `S3Compatible.php` after applying patches

- Added `setStorageType()` static method to `S3Compatible` class
- Modified `S3Request` constructor to use `storage_type` for URL style determination
- Updated `getResponse()` method to include port in URL when needed
- Port extraction from endpoint in `__construct()` and `setEndpoint()` methods
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 1.54%. Comparing base (3e44853) to head (668bf99).

Files with missing lines Patch % Lines
lib/S3Compatible.php 0.00% 39 Missing ⚠️
CdnEngine_S3_Compatible.php 0.00% 19 Missing ⚠️
inc/options/cdn/s3_compatible.php 0.00% 11 Missing ⚠️
Cdn_Core.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1318      +/-   ##
============================================
- Coverage      1.54%    1.54%   -0.01%     
- Complexity    20415    20431      +16     
============================================
  Files           636      636              
  Lines        101964   102024      +60     
============================================
  Hits           1579     1579              
- Misses       100385   100445      +60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants