Skip to content

Commit

Permalink
fix(s3exporter/config-validation): draft: endpoint should contain reg…
Browse files Browse the repository at this point in the history
…ion and S3 bucket
  • Loading branch information
dabcoder committed May 2, 2024
1 parent 73cde8d commit 1386794
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
7 changes: 5 additions & 2 deletions exporter/awss3exporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,15 @@ type Config struct {

func (c *Config) Validate() error {
var errs error
if c.S3Uploader.Region == "" {
if c.S3Uploader.Region == "" && c.S3Uploader.Endpoint == "" {
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"))
}
if c.S3Uploader.Endpoint == "" && (c.S3Uploader.S3Bucket == "" && c.S3Uploader.Region == "") {
errs = multierr.Append(errs, errors.New("endpoint is required"))
}
compression := c.S3Uploader.Compression
if compression.IsCompressed() {
if compression != configcompression.TypeGzip {
Expand Down
26 changes: 18 additions & 8 deletions exporter/awss3exporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/otelcol/otelcoltest"
"go.uber.org/multierr"

"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awss3exporter/internal/metadata"
)
Expand Down Expand Up @@ -108,13 +107,24 @@ func TestConfig_Validate(t *testing.T) {
config *Config
errExpected error
}{
// Valid config with endpoint provided
{
// endpoint should contain region and bucket name
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html
name: "valid with endpoint only",
config: func() *Config {
c := createDefaultConfig().(*Config)
c.S3Uploader.Endpoint = "http://example.com"
return c
}(),
errExpected: nil,
},
{
name: "valid",
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,26 +134,26 @@ 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")),
errExpected: errors.New("endpoint should be provided, or region and bucket"),
},
{
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
Expand Down

0 comments on commit 1386794

Please sign in to comment.