Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
38 changes: 30 additions & 8 deletions pkg/cmd/connection_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"os"
"strconv"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -158,16 +159,16 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
if f.RuleFilterBody != "" || f.RuleFilterHeaders != "" || f.RuleFilterQuery != "" || f.RuleFilterPath != "" {
rule := hookdeck.Rule{"type": "filter"}
if f.RuleFilterBody != "" {
rule["body"] = f.RuleFilterBody
rule["body"] = parseJSONOrString(f.RuleFilterBody)
}
Comment thread
leggetter marked this conversation as resolved.
if f.RuleFilterHeaders != "" {
rule["headers"] = f.RuleFilterHeaders
rule["headers"] = parseJSONOrString(f.RuleFilterHeaders)
}
if f.RuleFilterQuery != "" {
rule["query"] = f.RuleFilterQuery
rule["query"] = parseJSONOrString(f.RuleFilterQuery)
}
if f.RuleFilterPath != "" {
rule["path"] = f.RuleFilterPath
rule["path"] = parseJSONOrString(f.RuleFilterPath)
}
rules = append(rules, rule)
}
Expand All @@ -191,14 +192,35 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
rule["interval"] = f.RuleRetryInterval
}
if f.RuleRetryResponseStatusCode != "" {
codes := strings.Split(f.RuleRetryResponseStatusCode, ",")
for i := range codes {
codes[i] = strings.TrimSpace(codes[i])
parts := strings.Split(f.RuleRetryResponseStatusCode, ",")
intCodes := make([]int, 0, len(parts))
for _, part := range parts {
part = strings.TrimSpace(part)
if part == "" {
continue
}
n, err := strconv.Atoi(part)
if err != nil {
return nil, fmt.Errorf("invalid HTTP status code %q in --rule-retry-response-status-codes: must be an integer", part)
}
intCodes = append(intCodes, n)
}
Comment thread
leggetter marked this conversation as resolved.
rule["response_status_codes"] = codes
rule["response_status_codes"] = intCodes
}
rules = append(rules, rule)
}

return rules, nil
}

// parseJSONOrString attempts to parse s as JSON. If successful it returns the
// parsed value (object, array, number, bool, etc.); otherwise it returns s as
// a plain string. This lets filter flags accept both JSON objects and JQ
// expressions transparently.
func parseJSONOrString(s string) interface{} {
var v interface{}
if err := json.Unmarshal([]byte(s), &v); err == nil {
return v
}
Comment thread
leggetter marked this conversation as resolved.
return s
}
71 changes: 63 additions & 8 deletions pkg/cmd/connection_upsert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,69 @@ func strPtr(s string) *string {
return &s
}

// TestBuildConnectionRulesFilterHeadersJSON verifies that --rule-filter-headers
// parses JSON values into objects rather than storing them as escaped strings.
// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/192.
func TestBuildConnectionRulesFilterHeadersJSON(t *testing.T) {
t.Run("JSON object should be parsed, not stored as string", func(t *testing.T) {
flags := connectionRuleFlags{
RuleFilterHeaders: `{"x-shopify-topic":{"$startsWith":"order/"}}`,
}
rules, err := buildConnectionRules(&flags)
require.NoError(t, err)
require.Len(t, rules, 1)

filterRule := rules[0]
assert.Equal(t, "filter", filterRule["type"])

headers := filterRule["headers"]
_, isString := headers.(string)
assert.False(t, isString, "headers should be a parsed object, not a string")

headersMap, isMap := headers.(map[string]interface{})
assert.True(t, isMap, "headers should be map[string]interface{}, got %T", headers)
assert.Contains(t, headersMap, "x-shopify-topic")
Comment thread
leggetter marked this conversation as resolved.
Outdated
})

t.Run("non-JSON string (JQ expression) should remain a string", func(t *testing.T) {
flags := connectionRuleFlags{
RuleFilterHeaders: `.["x-topic"] == "order"`,
}
rules, err := buildConnectionRules(&flags)
require.NoError(t, err)
require.Len(t, rules, 1)

filterRule := rules[0]
headers := filterRule["headers"]
_, isString := headers.(string)
assert.True(t, isString, "non-JSON value should remain a string")
})

t.Run("filter body JSON should also be parsed", func(t *testing.T) {
flags := connectionRuleFlags{
RuleFilterBody: `{"event_type":"payment"}`,
}
rules, err := buildConnectionRules(&flags)
require.NoError(t, err)
require.Len(t, rules, 1)

filterRule := rules[0]
body := filterRule["body"]
_, isString := body.(string)
assert.False(t, isString, "body should be a parsed object, not a string")
_, isMap := body.(map[string]interface{})
assert.True(t, isMap, "body should be map[string]interface{}, got %T", body)
})
}

// TestBuildConnectionRulesRetryStatusCodesArray verifies that buildConnectionRules
// produces response_status_codes as a []string array, not a single string.
// produces response_status_codes as a []int array (HTTP status codes are integers).
// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3.
func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) {
tests := []struct {
name string
flags connectionRuleFlags
wantCodes []string
wantCodes []int
wantCodeCount int
wantRuleCount int
}{
Expand All @@ -32,7 +87,7 @@ func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) {
RuleRetryInterval: 5000,
RuleRetryResponseStatusCode: "500,502,503,504",
},
wantCodes: []string{"500", "502", "503", "504"},
wantCodes: []int{500, 502, 503, 504},
wantCodeCount: 4,
wantRuleCount: 1,
},
Expand All @@ -42,7 +97,7 @@ func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) {
RuleRetryStrategy: "exponential",
RuleRetryResponseStatusCode: "500",
},
wantCodes: []string{"500"},
wantCodes: []int{500},
wantCodeCount: 1,
wantRuleCount: 1,
},
Expand All @@ -52,7 +107,7 @@ func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) {
RuleRetryStrategy: "linear",
RuleRetryResponseStatusCode: "500, 502, 503",
},
wantCodes: []string{"500", "502", "503"},
wantCodes: []int{500, 502, 503},
wantCodeCount: 3,
wantRuleCount: 1,
},
Expand Down Expand Up @@ -90,12 +145,12 @@ func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) {
statusCodes, ok := retryRule["response_status_codes"]
require.True(t, ok, "response_status_codes should be present")

codesSlice, ok := statusCodes.([]string)
require.True(t, ok, "response_status_codes should be []string, got %T", statusCodes)
codesSlice, ok := statusCodes.([]int)
require.True(t, ok, "response_status_codes should be []int, got %T", statusCodes)
assert.Equal(t, tt.wantCodeCount, len(codesSlice))
assert.Equal(t, tt.wantCodes, codesSlice)

// Verify it serializes to a JSON array
// Verify it serializes to a JSON array of numbers
jsonBytes, err := json.Marshal(retryRule)
require.NoError(t, err)

Expand Down
131 changes: 131 additions & 0 deletions test/acceptance/connection_upsert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,135 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) {

t.Logf("Successfully upserted connection %s with source-name only", connID)
})

// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/192:
// --rule-filter-headers (and other filter flags) should store JSON as a parsed
// object, not as an escaped string.
t.Run("FilterHeadersJSONStoredAsObject", func(t *testing.T) {
Comment thread
leggetter marked this conversation as resolved.
Outdated
if testing.Short() {
t.Skip("Skipping acceptance test in short mode")
}

cli := NewCLIRunner(t)
timestamp := generateTimestamp()

connName := "test-filter-headers-" + timestamp
sourceName := "test-filter-src-" + timestamp
destName := "test-filter-dst-" + timestamp

// Create a connection using --rule-filter-headers with a JSON object
var createResp map[string]interface{}
err := cli.RunJSON(&createResp,
"gateway", "connection", "upsert", connName,
"--source-name", sourceName,
"--source-type", "WEBHOOK",
"--destination-name", destName,
"--destination-type", "HTTP",
"--destination-url", "https://example.com/webhook",
"--rule-filter-headers", `{"x-shopify-topic":{"$startsWith":"order/"}}`,
)
require.NoError(t, err, "Should create connection with --rule-filter-headers JSON")

connID, ok := createResp["id"].(string)
require.True(t, ok && connID != "", "Expected connection ID in response")

t.Cleanup(func() {
deleteConnection(t, cli, connID)
})

// Verify source and destination were created correctly
source, ok := createResp["source"].(map[string]interface{})
require.True(t, ok, "Expected source object in response")
assert.Equal(t, sourceName, source["name"], "Source name should match")

dest, ok := createResp["destination"].(map[string]interface{})
require.True(t, ok, "Expected destination object in response")
assert.Equal(t, destName, dest["name"], "Destination name should match")

// Verify the filter rule has headers as a JSON object, not an escaped string
rules, ok := createResp["rules"].([]interface{})
require.True(t, ok, "Expected rules array in response")

foundFilter := false
for _, r := range rules {
rule, ok := r.(map[string]interface{})
if !ok || rule["type"] != "filter" {
continue
}
foundFilter = true

headers := rule["headers"]
_, isString := headers.(string)
assert.False(t, isString,
"--rule-filter-headers should store JSON as an object, not an escaped string; got: %v", headers)

headersMap, isMap := headers.(map[string]interface{})
assert.True(t, isMap,
Comment thread
leggetter marked this conversation as resolved.
Outdated
"headers should be a JSON object (map[string]interface{}), got %T", headers)
assert.Contains(t, headersMap, "x-shopify-topic",
"headers object should contain the expected key")
break
}
assert.True(t, foundFilter, "Should have a filter rule")

t.Logf("Successfully verified --rule-filter-headers stores JSON as object for connection %s", connID)
})

// Verify that --rule-filter-body JSON is also stored as an object.
t.Run("FilterBodyJSONStoredAsObject", func(t *testing.T) {
if testing.Short() {
t.Skip("Skipping acceptance test in short mode")
}

cli := NewCLIRunner(t)
timestamp := generateTimestamp()

connName := "test-filter-body-" + timestamp
sourceName := "test-filter-body-src-" + timestamp
destName := "test-filter-body-dst-" + timestamp

var createResp map[string]interface{}
err := cli.RunJSON(&createResp,
"gateway", "connection", "upsert", connName,
"--source-name", sourceName,
"--source-type", "WEBHOOK",
"--destination-name", destName,
"--destination-type", "HTTP",
"--destination-url", "https://example.com/webhook",
"--rule-filter-body", `{"event_type":"payment"}`,
)
require.NoError(t, err, "Should create connection with --rule-filter-body JSON")

connID, ok := createResp["id"].(string)
require.True(t, ok && connID != "", "Expected connection ID in response")

t.Cleanup(func() {
deleteConnection(t, cli, connID)
})

rules, ok := createResp["rules"].([]interface{})
require.True(t, ok, "Expected rules array in response")

foundFilter := false
for _, r := range rules {
rule, ok := r.(map[string]interface{})
if !ok || rule["type"] != "filter" {
continue
}
foundFilter = true

body := rule["body"]
_, isString := body.(string)
assert.False(t, isString,
"--rule-filter-body should store JSON as an object, not an escaped string; got: %v", body)

bodyMap, isMap := body.(map[string]interface{})
assert.True(t, isMap, "body should be a JSON object, got %T", body)
Comment thread
leggetter marked this conversation as resolved.
Outdated
assert.Contains(t, bodyMap, "event_type", "body object should contain the expected key")
break
}
assert.True(t, foundFilter, "Should have a filter rule")

t.Logf("Successfully verified --rule-filter-body stores JSON as object for connection %s", connID)
})
}
Loading