Skip to content
Open
40 changes: 25 additions & 15 deletions operator/internal/handlers/internal/storage/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,36 +482,46 @@
}
}

func validateS3Endpoint(endpoint string, region string) error {
func validateS3Endpoint(endpoint, region string) error {
if len(endpoint) == 0 {
return fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSEndpoint)
}

parsedURL, err := url.Parse(endpoint)
u, err := url.Parse(endpoint)
if err != nil {
return fmt.Errorf("%w: %w", errS3EndpointUnparseable, err)
}

if parsedURL.Scheme == "" {
// Assume "just a hostname" when scheme is empty and produce a clearer error message
if u.Scheme == "" {
return errS3EndpointNoURL
}
if u.Scheme != "http" && u.Scheme != "https" {
return fmt.Errorf("%w: %s", errS3EndpointUnsupportedScheme, u.Scheme)
}

if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
return fmt.Errorf("%w: %s", errS3EndpointUnsupportedScheme, parsedURL.Scheme)
// Non-AWS S3 compatible endpoints (e.g., MinIO) - no further validation needed
if !strings.HasSuffix(endpoint, awsEndpointSuffix) {
return nil
}

if strings.HasSuffix(endpoint, awsEndpointSuffix) {
if len(region) == 0 {
return fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSRegion)
}
if len(region) == 0 {
return fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSRegion)
}

validEndpoint := fmt.Sprintf("https://s3.%s%s", region, awsEndpointSuffix)
if endpoint != validEndpoint {
return fmt.Errorf("%w: %s", errS3EndpointAWSInvalid, validEndpoint)
// VPC endpoints: reject bucket-specific prefixes
host := u.Hostname()
if strings.Contains(host, ".vpce.amazonaws.com") && strings.Contains(host, region) {
if !strings.HasPrefix(host, "vpce-") {
return fmt.Errorf("bucket name must not be included in VPC endpoint URL")

Check failure on line 514 in operator/internal/handlers/internal/storage/secrets.go

View workflow job for this annotation

GitHub Actions / lint

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"bucket name must not be included in VPC endpoint URL\")" (err113)
}
return nil
}

// Standard AWS S3 endpoint
valid := fmt.Sprintf("https://s3.%s%s", region, awsEndpointSuffix)
if endpoint == valid {
return nil
}
return nil
return fmt.Errorf("%w: %s", errS3EndpointAWSInvalid, valid)
}

func extractS3SSEConfig(d map[string][]byte) (storage.S3SSEConfig, error) {
Expand Down
33 changes: 33 additions & 0 deletions operator/internal/handlers/internal/storage/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,39 @@ func TestS3Extract_ForcePathStyle(t *testing.T) {
ForcePathStyle: true,
},
},
{
desc: "aws s3 vpc endpoint with bucket name should fail",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Data: map[string][]byte{
"endpoint": []byte("https://bucket.vpce-1234567-us-east-1c.s3.us-east-1.vpce.amazonaws.com"),
"region": []byte("us-east-1"),
"bucketnames": []byte("this,that"),
"access_key_id": []byte("id"),
"access_key_secret": []byte("secret"),
},
},
wantError: "bucket name must not be included in VPC endpoint URL",
},
{
desc: "aws s3 vpc endpoint without bucket prefix",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Data: map[string][]byte{
"endpoint": []byte("https://vpce-1234567-us-east-1c.s3.us-east-1.vpce.amazonaws.com"),
"region": []byte("us-east-1"),
"bucketnames": []byte("this,that"),
"access_key_id": []byte("id"),
"access_key_secret": []byte("secret"),
},
},
wantOptions: &storage.S3StorageConfig{
Endpoint: "https://vpce-1234567-us-east-1c.s3.us-east-1.vpce.amazonaws.com",
Region: "us-east-1",
Buckets: "this,that",
ForcePathStyle: false,
},
},
{
desc: "invalid forcepathstyle value",
secret: &corev1.Secret{
Expand Down
Loading