Skip to content

Commit 73d98b3

Browse files
Shuang WangpuretensionJoaoBraveCoding
committed
fix(operator): Support private VPC S3 endpoints in endpoint validation
The operator's S3 endpoint validation was rejecting private VPC endpoints (https://vpce-{id}.s3.{region}.vpce.amazonaws.com) because it only accepted the standard format (https://s3.{region}.amazonaws.com). This updates validateS3Endpoint to use regex-based validation that: - Accepts standard AWS S3 endpoints with region verification - Accepts VPC endpoints (vpce-*) with region verification - Rejects bucket-prefixed endpoints to prevent folder creation issues - Provides specific error messages for each failure case Fixes #19243 Co-authored-by: puretension <rlrlfhtm5@gmail.com> Co-authored-by: JoaoBraveCoding <joao.coding@gmail.com>
1 parent ac76b40 commit 73d98b3

2 files changed

Lines changed: 102 additions & 10 deletions

File tree

‎operator/internal/handlers/internal/storage/secrets.go‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"io"
1212
"net/url"
13+
"regexp"
1314
"sort"
1415
"strings"
1516

@@ -43,8 +44,10 @@ var (
4344
errS3EndpointUnparseable = errors.New("can not parse S3 endpoint as URL")
4445
errS3EndpointNoURL = errors.New("endpoint for S3 must be an HTTP or HTTPS URL")
4546
errS3EndpointUnsupportedScheme = errors.New("scheme of S3 endpoint URL is unsupported")
47+
errS3EndpointAWSNoRegion = errors.New("endpoint for AWS S3 must include correct region")
48+
errS3EndpointNoBucketName = errors.New("bucket name must not be included in AWS S3 endpoint URL")
49+
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")
4650
errS3EndpointPathNotAllowed = errors.New("endpoint for S3 must not include a path")
47-
errS3EndpointAWSInvalid = errors.New("endpoint for AWS S3 must include correct region")
4851
errS3ForcePathStyleInvalid = errors.New(`forcepathstyle must be "true" or "false"`)
4952

5053
errGCPParseCredentialsFile = errors.New("gcp storage secret cannot be parsed from JSON content")
@@ -65,6 +68,21 @@ const (
6568
gcpAccountTypeExternal = "external_account"
6669
)
6770

71+
var (
72+
// Regular AWS S3 endpoint: https://s3.{region}.amazonaws.com
73+
awsS3EndpointRegex = regexp.MustCompile(`^https://s3\.([a-z0-9-]+)\.amazonaws\.com$`)
74+
75+
// VPC endpoint: https://vpce-{id}.s3.{region}.vpce.amazonaws.com
76+
awsVPCEndpointRegex = regexp.MustCompile(`^https://vpce-[a-z0-9-]+\.s3\.([a-z0-9-]+)\.vpce\.amazonaws\.com$`)
77+
78+
// Invalid patterns with bucket names (to detect and reject)
79+
// Regular S3 with bucket: https://bucket-name.s3.region.amazonaws.com
80+
awsS3WithBucketRegex = regexp.MustCompile(`^https://([a-z0-9.-]+)\.s3\.([a-z0-9-]+)\.amazonaws\.com$`)
81+
82+
// VPC with bucket: https://bucket-name.vpce-id.s3.region.vpce.amazonaws.com
83+
awsVPCWithBucketRegex = regexp.MustCompile(`^https://([a-z0-9.-]+)\.vpce-[a-z0-9-]+\.s3\.([a-z0-9-]+)\.vpce\.amazonaws\.com$`)
84+
)
85+
6886
func getSecrets(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg configv1.FeatureGates) (*corev1.Secret, *corev1.Secret, error) {
6987
var (
7088
storageSecret corev1.Secret
@@ -501,7 +519,7 @@ func extractS3ConfigSecret(s *corev1.Secret, credentialMode lokiv1.CredentialMod
501519
}
502520
}
503521

504-
func validateS3Endpoint(endpoint string, region string) error {
522+
func validateS3Endpoint(endpoint, region string) error {
505523
if len(endpoint) == 0 {
506524
return fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSEndpoint)
507525
}
@@ -510,12 +528,9 @@ func validateS3Endpoint(endpoint string, region string) error {
510528
if err != nil {
511529
return fmt.Errorf("%w: %w", errS3EndpointUnparseable, err)
512530
}
513-
514531
if parsedURL.Scheme == "" {
515-
// Assume "just a hostname" when scheme is empty and produce a clearer error message
516532
return errS3EndpointNoURL
517533
}
518-
519534
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
520535
return fmt.Errorf("%w: %s", errS3EndpointUnsupportedScheme, parsedURL.Scheme)
521536
}
@@ -529,11 +544,27 @@ func validateS3Endpoint(endpoint string, region string) error {
529544
return fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSRegion)
530545
}
531546

532-
validEndpoint := fmt.Sprintf("https://s3.%s%s", region, awsEndpointSuffix)
533-
if endpoint != validEndpoint {
534-
return fmt.Errorf("%w: %s", errS3EndpointAWSInvalid, validEndpoint)
547+
if awsS3WithBucketRegex.MatchString(endpoint) || awsVPCWithBucketRegex.MatchString(endpoint) {
548+
return errS3EndpointNoBucketName
535549
}
550+
551+
if matches := awsS3EndpointRegex.FindStringSubmatch(endpoint); matches != nil {
552+
if extractedRegion := matches[1]; extractedRegion != region {
553+
return fmt.Errorf("%w: expected region %s, got %s", errS3EndpointAWSNoRegion, region, extractedRegion)
554+
}
555+
return nil
556+
}
557+
558+
if matches := awsVPCEndpointRegex.FindStringSubmatch(endpoint); matches != nil {
559+
if extractedRegion := matches[1]; extractedRegion != region {
560+
return fmt.Errorf("%w: expected region %s, got %s", errS3EndpointAWSNoRegion, region, extractedRegion)
561+
}
562+
return nil
563+
}
564+
565+
return fmt.Errorf("%w: %s", errS3EndpointAWSInvalid, endpoint)
536566
}
567+
537568
return nil
538569
}
539570

‎operator/internal/handlers/internal/storage/secrets_test.go‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ func TestS3Extract(t *testing.T) {
678678
"access_key_secret": []byte("secret"),
679679
},
680680
},
681-
wantError: "endpoint for AWS S3 must include correct region: https://s3.region.amazonaws.com",
681+
wantError: "endpoint for AWS S3 must include correct region: expected region region, got wrong",
682682
},
683683
{
684684
name: "s3 endpoint format is not a valid s3 URL",
@@ -692,7 +692,49 @@ func TestS3Extract(t *testing.T) {
692692
"access_key_secret": []byte("secret"),
693693
},
694694
},
695-
wantError: "endpoint for AWS S3 must include correct region: https://s3.region.amazonaws.com",
695+
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",
696+
},
697+
{
698+
name: "valid aws s3 vpc endpoint",
699+
secret: &corev1.Secret{
700+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
701+
Data: map[string][]byte{
702+
"endpoint": []byte("https://vpce-1234567abc.s3.us-east-1.vpce.amazonaws.com"),
703+
"region": []byte("us-east-1"),
704+
"bucketnames": []byte("this,that"),
705+
"access_key_id": []byte("id"),
706+
"access_key_secret": []byte("secret"),
707+
},
708+
},
709+
wantCredentialMode: lokiv1.CredentialModeStatic,
710+
},
711+
{
712+
name: "aws s3 vpc endpoint with bucket name should fail",
713+
secret: &corev1.Secret{
714+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
715+
Data: map[string][]byte{
716+
"endpoint": []byte("https://bucket.vpce-1234567abc.s3.us-east-1.vpce.amazonaws.com"),
717+
"region": []byte("us-east-1"),
718+
"bucketnames": []byte("this,that"),
719+
"access_key_id": []byte("id"),
720+
"access_key_secret": []byte("secret"),
721+
},
722+
},
723+
wantError: "bucket name must not be included in AWS S3 endpoint URL",
724+
},
725+
{
726+
name: "aws s3 vpc endpoint wrong region",
727+
secret: &corev1.Secret{
728+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
729+
Data: map[string][]byte{
730+
"endpoint": []byte("https://vpce-1234567abc.s3.eu-east-2.vpce.amazonaws.com"),
731+
"region": []byte("us-east-1"),
732+
"bucketnames": []byte("this,that"),
733+
"access_key_id": []byte("id"),
734+
"access_key_secret": []byte("secret"),
735+
},
736+
},
737+
wantError: "endpoint for AWS S3 must include correct region: expected region us-east-1, got eu-east-2",
696738
},
697739
}
698740
for _, tst := range table {
@@ -798,6 +840,25 @@ func TestS3Extract_ForcePathStyle(t *testing.T) {
798840
},
799841
wantError: `forcepathstyle must be "true" or "false": yes`,
800842
},
843+
{
844+
desc: "aws s3 vpc endpoint",
845+
secret: &corev1.Secret{
846+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
847+
Data: map[string][]byte{
848+
"endpoint": []byte("https://vpce-1234567abc.s3.us-east-1.vpce.amazonaws.com"),
849+
"region": []byte("us-east-1"),
850+
"bucketnames": []byte("this,that"),
851+
"access_key_id": []byte("id"),
852+
"access_key_secret": []byte("secret"),
853+
},
854+
},
855+
wantOptions: &storage.S3StorageConfig{
856+
Endpoint: "vpce-1234567abc.s3.us-east-1.vpce.amazonaws.com",
857+
Region: "us-east-1",
858+
Buckets: "this,that",
859+
ForcePathStyle: false,
860+
},
861+
},
801862
}
802863

803864
for _, tc := range tt {

0 commit comments

Comments
 (0)