diff --git a/REFERENCE.md b/REFERENCE.md index 362c7082..d4f0f2a1 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -402,9 +402,9 @@ hookdeck gateway connection create [flags] | `--rule-retry-interval` | `int` | Interval between retries in milliseconds (default "0") | | `--rule-retry-response-status-codes` | `string` | Comma-separated HTTP status codes to retry on | | `--rule-retry-strategy` | `string` | Retry strategy (linear, exponential) | -| `--rule-transform-code` | `string` | Transformation code (if creating inline) | -| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation | -| `--rule-transform-name` | `string` | Name or ID of the transformation to apply | +| `--rule-transform-code` | `string` | Transformation code. Creates the transformation named by `--rule-transform-name`, or replaces its code if it exists | +| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation. Replaces the env of an existing transformation | +| `--rule-transform-name` | `string` | Name or ID of an existing transformation to apply. With `--rule-transform-code`, an existing transformation's code is replaced, or a new transformation is created with this name | | `--rules` | `string` | JSON string representing the entire rules array | | `--rules-file` | `string` | Path to a JSON file containing the rules array | | `--source-allowed-http-methods` | `string` | Comma-separated list of allowed HTTP methods (GET, POST, PUT, PATCH, DELETE) | @@ -521,9 +521,9 @@ hookdeck gateway connection update [flags] | `--rule-retry-interval` | `int` | Interval between retries in milliseconds (default "0") | | `--rule-retry-response-status-codes` | `string` | Comma-separated HTTP status codes to retry on | | `--rule-retry-strategy` | `string` | Retry strategy (linear, exponential) | -| `--rule-transform-code` | `string` | Transformation code (if creating inline) | -| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation | -| `--rule-transform-name` | `string` | Name or ID of the transformation to apply | +| `--rule-transform-code` | `string` | Transformation code. Creates the transformation named by `--rule-transform-name`, or replaces its code if it exists | +| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation. Replaces the env of an existing transformation | +| `--rule-transform-name` | `string` | Name or ID of an existing transformation to apply. With `--rule-transform-code`, an existing transformation's code is replaced, or a new transformation is created with this name | | `--rules` | `string` | JSON string representing the entire rules array | | `--rules-file` | `string` | Path to a JSON file containing the rules array | | `--source-id` | `string` | Update source by ID | @@ -661,9 +661,9 @@ hookdeck gateway connection upsert [flags] | `--rule-retry-interval` | `int` | Interval between retries in milliseconds (default "0") | | `--rule-retry-response-status-codes` | `string` | Comma-separated HTTP status codes to retry on | | `--rule-retry-strategy` | `string` | Retry strategy (linear, exponential) | -| `--rule-transform-code` | `string` | Transformation code (if creating inline) | -| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation | -| `--rule-transform-name` | `string` | Name or ID of the transformation to apply | +| `--rule-transform-code` | `string` | Transformation code. Creates the transformation named by `--rule-transform-name`, or replaces its code if it exists | +| `--rule-transform-env` | `string` | JSON string representing environment variables for transformation. Replaces the env of an existing transformation | +| `--rule-transform-name` | `string` | Name or ID of an existing transformation to apply. With `--rule-transform-code`, an existing transformation's code is replaced, or a new transformation is created with this name | | `--rules` | `string` | JSON string representing the entire rules array | | `--rules-file` | `string` | Path to a JSON file containing the rules array | | `--source-allowed-http-methods` | `string` | Comma-separated list of allowed HTTP methods (GET, POST, PUT, PATCH, DELETE) | diff --git a/pkg/cmd/connection_common.go b/pkg/cmd/connection_common.go index 0029d16b..9af86284 100644 --- a/pkg/cmd/connection_common.go +++ b/pkg/cmd/connection_common.go @@ -82,9 +82,9 @@ func addConnectionRuleFlags(cmd *cobra.Command, f *connectionRuleFlags) { cmd.Flags().StringVar(&f.RuleFilterQuery, "rule-filter-query", "", "Filter on request query parameters using Hookdeck filter syntax (JSON)") cmd.Flags().StringVar(&f.RuleFilterPath, "rule-filter-path", "", "Filter on request path using Hookdeck filter syntax (JSON)") - cmd.Flags().StringVar(&f.RuleTransformName, "rule-transform-name", "", "Name or ID of the transformation to apply") - cmd.Flags().StringVar(&f.RuleTransformCode, "rule-transform-code", "", "Transformation code (if creating inline)") - cmd.Flags().StringVar(&f.RuleTransformEnv, "rule-transform-env", "", "JSON string representing environment variables for transformation") + cmd.Flags().StringVar(&f.RuleTransformName, "rule-transform-name", "", "Name or ID of an existing transformation to apply. With --rule-transform-code, an existing transformation's code is replaced, or a new transformation is created with this name") + cmd.Flags().StringVar(&f.RuleTransformCode, "rule-transform-code", "", "Transformation code. Creates the transformation named by --rule-transform-name, or replaces its code if it exists") + cmd.Flags().StringVar(&f.RuleTransformEnv, "rule-transform-env", "", "JSON string representing environment variables for transformation. Replaces the env of an existing transformation") cmd.Flags().IntVar(&f.RuleDelay, "rule-delay", 0, "Delay in milliseconds") diff --git a/pkg/cmd/connection_create.go b/pkg/cmd/connection_create.go index a20bb7f0..8485fd1c 100644 --- a/pkg/cmd/connection_create.go +++ b/pkg/cmd/connection_create.go @@ -459,6 +459,9 @@ func (cc *connectionCreateCmd) runConnectionCreateCmd(cmd *cobra.Command, args [ if err != nil { return err } + if err := resolveRuleTransformation(context.Background(), client, &cc.connectionRuleFlags, rules); err != nil { + return err + } if len(rules) > 0 { req.Rules = &rules } diff --git a/pkg/cmd/connection_transform_rule.go b/pkg/cmd/connection_transform_rule.go new file mode 100644 index 00000000..34f23d7d --- /dev/null +++ b/pkg/cmd/connection_transform_rule.go @@ -0,0 +1,110 @@ +package cmd + +import ( + "context" + "fmt" + "strings" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +) + +// transformationLookup is the subset of the API client used to resolve +// --rule-transform-name. +type transformationLookup interface { + GetTransformation(ctx context.Context, id string) (*hookdeck.Transformation, error) + ListTransformations(ctx context.Context, params map[string]string) (*hookdeck.TransformationListResponse, error) +} + +// findTransformationID returns the ID of the transformation whose ID or exact +// name is nameOrID, or "" when there is none. +func findTransformationID(ctx context.Context, client transformationLookup, nameOrID string) (string, error) { + if strings.HasPrefix(nameOrID, "trs_") { + trn, err := client.GetTransformation(ctx, nameOrID) + if err == nil { + return trn.ID, nil + } + if !hookdeck.IsNotFoundError(err) { + return "", fmt.Errorf("failed to look up transformation '%s': %w", nameOrID, err) + } + } + + result, err := client.ListTransformations(ctx, map[string]string{"name": nameOrID}) + if err != nil { + return "", fmt.Errorf("failed to look up transformation '%s': %w", nameOrID, err) + } + for _, trn := range result.Models { + if trn.Name == nameOrID { + return trn.ID, nil + } + } + return "", nil +} + +// resolveRuleTransformation makes --rule-transform-name accept a transformation +// name or ID, as its help text says. +// +// The API matches a transform rule's transformation by name and creates one when +// the name is new, even without code. Sent as a name, an ID or a mistyped name +// therefore created an empty transformation that failed every event. So the value +// is resolved first: +// - found: the rule references it by transformation_id; --rule-transform-code and +// --rule-transform-env, if given, update that transformation +// - not found, with --rule-transform-code: a new transformation is created with +// that name, as before (not allowed for an ID) +// - not found, without --rule-transform-code: an error +// +// Rules from --rules or --rules-file are sent as given. +func resolveRuleTransformation(ctx context.Context, client transformationLookup, f *connectionRuleFlags, rules []hookdeck.Rule) error { + if f.Rules != "" || f.RulesFile != "" { + return nil + } + if f.RuleTransformName == "" { + if f.RuleTransformCode != "" || f.RuleTransformEnv != "" { + return fmt.Errorf("--rule-transform-name is required when using transform rule flags") + } + return nil + } + + idx := -1 + for i, r := range rules { + if r["type"] == "transform" { + idx = i + break + } + } + if idx == -1 { + return nil + } + + nameOrID := f.RuleTransformName + id, err := findTransformationID(ctx, client, nameOrID) + if err != nil { + return err + } + + if id == "" { + if strings.HasPrefix(nameOrID, "trs_") { + return fmt.Errorf("transformation '%s' not found. Check the ID with 'hookdeck gateway transformation list'", nameOrID) + } + if f.RuleTransformCode == "" { + return fmt.Errorf("transformation '%s' not found. Check the name with 'hookdeck gateway transformation list', or pass --rule-transform-code to create a new transformation with this name", nameOrID) + } + // New transformation: keep name and code so the API creates it + return nil + } + + rule := hookdeck.Rule{"type": "transform", "transformation_id": id} + if existing, ok := rules[idx]["transformation"].(map[string]interface{}); ok { + update := make(map[string]interface{}) + for _, key := range []string{"code", "env"} { + if v, ok := existing[key]; ok { + update[key] = v + } + } + if len(update) > 0 { + rule["transformation"] = update + } + } + rules[idx] = rule + return nil +} diff --git a/pkg/cmd/connection_transform_rule_test.go b/pkg/cmd/connection_transform_rule_test.go new file mode 100644 index 00000000..b7b8f3bf --- /dev/null +++ b/pkg/cmd/connection_transform_rule_test.go @@ -0,0 +1,145 @@ +package cmd + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +) + +// fakeTransformations serves GetTransformation by ID and ListTransformations by +// name the way the API does: list may return near matches, so callers must +// compare names exactly. +type fakeTransformations struct { + byID map[string]hookdeck.Transformation + listErr error + calls int +} + +func (f *fakeTransformations) GetTransformation(_ context.Context, id string) (*hookdeck.Transformation, error) { + f.calls++ + if trn, ok := f.byID[id]; ok { + return &trn, nil + } + return nil, &hookdeck.APIError{StatusCode: 404, Message: "not found"} +} + +func (f *fakeTransformations) ListTransformations(_ context.Context, params map[string]string) (*hookdeck.TransformationListResponse, error) { + f.calls++ + if f.listErr != nil { + return nil, f.listErr + } + resp := &hookdeck.TransformationListResponse{} + for _, trn := range f.byID { + if len(params["name"]) > 0 && len(trn.Name) >= len(params["name"]) && trn.Name[:len(params["name"])] == params["name"] { + resp.Models = append(resp.Models, trn) + } + } + return resp, nil +} + +func newFakeTransformations() *fakeTransformations { + return &fakeTransformations{byID: map[string]hookdeck.Transformation{ + "trs_abc123": {ID: "trs_abc123", Name: "orders-transform"}, + }} +} + +func buildTransformRules(t *testing.T, f *connectionRuleFlags) []hookdeck.Rule { + t.Helper() + rules, err := buildConnectionRules(f) + require.NoError(t, err) + return rules +} + +func TestResolveRuleTransformationAttachesExisting(t *testing.T) { + for _, nameOrID := range []string{"trs_abc123", "orders-transform"} { + t.Run(nameOrID, func(t *testing.T) { + f := &connectionRuleFlags{RuleTransformName: nameOrID, RuleFilterBody: `{"type":"order"}`} + rules := buildTransformRules(t, f) + + require.NoError(t, resolveRuleTransformation(context.Background(), newFakeTransformations(), f, rules)) + + assert.Equal(t, hookdeck.Rule{"type": "transform", "transformation_id": "trs_abc123"}, rules[0], + "an existing transformation is referenced by ID, never sent as a name") + assert.Equal(t, "filter", rules[1]["type"], "other rules are untouched") + }) + } +} + +func TestResolveRuleTransformationUpdatesExistingWithCodeAndEnv(t *testing.T) { + f := &connectionRuleFlags{ + RuleTransformName: "trs_abc123", + RuleTransformCode: `addHandler("transform", (request) => request);`, + RuleTransformEnv: `{"KEY":"value"}`, + } + rules := buildTransformRules(t, f) + + require.NoError(t, resolveRuleTransformation(context.Background(), newFakeTransformations(), f, rules)) + + assert.Equal(t, "trs_abc123", rules[0]["transformation_id"]) + update, ok := rules[0]["transformation"].(map[string]interface{}) + require.True(t, ok, "code and env are sent as an update to the existing transformation") + assert.Equal(t, f.RuleTransformCode, update["code"]) + assert.Equal(t, map[string]interface{}{"KEY": "value"}, update["env"]) + assert.NotContains(t, update, "name", "the ID must not be sent as the transformation's new name") +} + +func TestResolveRuleTransformationNotFound(t *testing.T) { + t.Run("name without code is an error", func(t *testing.T) { + // "orders" is a prefix of an existing name, so only an exact match may count + f := &connectionRuleFlags{RuleTransformName: "orders"} + err := resolveRuleTransformation(context.Background(), newFakeTransformations(), f, buildTransformRules(t, f)) + require.Error(t, err) + assert.Contains(t, err.Error(), "transformation 'orders' not found") + assert.Contains(t, err.Error(), "--rule-transform-code") + }) + + t.Run("ID is an error, even with code", func(t *testing.T) { + f := &connectionRuleFlags{RuleTransformName: "trs_missing", RuleTransformCode: "x"} + err := resolveRuleTransformation(context.Background(), newFakeTransformations(), f, buildTransformRules(t, f)) + require.Error(t, err) + assert.Contains(t, err.Error(), "transformation 'trs_missing' not found") + }) + + t.Run("name with code creates a new transformation", func(t *testing.T) { + f := &connectionRuleFlags{RuleTransformName: "new-transform", RuleTransformCode: "x"} + rules := buildTransformRules(t, f) + require.NoError(t, resolveRuleTransformation(context.Background(), newFakeTransformations(), f, rules)) + assert.Equal(t, map[string]interface{}{"name": "new-transform", "code": "x"}, rules[0]["transformation"]) + assert.NotContains(t, rules[0], "transformation_id") + }) +} + +func TestResolveRuleTransformationRequiresName(t *testing.T) { + f := &connectionRuleFlags{RuleTransformCode: "x"} + err := resolveRuleTransformation(context.Background(), newFakeTransformations(), f, buildTransformRules(t, f)) + require.Error(t, err) + assert.Contains(t, err.Error(), "--rule-transform-name is required") +} + +func TestResolveRuleTransformationLeavesRulesJSONAlone(t *testing.T) { + f := &connectionRuleFlags{ + Rules: `[{"type":"transform","transformation":{"name":"trs_abc123"}}]`, + RuleTransformName: "ignored", + } + rules := buildTransformRules(t, f) + fake := newFakeTransformations() + + require.NoError(t, resolveRuleTransformation(context.Background(), fake, f, rules)) + assert.Equal(t, map[string]interface{}{"name": "trs_abc123"}, rules[0]["transformation"]) + assert.Zero(t, fake.calls, "--rules is sent as given, without lookups") +} + +func TestResolveRuleTransformationReturnsLookupErrors(t *testing.T) { + fake := newFakeTransformations() + fake.listErr = errors.New("boom") + f := &connectionRuleFlags{RuleTransformName: "orders-transform"} + + err := resolveRuleTransformation(context.Background(), fake, f, buildTransformRules(t, f)) + require.Error(t, err) + assert.Contains(t, err.Error(), "boom") +} diff --git a/pkg/cmd/connection_update.go b/pkg/cmd/connection_update.go index 8728d460..5c1534c7 100644 --- a/pkg/cmd/connection_update.go +++ b/pkg/cmd/connection_update.go @@ -118,6 +118,9 @@ func (cu *connectionUpdateCmd) runConnectionUpdateCmd(cmd *cobra.Command, args [ if err != nil { return err } + if err := resolveRuleTransformation(ctx, client, &cu.connectionRuleFlags, rules); err != nil { + return err + } // Keyed off whether the flag was given rather than whether it produced any // rules, so that an explicit empty array reaches the API and clears the // ruleset instead of being read as "no rules mentioned". diff --git a/pkg/cmd/connection_upsert.go b/pkg/cmd/connection_upsert.go index 84e3c388..04b484fb 100644 --- a/pkg/cmd/connection_upsert.go +++ b/pkg/cmd/connection_upsert.go @@ -373,6 +373,11 @@ func (cu *connectionUpsertCmd) runConnectionUpsertCmd(cmd *cobra.Command, args [ if err != nil { return err } + if req.Rules != nil { + if err := resolveRuleTransformation(context.Background(), client, &cu.connectionCreateCmd.connectionRuleFlags, *req.Rules); err != nil { + return err + } + } // For dry-run mode, preview changes without applying if cu.dryRun { diff --git a/test/acceptance/connection_test.go b/test/acceptance/connection_test.go index 8f6a4ea4..b45b9c89 100644 --- a/test/acceptance/connection_test.go +++ b/test/acceptance/connection_test.go @@ -1391,6 +1391,68 @@ func TestConnectionWithTransformRule(t *testing.T) { t.Logf("Successfully created and verified connection with transform rule: %s", conn.ID) } +// TestConnectionTransformRuleNameOrID verifies that --rule-transform-name attaches an +// existing transformation by ID or by name, and that an unknown value without +// --rule-transform-code is an error instead of an empty transformation being created. +func TestConnectionTransformRuleNameOrID(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + trnID := createTestTransformation(t, cli) + t.Cleanup(func() { deleteTransformation(t, cli, trnID) }) + + var trn Transformation + require.NoError(t, cli.RunJSON(&trn, "gateway", "transformation", "get", trnID)) + require.NotEmpty(t, trn.Name, "transformation name") + + createArgs := func(suffix string) []string { + timestamp := generateTimestamp() + return []string{ + "gateway", "connection", "create", + "--name", "test-trn-ref-" + suffix + "-" + timestamp, + "--source-name", "test-trn-ref-src-" + timestamp, + "--source-type", "WEBHOOK", + "--destination-name", "test-trn-ref-dst-" + timestamp, + "--destination-type", "CLI", + "--destination-cli-path", "/webhooks", + } + } + + for _, tc := range []struct{ name, value string }{{"by ID", trnID}, {"by name", trn.Name}} { + t.Run("attaches existing "+tc.name, func(t *testing.T) { + var conn Connection + err := cli.RunJSON(&conn, append(createArgs("ok"), "--rule-transform-name", tc.value)...) + require.NoError(t, err, "Should create connection referencing an existing transformation") + t.Cleanup(func() { deleteConnection(t, cli, conn.ID) }) + + var getConn Connection + require.NoError(t, cli.RunJSON(&getConn, "gateway", "connection", "get", conn.ID)) + require.Len(t, getConn.Rules, 1, "Connection should have one rule") + assert.Equal(t, trnID, getConn.Rules[0]["transformation_id"], + "Rule should reference the existing transformation, not a new one") + }) + } + + for _, value := range []string{"trs_doesnotexist0", "test-trn-missing-" + generateTimestamp()} { + t.Run("rejects unknown "+value, func(t *testing.T) { + stdout, stderr, err := cli.Run(append(createArgs("missing"), "--rule-transform-name", value)...) + require.Error(t, err, "Unknown transformation without code should fail\nstdout: %s", stdout) + assert.Contains(t, stderr+stdout, "transformation '"+value+"' not found") + + // Nothing should have been created with that name + var list struct { + Models []Transformation `json:"models"` + } + require.NoError(t, cli.RunJSON(&list, "gateway", "transformation", "list", "--name", value)) + for _, m := range list.Models { + assert.NotEqual(t, value, m.Name, "No transformation should be created for an unknown value") + } + }) + } +} + // TestConnectionWithDelayRule tests creating a connection with a delay rule func TestConnectionWithDelayRule(t *testing.T) { if testing.Short() {