Skip to content

Commit 224c616

Browse files
alexluongclaude
andauthored
fix: Treat empty custom_headers as absent (#741)
* fix: Treat empty custom_headers ({}) as absent instead of validation error Closes #709 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: Remove portal workaround for empty custom_headers No longer needed now that the backend treats {} as absent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 82abcc0 commit 224c616

8 files changed

Lines changed: 18 additions & 62 deletions

File tree

internal/destregistry/providers/destwebhook/destwebhook.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,12 +336,8 @@ func (d *WebhookDestination) resolveConfig(ctx context.Context, destination *mod
336336
}})
337337
}
338338
if len(config.CustomHeaders) == 0 {
339-
return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{
340-
Field: "config.custom_headers",
341-
Type: "invalid",
342-
}})
343-
}
344-
if err := ValidateCustomHeaders(config.CustomHeaders); err != nil {
339+
config.CustomHeaders = nil
340+
} else if err := ValidateCustomHeaders(config.CustomHeaders); err != nil {
345341
return nil, nil, err
346342
}
347343
}

internal/destregistry/providers/destwebhook/destwebhook_config_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) {
105105
assert.NoError(t, err)
106106
})
107107

108-
t.Run("should fail on empty custom_headers object", func(t *testing.T) {
108+
t.Run("should accept empty custom_headers object", func(t *testing.T) {
109109
t.Parallel()
110110
destination := testutil.DestinationFactory.Any(
111111
testutil.DestinationFactory.WithType("webhook"),
@@ -119,11 +119,7 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) {
119119
)
120120

121121
err := webhookDestination.Validate(context.Background(), &destination)
122-
assert.Error(t, err)
123-
var validationErr *destregistry.ErrDestinationValidation
124-
assert.ErrorAs(t, err, &validationErr)
125-
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
126-
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
122+
assert.NoError(t, err)
127123
})
128124

129125
t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) {

internal/destregistry/providers/destwebhook/destwebhook_publish_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"testing"
1313
"time"
1414

15-
"github.com/hookdeck/outpost/internal/destregistry"
1615
"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook"
1716
testsuite "github.com/hookdeck/outpost/internal/destregistry/testing"
1817
"github.com/hookdeck/outpost/internal/models"
@@ -568,7 +567,7 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) {
568567
assert.Equal(t, "delivery-metadata-value", req.Header.Get("x-outpost-source"))
569568
})
570569

571-
t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) {
570+
t.Run("should accept CreatePublisher when custom_headers is empty object", func(t *testing.T) {
572571
t.Parallel()
573572

574573
provider, err := destwebhook.New(testutil.Registry.MetadataLoader(), nil)
@@ -585,12 +584,9 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) {
585584
}),
586585
)
587586

588-
_, err = provider.CreatePublisher(context.Background(), &destination)
589-
require.Error(t, err)
590-
var validationErr *destregistry.ErrDestinationValidation
591-
assert.ErrorAs(t, err, &validationErr)
592-
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
593-
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
587+
publisher, err := provider.CreatePublisher(context.Background(), &destination)
588+
require.NoError(t, err)
589+
defer publisher.Close()
594590
})
595591

596592
t.Run("should work without custom_headers field", func(t *testing.T) {

internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,8 @@ func (d *StandardWebhookDestination) resolveConfig(ctx context.Context, destinat
264264
}})
265265
}
266266
if len(config.CustomHeaders) == 0 {
267-
return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{
268-
Field: "config.custom_headers",
269-
Type: "invalid",
270-
}})
271-
}
272-
if err := destwebhook.ValidateCustomHeaders(config.CustomHeaders); err != nil {
267+
config.CustomHeaders = nil
268+
} else if err := destwebhook.ValidateCustomHeaders(config.CustomHeaders); err != nil {
273269
return nil, nil, err
274270
}
275271
}

internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) {
3434
assert.NoError(t, err)
3535
})
3636

37-
t.Run("should fail on empty custom_headers object", func(t *testing.T) {
37+
t.Run("should accept empty custom_headers object", func(t *testing.T) {
3838
t.Parallel()
3939
destination := testutil.DestinationFactory.Any(
4040
testutil.DestinationFactory.WithType("webhook"),
@@ -48,11 +48,7 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) {
4848
)
4949

5050
err := provider.Validate(context.Background(), &destination)
51-
assert.Error(t, err)
52-
var validationErr *destregistry.ErrDestinationValidation
53-
assert.ErrorAs(t, err, &validationErr)
54-
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
55-
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
51+
assert.NoError(t, err)
5652
})
5753

5854
t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) {

internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"testing"
1313
"time"
1414

15-
"github.com/hookdeck/outpost/internal/destregistry"
1615
"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard"
1716
testsuite "github.com/hookdeck/outpost/internal/destregistry/testing"
1817
"github.com/hookdeck/outpost/internal/models"
@@ -523,7 +522,7 @@ func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) {
523522
}
524523
})
525524

526-
t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) {
525+
t.Run("should accept CreatePublisher when custom_headers is empty object", func(t *testing.T) {
527526
t.Parallel()
528527

529528
provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil)
@@ -540,12 +539,9 @@ func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) {
540539
}),
541540
)
542541

543-
_, err = provider.CreatePublisher(context.Background(), &dest)
544-
require.Error(t, err)
545-
var validationErr *destregistry.ErrDestinationValidation
546-
assert.ErrorAs(t, err, &validationErr)
547-
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
548-
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
542+
publisher, err := provider.CreatePublisher(context.Background(), &dest)
543+
require.NoError(t, err)
544+
defer publisher.Close()
549545
})
550546

551547
t.Run("should work without custom_headers field", func(t *testing.T) {

internal/portal/src/scenes/CreateDestination/CreateDestination.tsx

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -321,18 +321,7 @@ export default function CreateDestination() {
321321
(field) => field.key === key,
322322
),
323323
)
324-
.map(([key, value]) => {
325-
let configValue = String(value);
326-
// Webhook custom_headers must have at least one header or be null/empty
327-
if (
328-
destination_type?.type === "webhook" &&
329-
key === "custom_headers" &&
330-
(configValue === "{}" || configValue === "")
331-
) {
332-
configValue = "";
333-
}
334-
return [key, configValue];
335-
}),
324+
.map(([key, value]) => [key, String(value)]),
336325
),
337326
credentials: Object.fromEntries(
338327
Object.entries(values).filter(([key]) =>

internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,7 @@ const DestinationSettings = ({
120120
if (type.credential_fields.some((field) => field.key === key)) {
121121
credentials[key] = String(value);
122122
} else {
123-
let configValue = String(value);
124-
// Webhook custom_headers must have at least one header or be null/empty
125-
if (
126-
type.type === "webhook" &&
127-
key === "custom_headers" &&
128-
(configValue === "{}" || configValue === "")
129-
) {
130-
configValue = "";
131-
}
132-
config[key] = configValue;
123+
config[key] = String(value);
133124
}
134125
});
135126

0 commit comments

Comments
 (0)