Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions docs/content/self-hosting/configuration.mdoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ Choose one message queue provider. The selected provider is used for both event

| Variable | Description |
|----------|-------------|
| `AWS_SQS_ACCESS_KEY_ID` | AWS Access Key ID |
| `AWS_SQS_SECRET_ACCESS_KEY` | AWS Secret Access Key |
| `AWS_SQS_ACCESS_KEY_ID` | AWS Access Key ID (optional; omit to use the AWS SDK default credential chain, e.g. an IAM role) |
| `AWS_SQS_SECRET_ACCESS_KEY` | AWS Secret Access Key (optional; omit to use the AWS SDK default credential chain, e.g. an IAM role) |
| `AWS_SQS_REGION` | AWS Region |

**GCP Pub/Sub:**
Expand Down
15 changes: 12 additions & 3 deletions internal/config/mqconfig_aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@ package config

import (
"context"
"errors"
"fmt"
"time"

"github.com/hookdeck/outpost/internal/mqinfra"
"github.com/hookdeck/outpost/internal/mqs"
)

// errPartialAWSSQSCredentials rejects a config with exactly one static key set.
var errPartialAWSSQSCredentials = errors.New("AWS SQS: both access_key_id and secret_access_key must be set together, or both omitted to use the default credential chain")

type AWSSQSConfig struct {
AccessKeyID string `yaml:"access_key_id" env:"AWS_SQS_ACCESS_KEY_ID" desc:"AWS Access Key ID for SQS. Required if AWS SQS is the chosen MQ provider." required:"C"`
SecretAccessKey string `yaml:"secret_access_key" env:"AWS_SQS_SECRET_ACCESS_KEY" desc:"AWS Secret Access Key for SQS. Required if AWS SQS is the chosen MQ provider." required:"C"`
AccessKeyID string `yaml:"access_key_id" env:"AWS_SQS_ACCESS_KEY_ID" desc:"AWS Access Key ID for SQS. Optional: omit (with the secret access key) to use the AWS SDK default credential chain, e.g. an IAM role." required:"N"`
SecretAccessKey string `yaml:"secret_access_key" env:"AWS_SQS_SECRET_ACCESS_KEY" desc:"AWS Secret Access Key for SQS. Optional: omit (with the access key ID) to use the AWS SDK default credential chain, e.g. an IAM role." required:"N"`
Region string `yaml:"region" env:"AWS_SQS_REGION" desc:"AWS Region for SQS. Required if AWS SQS is the chosen MQ provider." required:"C"`
Endpoint string `yaml:"endpoint" env:"AWS_SQS_ENDPOINT" desc:"Custom AWS SQS endpoint URL. Optional, typically used for local testing (e.g., LocalStack)." required:"N"`
DeliveryQueue string `yaml:"delivery_queue" env:"AWS_SQS_DELIVERY_QUEUE" desc:"Name of the SQS queue for delivery events." required:"N"`
Expand Down Expand Up @@ -45,6 +49,9 @@ func (c *AWSSQSConfig) ToInfraConfig(queueType string) *mqinfra.MQInfraConfig {
}

func (c *AWSSQSConfig) ToQueueConfig(ctx context.Context, queueType string) (*mqs.QueueConfig, error) {
if (c.AccessKeyID == "") != (c.SecretAccessKey == "") {
return nil, errPartialAWSSQSCredentials
}
return &mqs.QueueConfig{
AWSSQS: &mqs.AWSSQSConfig{
Endpoint: c.Endpoint,
Expand All @@ -60,6 +67,8 @@ func (c *AWSSQSConfig) GetProviderType() string {
return "awssqs"
}

// IsConfigured selects SQS on Region alone; keys are optional so IAM-role auth
// can use the AWS SDK default credential chain.
func (c *AWSSQSConfig) IsConfigured() bool {
return c.AccessKeyID != "" && c.SecretAccessKey != "" && c.Region != ""
return c.Region != ""
}
Comment on lines +70 to 74

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is correct, I discovered a config data issue where if only 1 of AWS_SQS_ACCESS_KEY_ID or AWS_SQS_SECRET_ACCESS_KEY is provided, it is accepted and led to a runtime auth error at a later stage even though this is invalid.

I don't think IsConfigured() is the right place but maybe we can apply this validation check at ToQueueConfig()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alexluong! addressed both points:

  • added the partial-credential guard in ToQueueConfig() as suggested so it fails at startup via validateMQs instead of deferring to a runtime error
  • extended IAM-role support to publishmq. The gap was in the config layer, since it already shared the IAM-ready SDK path. Relaxed the key requirementss, make selection region-only and applied the same partial-credentials check

90 changes: 90 additions & 0 deletions internal/config/mqconfig_aws_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package config_test

import (
"testing"

"github.com/hookdeck/outpost/internal/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestAWSSQSConfig_IsConfigured(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cfg config.AWSSQSConfig
want bool
}{
{
name: "region only (IAM role via default credential chain)",
cfg: config.AWSSQSConfig{Region: "us-east-1"},
want: true,
},
{
name: "region with static keys",
cfg: config.AWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET", Region: "us-east-1"},
want: true,
},
{
name: "keys without region",
cfg: config.AWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET"},
want: false,
},
{
name: "empty",
cfg: config.AWSSQSConfig{},
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, tt.cfg.IsConfigured())
})
}
}

func TestAWSSQSConfig_ToQueueConfig(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cfg config.AWSSQSConfig
wantErr bool
}{
{
name: "both keys set (static credentials)",
cfg: config.AWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET", Region: "us-east-1"},
},
{
name: "both keys empty (default credential chain)",
cfg: config.AWSSQSConfig{Region: "us-east-1"},
},
{
name: "only access key set",
cfg: config.AWSSQSConfig{AccessKeyID: "AKID", Region: "us-east-1"},
wantErr: true,
},
{
name: "only secret key set",
cfg: config.AWSSQSConfig{SecretAccessKey: "SECRET", Region: "us-east-1"},
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cfg, err := tt.cfg.ToQueueConfig(t.Context(), "deliverymq")
if tt.wantErr {
require.Error(t, err)
assert.Nil(t, cfg)
return
}
require.NoError(t, err)
require.NotNil(t, cfg)
})
}
}
19 changes: 15 additions & 4 deletions internal/config/publishmq.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
)

type PublishAWSSQSConfig struct {
AccessKeyID string `yaml:"access_key_id" env:"PUBLISH_AWS_SQS_ACCESS_KEY_ID" desc:"AWS Access Key ID for the SQS publish queue. Required if AWS SQS is the chosen publish MQ provider." required:"C"`
SecretAccessKey string `yaml:"secret_access_key" env:"PUBLISH_AWS_SQS_SECRET_ACCESS_KEY" desc:"AWS Secret Access Key for the SQS publish queue. Required if AWS SQS is the chosen publish MQ provider." required:"C"`
AccessKeyID string `yaml:"access_key_id" env:"PUBLISH_AWS_SQS_ACCESS_KEY_ID" desc:"AWS Access Key ID for the SQS publish queue. Optional: omit (with the secret access key) to use the AWS SDK default credential chain, e.g. an IAM role." required:"N"`
SecretAccessKey string `yaml:"secret_access_key" env:"PUBLISH_AWS_SQS_SECRET_ACCESS_KEY" desc:"AWS Secret Access Key for the SQS publish queue. Optional: omit (with the access key ID) to use the AWS SDK default credential chain, e.g. an IAM role." required:"N"`
Region string `yaml:"region" env:"PUBLISH_AWS_SQS_REGION" desc:"AWS Region for the SQS publish queue. Required if AWS SQS is the chosen publish MQ provider." required:"C"`
Endpoint string `yaml:"endpoint" env:"PUBLISH_AWS_SQS_ENDPOINT" desc:"Custom AWS SQS endpoint URL for the publish queue. Optional." required:"N"`
Queue string `yaml:"queue" env:"PUBLISH_AWS_SQS_QUEUE" desc:"Name of the SQS queue for publishing events. Required if AWS SQS is the chosen publish MQ provider." required:"C"`
Expand Down Expand Up @@ -99,9 +99,20 @@ func (c *PublishMQConfig) GetQueueConfig() *mqs.QueueConfig {
}
}

// Validate enforces the AWS SQS partial-credential rule for the selected provider.
func (c *PublishMQConfig) Validate() error {
if c.GetInfraType() == "awssqs" {
if (c.AWSSQS.AccessKeyID == "") != (c.AWSSQS.SecretAccessKey == "") {
return errPartialAWSSQSCredentials
}
}
return nil
}

// hasPublishAWSSQSConfig selects SQS on Region alone; keys are optional so
// IAM-role auth can use the AWS SDK default credential chain.
func hasPublishAWSSQSConfig(config PublishAWSSQSConfig) bool {
return config.AccessKeyID != "" &&
config.SecretAccessKey != "" && config.Region != ""
return config.Region != ""
}

func hasPublishAzureServiceBusConfig(config PublishAzureServiceBusConfig) bool {
Expand Down
93 changes: 93 additions & 0 deletions internal/config/publishmq_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package config_test

import (
"testing"

"github.com/hookdeck/outpost/internal/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestPublishMQConfig_GetInfraType_AWSSQS(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cfg config.PublishAWSSQSConfig
want string
}{
{
name: "region only (IAM role via default credential chain)",
cfg: config.PublishAWSSQSConfig{Region: "us-east-1"},
want: "awssqs",
},
{
name: "region with static keys",
cfg: config.PublishAWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET", Region: "us-east-1"},
want: "awssqs",
},
{
name: "keys without region",
cfg: config.PublishAWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET"},
want: "",
},
{
name: "empty",
cfg: config.PublishAWSSQSConfig{},
want: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cfg := config.PublishMQConfig{AWSSQS: tt.cfg}
assert.Equal(t, tt.want, cfg.GetInfraType())
})
}
}

func TestPublishMQConfig_Validate(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cfg config.PublishMQConfig
wantErr bool
}{
{
name: "aws sqs both keys set",
cfg: config.PublishMQConfig{AWSSQS: config.PublishAWSSQSConfig{AccessKeyID: "AKID", SecretAccessKey: "SECRET", Region: "us-east-1"}},
},
{
name: "aws sqs both keys empty (default credential chain)",
cfg: config.PublishMQConfig{AWSSQS: config.PublishAWSSQSConfig{Region: "us-east-1"}},
},
{
name: "aws sqs only access key set",
cfg: config.PublishMQConfig{AWSSQS: config.PublishAWSSQSConfig{AccessKeyID: "AKID", Region: "us-east-1"}},
wantErr: true,
},
{
name: "aws sqs only secret key set",
cfg: config.PublishMQConfig{AWSSQS: config.PublishAWSSQSConfig{SecretAccessKey: "SECRET", Region: "us-east-1"}},
wantErr: true,
},
{
name: "no publish provider configured",
cfg: config.PublishMQConfig{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := tt.cfg.Validate()
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}
12 changes: 12 additions & 0 deletions internal/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ func (c *Config) Validate(flags Flags) error {
return err
}

if err := c.validatePublishMQ(); err != nil {
return err
}

if err := c.validateAESEncryptionSecret(); err != nil {
return err
}
Expand Down Expand Up @@ -134,6 +138,14 @@ func (c *Config) validateMQs() error {
return nil
}

// validatePublishMQ validates the optional publish MQ configuration
func (c *Config) validatePublishMQ() error {
if err := c.PublishMQ.Validate(); err != nil {
return fmt.Errorf("failed to validate publish queue config: %w", err)
}
return nil
}

// validateAESEncryptionSecret validates the AES encryption secret
func (c *Config) validateAESEncryptionSecret() error {
if c.AESEncryptionSecret == "" {
Expand Down
91 changes: 91 additions & 0 deletions internal/mqinfra/mqinfra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,97 @@ func TestIntegrationMQInfra_AWSSQS(t *testing.T) {
)
}

// TestIntegrationMQInfra_AWSSQS_DefaultCredentialChain verifies that when no static
// ServiceAccountCredentials are configured, the AWS SDK default credential chain is
// used instead. Here the chain resolves credentials from environment variables (the
// same mechanism an ECS/EKS task role or EC2 instance profile relies on further down
// the chain). LocalStack accepts any credentials, so this proves the empty-credentials
// path builds a working client and can declare/publish/receive.
func TestIntegrationMQInfra_AWSSQS_DefaultCredentialChain(t *testing.T) {
testutil.CheckIntegrationTest(t)

// Provide credentials via the environment so the SDK default chain resolves them.
// t.Setenv is incompatible with t.Parallel, so this test runs serially.
t.Setenv("AWS_ACCESS_KEY_ID", "test")
t.Setenv("AWS_SECRET_ACCESS_KEY", "test")

endpoint := testinfra.EnsureLocalStack()
t.Cleanup(testinfra.Start(t))

q := idgen.String()
infraCfg := mqinfra.MQInfraConfig{
AWSSQS: &mqinfra.AWSSQSInfraConfig{
Endpoint: endpoint,
ServiceAccountCredentials: "", // no static creds → default credential chain
Region: "us-east-1",
Topic: q,
},
Policy: mqinfra.Policy{
RetryLimit: retryLimit,
VisibilityTimeout: 1,
},
}
mqCfg := mqs.QueueConfig{
AWSSQS: &mqs.AWSSQSConfig{
Endpoint: endpoint,
ServiceAccountCredentials: "", // no static creds → default credential chain
Region: "us-east-1",
Topic: q,
WaitTime: 1 * time.Second,
},
}

ctx := context.Background()
infra := mqinfra.New(&infraCfg)
require.NoError(t, infra.Declare(ctx))
t.Cleanup(func() {
require.NoError(t, infra.TearDown(ctx))
})

exists, err := infra.Exist(ctx)
require.NoError(t, err)
require.True(t, exists)

mq := mqs.NewQueue(&mqCfg)
cleanup, err := mq.Init(ctx)
require.NoError(t, err)
t.Cleanup(cleanup)

subscription, err := mq.Subscribe(ctx)
require.NoError(t, err)
t.Cleanup(func() {
subscription.Shutdown(ctx)
})

msgchan := make(chan *testutil.MockMsg)
go func() {
for {
msg, err := subscription.Receive(ctx)
if err != nil {
log.Println(err)
return
}
msg.Ack()
mockMsg := &testutil.MockMsg{}
if err := mockMsg.FromMessage(msg); err != nil {
log.Println("Error parsing message", err)
} else {
msgchan <- mockMsg
}
}
}()

msg := &testutil.MockMsg{ID: idgen.String()}
require.NoError(t, mq.Publish(ctx, msg))

select {
case receivedMsg := <-msgchan:
assert.Equal(t, msg.ID, receivedMsg.ID)
case <-time.After(1 * time.Second):
require.Fail(t, "timeout waiting for message")
}
}

func TestIntegrationMQInfra_GCPPubSub(t *testing.T) {
testutil.CheckIntegrationTest(t)
// Set PUBSUB_EMULATOR_HOST environment variable
Expand Down
Loading
Loading