Skip to content

Commit

Permalink
Added validation to avoid partial configuration for sigv4
Browse files Browse the repository at this point in the history
  • Loading branch information
obanby committed Oct 15, 2024
1 parent ecb96cc commit 72c8512
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 14 deletions.
18 changes: 18 additions & 0 deletions pkg/remotewrite/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package remotewrite
import (
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"github.com/grafana/xk6-output-prometheus-remote/pkg/sigv4"
"net/http"
Expand Down Expand Up @@ -123,6 +124,14 @@ func (conf Config) RemoteConfig() (*remote.HTTPConfig, error) {
hc.TLSConfig.Certificates = []tls.Certificate{cert}
}

if isSigV4PartiallyConfigured(conf.SigV4Region, conf.SigV4AccessKey, conf.SigV4SecretKey) {
return nil, errors.New(
"sigv4 seems to be partially configured. All of " +
"K6_PROMETHEUS_RW_SIGV4_REGION, K6_PROMETHEUS_RW_SIGV4_ACCESS_KEY, K6_PROMETHEUS_RW_SIGV4_SECRET_KEY " +
"must all be set. Unset all to bypass sigv4",
)
}

if conf.SigV4Region.Valid && conf.SigV4AccessKey.Valid && conf.SigV4SecretKey.Valid {
hc.SigV4 = &sigv4.Config{
Region: conf.SigV4Region.String,
Expand Down Expand Up @@ -429,3 +438,12 @@ func parseArg(text string) (Config, error) {

return c, nil
}

func isSigV4PartiallyConfigured(region, accessKey, secretKey null.String) bool {
hasRegion := region.Valid && len(strings.TrimSpace(region.String)) != 0
hasAccessID := accessKey.Valid && len(strings.TrimSpace(accessKey.String)) != 0
hasSecretAccessKey := secretKey.Valid && len(strings.TrimSpace(secretKey.String)) != 0
// either they are all set, or all not set. False if partial
isComplete := (hasRegion && hasAccessID && hasSecretAccessKey) || (!hasRegion && !hasAccessID && !hasSecretAccessKey)
return !isComplete
}
29 changes: 15 additions & 14 deletions pkg/sigv4/tripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,32 @@ type Tripper struct {

type Config struct {

Check warning on line 15 in pkg/sigv4/tripper.go

View workflow job for this annotation

GitHub Actions / checks / lint

exported: exported type Config should have comment or be unexported (revive)
Region string
AwsSecretAccessKey string
AwsAccessKeyID string
AwsSecretAccessKey string
}

func NewRoundTripper(config *Config, next http.RoundTripper) (*Tripper, error) {
if config == nil {
return nil, errors.New("can't initialize a sigv4 round tripper with nil config")
}

if len(strings.TrimSpace(config.Region)) == 0 {
return nil, errors.New("sigV4 config `Region` must be set")
func (c *Config) validate() error {
if c == nil {
return errors.New("config should not be nil")
}

if len(strings.TrimSpace(config.AwsSecretAccessKey)) == 0 {
return nil, errors.New("sigV4 config `AwsSecretAccessKey` must be set")
hasRegion := len(strings.TrimSpace(c.Region)) != 0
hasAccessID := len(strings.TrimSpace(c.AwsAccessKeyID)) != 0
hasSecretAccessKey := len(strings.TrimSpace(c.AwsSecretAccessKey)) != 0
if !hasRegion || !hasAccessID || !hasSecretAccessKey {
return errors.New("sigV4 config `Region`, `AwsAccessKeyID`, `AwsSecretAccessKey` must all be set")
}
return nil
}

if len(strings.TrimSpace(config.AwsAccessKeyID)) == 0 {
return nil, errors.New("sigV4 config `AwsAccessKeyID` must be set")
func NewRoundTripper(config *Config, next http.RoundTripper) (*Tripper, error) {

Check warning on line 34 in pkg/sigv4/tripper.go

View workflow job for this annotation

GitHub Actions / checks / lint

exported: exported function NewRoundTripper should have comment or be unexported (revive)
if err := config.validate(); err != nil {
return nil, err
}

if next == nil {
next = http.DefaultTransport
}

tripper := &Tripper{
config: config,
next: next,
Expand Down
49 changes: 49 additions & 0 deletions pkg/sigv4/tripper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,52 @@ func TestTripper_request_includes_required_headers(t *testing.T) {

client.Do(req)

Check failure on line 44 in pkg/sigv4/tripper_test.go

View workflow job for this annotation

GitHub Actions / checks / lint

Error return value of `client.Do` is not checked (errcheck)
}

func TestConfig_Validation(t *testing.T) {
testCases := []struct {
shouldError bool
arg *Config
}{
{
shouldError: false,
arg: &Config{
Region: "us-east1",
AwsAccessKeyID: "someAccessKey",
AwsSecretAccessKey: "someSecretKey",
},
},
{
shouldError: true,
arg: nil,
},
{
shouldError: true,
arg: &Config{
Region: "us-east1",
},
},
{
shouldError: true,
arg: &Config{
Region: "us-east1",
AwsAccessKeyID: "someAccessKeyId",
},
},
{
shouldError: true,
arg: &Config{
AwsAccessKeyID: "SomeAccessKey",
AwsSecretAccessKey: "SomeSecretKey",
},
},
}

for _, tc := range testCases {
got := tc.arg.validate()
if tc.shouldError {
assert.Error(t, got)
continue
}
assert.NoError(t, got)
}
}

0 comments on commit 72c8512

Please sign in to comment.