Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions operator/internal/handlers/internal/storage/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"io"
"net/url"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -42,7 +43,9 @@ var (
errS3EndpointUnparseable = errors.New("can not parse S3 endpoint as URL")
errS3EndpointNoURL = errors.New("endpoint for S3 must be an HTTP or HTTPS URL")
errS3EndpointUnsupportedScheme = errors.New("scheme of S3 endpoint URL is unsupported")
errS3EndpointAWSInvalid = errors.New("endpoint for AWS S3 must include correct region")
errS3EndpointAWSNoRegion = errors.New("endpoint for AWS S3 must include correct region")
errS3EndpointNoBucketName = errors.New("bucket name must not be included in AWS S3 endpoint URL")
errS3EndpointAWSInvalid = errors.New("endpoint for AWS S3 is invalid, must match either https://s3.region.amazonaws.com or https://vpce-id.s3.region.vpce.amazonaws.com")
errS3ForcePathStyleInvalid = errors.New(`forcepathstyle must be "true" or "false"`)

errGCPParseCredentialsFile = errors.New("gcp storage secret cannot be parsed from JSON content")
Expand All @@ -62,6 +65,21 @@ const (
gcpAccountTypeExternal = "external_account"
)

var (
// Regular AWS S3 endpoint: https://s3.{region}.amazonaws.com
awsS3EndpointRegex = regexp.MustCompile(`^https://s3\.([a-z0-9-]+)\.amazonaws\.com$`)

// VPC endpoint: https://vpce-{id}.s3.{region}.vpce.amazonaws.com
awsVPCEndpointRegex = regexp.MustCompile(`^https://bucket.vpce-[a-z0-9-]+\.s3\.([a-z0-9-]+)\.vpce\.amazonaws\.com$`)

// Invalid patterns with bucket names (to detect and reject)
// Regular S3 with bucket: https://bucket-name.s3.region.amazonaws.com
awsS3WithBucketRegex = regexp.MustCompile(`^https://([a-z0-9.-]+)\.s3\.([a-z0-9-]+)\.amazonaws\.com$`)

// VPC with bucket: https://bucket-name.vpce-id.s3.region.vpce.amazonaws.com
awsVPCWithBucketRegex = regexp.MustCompile(`^https://([a-z0-9.-]+)\.bucket.vpce-[a-z0-9-]+\.s3\.([a-z0-9-]+)\.vpce\.amazonaws\.com$`)
)

func getSecrets(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg configv1.FeatureGates) (*corev1.Secret, *corev1.Secret, error) {
var (
storageSecret corev1.Secret
Expand Down Expand Up @@ -482,7 +500,7 @@ func extractS3ConfigSecret(s *corev1.Secret, credentialMode lokiv1.CredentialMod
}
}

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)
}
Expand All @@ -491,26 +509,43 @@ func validateS3Endpoint(endpoint string, region string) error {
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
return errS3EndpointNoURL
}

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) {
// AWS endpoint validation
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)
if awsS3WithBucketRegex.MatchString(endpoint) || awsVPCWithBucketRegex.MatchString(endpoint) {
return errS3EndpointNoBucketName
}

// Validate correct endpoint formats
if matches := awsS3EndpointRegex.FindStringSubmatch(endpoint); matches != nil {
if extractedRegion := matches[1]; extractedRegion != region {
return fmt.Errorf("%w: expected region %s, got %s", errS3EndpointAWSNoRegion, region, extractedRegion)
}
return nil
}

if matches := awsVPCEndpointRegex.FindStringSubmatch(endpoint); matches != nil {
if extractedRegion := matches[1]; extractedRegion != region {
return fmt.Errorf("%w: expected region %s, got %s", errS3EndpointAWSNoRegion, region, extractedRegion)
}
return nil
}

// If doesn't match any of the valid patterns, just return the error
return fmt.Errorf("%w: %s", errS3EndpointAWSInvalid, endpoint)
}

return nil
}

Expand Down
65 changes: 63 additions & 2 deletions operator/internal/handlers/internal/storage/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ func TestS3Extract(t *testing.T) {
"access_key_secret": []byte("secret"),
},
},
wantError: "endpoint for AWS S3 must include correct region: https://s3.region.amazonaws.com",
wantError: "endpoint for AWS S3 must include correct region: expected region region, got wrong",
},
{
name: "s3 endpoint format is not a valid s3 URL",
Expand All @@ -629,7 +629,49 @@ func TestS3Extract(t *testing.T) {
"access_key_secret": []byte("secret"),
},
},
wantError: "endpoint for AWS S3 must include correct region: https://s3.region.amazonaws.com",
wantError: "endpoint for AWS S3 is invalid, must match either https://s3.region.amazonaws.com or https://vpce-id.s3.region.vpce.amazonaws.com: http://region.amazonaws.com",
},
{
name: "valid aws s3 vpc endpoint",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Data: map[string][]byte{
"endpoint": []byte("https://vpce-1234567abc.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"),
},
},
wantCredentialMode: lokiv1.CredentialModeStatic,
},
{
name: "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-1234567abc.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 AWS S3 endpoint URL",
},
{
name: "aws s3 vpc endpoint wrong region",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Data: map[string][]byte{
"endpoint": []byte("https://vpce-1234567abc.s3.eu-east-2.vpce.amazonaws.com"),
"region": []byte("us-east-1"),
"bucketnames": []byte("this,that"),
"access_key_id": []byte("id"),
"access_key_secret": []byte("secret"),
},
},
wantError: "endpoint for AWS S3 must include correct region: expected region us-east-1, got eu-east-2",
},
}
for _, tst := range table {
Expand Down Expand Up @@ -734,6 +776,25 @@ func TestS3Extract_ForcePathStyle(t *testing.T) {
},
wantError: `forcepathstyle must be "true" or "false": yes`,
},
{
desc: "aws s3 vpc endpoint",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Data: map[string][]byte{
"endpoint": []byte("https://vpce-1234567abc.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-1234567abc.s3.us-east-1.vpce.amazonaws.com",
Region: "us-east-1",
Buckets: "this,that",
ForcePathStyle: false,
},
},
}

for _, tc := range tt {
Expand Down
Loading