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

fix(s3exporter/config-validation): endpoint should contain region and S3 bucket #32815

Merged
Merged
2 changes: 1 addition & 1 deletion exporter/awss3exporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ The following exporter configuration parameters are supported.
| `marshaler` | marshaler used to produce output data | `otlp_json` |
| `encoding` | Encoding extension to use to marshal data. Overrides the `marshaler` configuration option if set. | |
| `encoding_file_extension` | file format extension suffix when using the `encoding` configuration option. May be left empty for no suffix to be appended. | |
| `endpoint` | overrides the endpoint used by the exporter instead of constructing it from `region` and `s3_bucket` | |
| `endpoint` | (REST API endpoint) overrides the endpoint used by the exporter instead of constructing it from `region` and `s3_bucket` | |
| `s3_force_path_style` | [set this to `true` to force the request to use path-style addressing](http://docs.aws.amazon.com/AmazonS3/latest/dev/VirtualHosting.html) | false |
| `disable_ssl` | set this to `true` to disable SSL when sending requests | false |
| `compression` | should the file be compressed | none |
Expand Down
2 changes: 1 addition & 1 deletion exporter/awss3exporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (c *Config) Validate() error {
if c.S3Uploader.Region == "" {
errs = multierr.Append(errs, errors.New("region is required"))
}
if c.S3Uploader.S3Bucket == "" {
if c.S3Uploader.S3Bucket == "" && c.S3Uploader.Endpoint == "" {
errs = multierr.Append(errs, errors.New("bucket is required"))
}
compression := c.S3Uploader.Compression
Expand Down
37 changes: 30 additions & 7 deletions exporter/awss3exporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,24 @@ func TestConfig_Validate(t *testing.T) {
errExpected error
}{
{
name: "valid",
// endpoint overrides region and bucket name.
name: "valid with endpoint and region",
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Endpoint = "http://example.com"
c.S3Uploader.Region = "foo"
return c
}(),
errExpected: nil,
},
{
// Endpoint will be built from bucket and region.
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html
name: "valid with S3Bucket and region",
dabcoder marked this conversation as resolved.
Show resolved Hide resolved
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Region = "foo"
c.S3Uploader.S3Bucket = "bar"
c.S3Uploader.Endpoint = "http://example.com"
return c
}(),
errExpected: nil,
Expand All @@ -124,32 +136,43 @@ func TestConfig_Validate(t *testing.T) {
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Region = ""
c.S3Uploader.S3Bucket = ""
c.S3Uploader.Endpoint = ""
return c
}(),
errExpected: multierr.Append(errors.New("region is required"),
errors.New("bucket is required")),
errors.New("bucket or endpoint is required")),
},
{
name: "endpoint and region",
name: "region only",
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Endpoint = "http://example.com"
c.S3Uploader.Region = "foo"
c.S3Uploader.S3Bucket = ""
return c
}(),
errExpected: errors.New("bucket is required"),
},
{
name: "endpoint and bucket",
name: "bucket only",
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Endpoint = "http://example.com"
c.S3Uploader.S3Bucket = "foo"
c.S3Uploader.Region = ""
return c
}(),
errExpected: errors.New("region is required"),
},
{
name: "endpoint only",
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Endpoint = "http://example.com"
c.S3Uploader.Region = ""
return c
}(),
errExpected: errors.New("region is required"),
},
}

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