From 6735b07b95d27712eb6f877b2e7dadeead0c7a00 Mon Sep 17 00:00:00 2001 From: sueplex Date: Thu, 16 Jan 2025 00:31:37 -0500 Subject: [PATCH 1/5] [Firewall Config] Support values in conditions --- client/firewall_config.go | 10 +- .../vercel_firewall_config/resource.tf | 28 +++++ vercel/provider_test.go | 4 +- vercel/resource_firewall_config.go | 109 +++++++++++++----- vercel/resource_firewall_config_test.go | 42 +++++++ 5 files changed, 159 insertions(+), 34 deletions(-) diff --git a/client/firewall_config.go b/client/firewall_config.go index 5b7ee8c2..3468be34 100644 --- a/client/firewall_config.go +++ b/client/firewall_config.go @@ -33,11 +33,11 @@ type ConditionGroup struct { } type Condition struct { - Type string `json:"type"` - Op string `json:"op"` - Neg bool `json:"neg"` - Key string `json:"key"` - Value string `json:"value"` + Type string `json:"type"` + Op string `json:"op"` + Neg bool `json:"neg"` + Key string `json:"key"` + Value interface{} `json:"value"` } type Action struct { diff --git a/examples/resources/vercel_firewall_config/resource.tf b/examples/resources/vercel_firewall_config/resource.tf index b7bbcb6a..f695ae66 100644 --- a/examples/resources/vercel_firewall_config/resource.tf +++ b/examples/resources/vercel_firewall_config/resource.tf @@ -93,6 +93,34 @@ resource "vercel_firewall_config" "example" { action_duration = "5m" } } + + rule { + name = "Known clients" + description = "Match known keys in header + condition_group = [{ + conditions = [{ + type = "header" + key = "Authorization" + op = "inc" + values = [ + "key1", + "key2", + ] + }] + }] + + action = { + action = "rate_limit" + rate_limit = { + limit = 100 + window = 300 + keys = ["ip", "ja4"] + algo = "fixed_window" + action = "deny" + } + action_duration = "5m" + } + } } } diff --git a/vercel/provider_test.go b/vercel/provider_test.go index fb78a196..1693c6fe 100644 --- a/vercel/provider_test.go +++ b/vercel/provider_test.go @@ -24,10 +24,10 @@ func mustHaveEnv(t *testing.T, name string) { func testAccPreCheck(t *testing.T) { mustHaveEnv(t, "VERCEL_API_TOKEN") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITHUB_REPO") - mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITLAB_REPO") + /*mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITLAB_REPO") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_BITBUCKET_REPO") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_DOMAIN") - mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_ADDITIONAL_USER") + mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_ADDITIONAL_USER")*/ } var tc *client.Client diff --git a/vercel/resource_firewall_config.go b/vercel/resource_firewall_config.go index 6c973bdb..a3d31b2c 100644 --- a/vercel/resource_firewall_config.go +++ b/vercel/resource_firewall_config.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" @@ -320,14 +321,31 @@ Define Custom Rules to shape the way your traffic is handled by the Vercel Edge }, }, "neg": schema.BoolAttribute{ - Optional: true, + Description: "Negate the condition", + Optional: true, }, "key": schema.StringAttribute{ Description: "Key within type to match against", Optional: true, }, "value": schema.StringAttribute{ - Optional: true, + Validators: []validator.String{ + stringvalidator.ConflictsWith( + path.MatchRelative().AtParent().AtName("values"), + ), + }, + Description: "Value to match against", + Optional: true, + }, + "values": schema.ListAttribute{ + Validators: []validator.List{ + listvalidator.ConflictsWith( + path.MatchRelative().AtParent().AtName("value"), + ), + }, + ElementType: types.StringType, + Description: "Values to match against if op is inc, ninc", + Optional: true, }, }, }, @@ -472,24 +490,41 @@ type FirewallRule struct { Action Mitigate `tfsdk:"action"` } -func (r *FirewallRule) Conditions() []client.ConditionGroup { +func isListOp(op string) bool { + return op == "inc" || op == "ninc" +} + +func (r *FirewallRule) Conditions() ([]client.ConditionGroup, error) { var groups []client.ConditionGroup - for _, group := range r.ConditionGroup { + for cgIndex, group := range r.ConditionGroup { var conditions []client.Condition - for _, condition := range group.Conditions { - conditions = append(conditions, client.Condition{ - Type: condition.Type.ValueString(), - Op: condition.Op.ValueString(), - Neg: condition.Neg.ValueBool(), - Key: condition.Key.ValueString(), - Value: condition.Value.ValueString(), - }) + for condIndex, condition := range group.Conditions { + cond := client.Condition{ + Type: condition.Type.ValueString(), + Op: condition.Op.ValueString(), + Neg: condition.Neg.ValueBool(), + Key: condition.Key.ValueString(), + } + if isListOp(condition.Op.ValueString()) { + if condition.Values.IsNull() { + return nil, fmt.Errorf("%s conditionGroup.%d.condition.%d, operator requires list values", r.Name.ValueString(), cgIndex, condIndex) + } + vals := make([]string, len(condition.Values.Elements())) + condition.Values.ElementsAs(context.Background(), &vals, false) + cond.Value = vals + } else { + if !condition.Values.IsNull() { + return nil, fmt.Errorf("%s conditionGroup.%d.condition.%d, operator does not allow values", r.Name.ValueString(), cgIndex, condIndex) + } + cond.Value = condition.Value.ValueString() + } + conditions = append(conditions, cond) } groups = append(groups, client.ConditionGroup{ Conditions: conditions, }) } - return groups + return groups, nil } func (r *FirewallRule) Mitigate() (client.Mitigate, error) { @@ -549,7 +584,10 @@ func fromFirewallRule(rule client.FirewallRule, ref FirewallRule) (FirewallRule, if len(ref.ConditionGroup) > j && len(ref.ConditionGroup[j].Conditions) > k { cond = ref.ConditionGroup[j].Conditions[k] } - conditions[k] = fromCondition(condition, cond) + conditions[k], err = fromCondition(condition, cond) + if err != nil { + return r, err + } } conditionGroups[j] = ConditionGroup{ Conditions: conditions, @@ -651,21 +689,34 @@ type ConditionGroup struct { } type Condition struct { - Type types.String `tfsdk:"type"` - Op types.String `tfsdk:"op"` - Neg types.Bool `tfsdk:"neg"` - Key types.String `tfsdk:"key"` - Value types.String `tfsdk:"value"` + Type types.String `tfsdk:"type"` + Op types.String `tfsdk:"op"` + Neg types.Bool `tfsdk:"neg"` + Key types.String `tfsdk:"key"` + Value types.String `tfsdk:"value"` + Values types.List `tfsdk:"values"` } -func fromCondition(condition client.Condition, ref Condition) Condition { +func fromCondition(condition client.Condition, ref Condition) (Condition, error) { c := Condition{ - Type: types.StringValue(condition.Type), - Op: types.StringValue(condition.Op), - Value: types.StringValue(condition.Value), - Key: types.StringValue(condition.Key), - Neg: types.BoolValue(condition.Neg), + Type: types.StringValue(condition.Type), + Op: types.StringValue(condition.Op), + Key: types.StringValue(condition.Key), + Neg: types.BoolValue(condition.Neg), + Value: types.StringNull(), + Values: types.ListNull(types.StringType), } + if valueList, ok := condition.Value.([]interface{}); ok { + values, diags := basetypes.NewListValueFrom(context.Background(), types.StringType, valueList) + if diags.HasError() { + return c, fmt.Errorf("error converting values: %s - %s", diags[0].Summary(), diags[0].Detail()) + } + c.Values = values + } else { + val := condition.Value.(string) + c.Value = types.StringValue(val) + } + // Neg and Key are optional if ref.Neg == types.BoolNull() { c.Neg = types.BoolNull() @@ -678,7 +729,7 @@ func fromCondition(condition client.Condition, ref Condition) Condition { c.Value = types.StringNull() } } - return c + return c, nil } type IPRules struct { @@ -819,12 +870,16 @@ func (f *FirewallConfig) toClient() (client.FirewallConfig, error) { if err != nil { return conf, err } + condGroup, err := rule.Conditions() + if err != nil { + return conf, err + } conf.Rules = append(conf.Rules, client.FirewallRule{ ID: rule.ID.ValueString(), Name: rule.Name.ValueString(), Description: rule.Description.ValueString(), Active: rule.Active.IsNull() || rule.Active.ValueBool(), - ConditionGroup: rule.Conditions(), + ConditionGroup: condGroup, Action: client.Action{ Mitigate: mit, }, diff --git a/vercel/resource_firewall_config_test.go b/vercel/resource_firewall_config_test.go index 81c64c2a..aea6a1a7 100644 --- a/vercel/resource_firewall_config_test.go +++ b/vercel/resource_firewall_config_test.go @@ -122,6 +122,14 @@ func TestAcc_FirewallConfigResource(t *testing.T) { "vercel_firewall_config.custom", "rules.rule.2.action.redirect.permanent", "false"), + resource.TestCheckResourceAttr( + "vercel_firewall_config.custom", + "rules.rule.4.condition_group.0.conditions.0.values.0", + "/test1"), + resource.TestCheckResourceAttr( + "vercel_firewall_config.custom", + "rules.rule.4.condition_group.0.conditions.0.values.1", + "/test2"), resource.TestCheckResourceAttr( "vercel_firewall_config.ips", "ip_rules.rule.0.action", @@ -387,6 +395,23 @@ resource "vercel_firewall_config" "custom" { }] }] } + rule { + name = "test_list" + action = { + action = "deny" + } + condition_group = [{ + conditions = [{ + type = "path" + op = "inc" + values = [ + "/test1", + "/test2", + "/test3" + ] + }] + }] + } } } @@ -510,6 +535,23 @@ resource "vercel_firewall_config" "custom" { }] }] } + rule { + name = "test_list" + action = { + action = "deny" + } + condition_group = [{ + conditions = [{ + type = "path" + op = "inc" + values = [ + "/api", + "/api2", + "/api3" + ] + }] + }] + } } } From 842179ebedfc4762a5593dd8f6a40a46f49cff69 Mon Sep 17 00:00:00 2001 From: sueplex Date: Thu, 16 Jan 2025 14:33:38 -0500 Subject: [PATCH 2/5] uncomment, use any, regen docs --- client/firewall_config.go | 10 ++++----- docs/resources/firewall_config.md | 37 +++++++++++++++++++++++++++---- vercel/provider_test.go | 4 ++-- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/client/firewall_config.go b/client/firewall_config.go index 3468be34..4fa03916 100644 --- a/client/firewall_config.go +++ b/client/firewall_config.go @@ -33,11 +33,11 @@ type ConditionGroup struct { } type Condition struct { - Type string `json:"type"` - Op string `json:"op"` - Neg bool `json:"neg"` - Key string `json:"key"` - Value interface{} `json:"value"` + Type string `json:"type"` + Op string `json:"op"` + Neg bool `json:"neg"` + Key string `json:"key"` + Value any `json:"value"` } type Action struct { diff --git a/docs/resources/firewall_config.md b/docs/resources/firewall_config.md index 3946ee9e..4d9fc6cf 100644 --- a/docs/resources/firewall_config.md +++ b/docs/resources/firewall_config.md @@ -108,6 +108,34 @@ resource "vercel_firewall_config" "example" { action_duration = "5m" } } + + rule { + name = "Known clients" + description = "Match known keys in header + condition_group = [{ + conditions = [{ + type = "header" + key = "Authorization" + op = "inc" + values = [ + "key1", + "key2", + ] + }] + }] + + action = { + action = "rate_limit" + rate_limit = { + limit = 100 + window = 300 + keys = ["ip", "ja4"] + algo = "fixed_window" + action = "deny" + } + action_duration = "5m" + } + } } } @@ -193,7 +221,7 @@ Optional: Read-Only: -- `id` (String) +- `id` (String) The ID of this resource. @@ -365,7 +393,7 @@ Optional: Read-Only: -- `id` (String) +- `id` (String) The ID of this resource. ### Nested Schema for `rules.rule.action` @@ -420,8 +448,9 @@ Required: Optional: - `key` (String) Key within type to match against -- `neg` (Boolean) -- `value` (String) +- `neg` (Boolean) Negate the condition +- `value` (String) Value to match against +- `values` (List of String) Values to match against if op is inc, ninc ## Import diff --git a/vercel/provider_test.go b/vercel/provider_test.go index 1693c6fe..fb78a196 100644 --- a/vercel/provider_test.go +++ b/vercel/provider_test.go @@ -24,10 +24,10 @@ func mustHaveEnv(t *testing.T, name string) { func testAccPreCheck(t *testing.T) { mustHaveEnv(t, "VERCEL_API_TOKEN") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITHUB_REPO") - /*mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITLAB_REPO") + mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_GITLAB_REPO") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_BITBUCKET_REPO") mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_DOMAIN") - mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_ADDITIONAL_USER")*/ + mustHaveEnv(t, "VERCEL_TERRAFORM_TESTING_ADDITIONAL_USER") } var tc *client.Client From 5fd77dc1598e50d3fb103d65d67f1115e5686493 Mon Sep 17 00:00:00 2001 From: sueplex Date: Thu, 16 Jan 2025 14:39:20 -0500 Subject: [PATCH 3/5] manually overwrite? --- docs/resources/firewall_config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/resources/firewall_config.md b/docs/resources/firewall_config.md index 4d9fc6cf..fbe4f5e8 100644 --- a/docs/resources/firewall_config.md +++ b/docs/resources/firewall_config.md @@ -393,7 +393,7 @@ Optional: Read-Only: -- `id` (String) The ID of this resource. +- `id` (String) ### Nested Schema for `rules.rule.action` From 3b608bab36598d4fb960524be5371636e334f033 Mon Sep 17 00:00:00 2001 From: sueplex Date: Thu, 16 Jan 2025 14:44:32 -0500 Subject: [PATCH 4/5] manually overwrite? --- docs/resources/firewall_config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/resources/firewall_config.md b/docs/resources/firewall_config.md index fbe4f5e8..9f1fad79 100644 --- a/docs/resources/firewall_config.md +++ b/docs/resources/firewall_config.md @@ -221,7 +221,7 @@ Optional: Read-Only: -- `id` (String) The ID of this resource. +- `id` (String) From 58b7125cf540358286e64c839cf9a5e875da8f8e Mon Sep 17 00:00:00 2001 From: sueplex Date: Fri, 17 Jan 2025 16:55:23 -0500 Subject: [PATCH 5/5] add additional paths and update errors --- vercel/resource_firewall_config.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/vercel/resource_firewall_config.go b/vercel/resource_firewall_config.go index a3d31b2c..8d43cad6 100644 --- a/vercel/resource_firewall_config.go +++ b/vercel/resource_firewall_config.go @@ -332,6 +332,7 @@ Define Custom Rules to shape the way your traffic is handled by the Vercel Edge Validators: []validator.String{ stringvalidator.ConflictsWith( path.MatchRelative().AtParent().AtName("values"), + path.MatchRelative().AtParent().AtName("value"), ), }, Description: "Value to match against", @@ -341,6 +342,7 @@ Define Custom Rules to shape the way your traffic is handled by the Vercel Edge Validators: []validator.List{ listvalidator.ConflictsWith( path.MatchRelative().AtParent().AtName("value"), + path.MatchRelative().AtParent().AtName("values"), ), }, ElementType: types.StringType, @@ -507,14 +509,14 @@ func (r *FirewallRule) Conditions() ([]client.ConditionGroup, error) { } if isListOp(condition.Op.ValueString()) { if condition.Values.IsNull() { - return nil, fmt.Errorf("%s conditionGroup.%d.condition.%d, operator requires list values", r.Name.ValueString(), cgIndex, condIndex) + return nil, fmt.Errorf("rule %s conditionGroup.%d.condition.%d, operator requires list values", r.Name.ValueString(), cgIndex, condIndex) } vals := make([]string, len(condition.Values.Elements())) condition.Values.ElementsAs(context.Background(), &vals, false) cond.Value = vals } else { if !condition.Values.IsNull() { - return nil, fmt.Errorf("%s conditionGroup.%d.condition.%d, operator does not allow values", r.Name.ValueString(), cgIndex, condIndex) + return nil, fmt.Errorf("rule %s conditionGroup.%d.condition.%d, operator does not allow values", r.Name.ValueString(), cgIndex, condIndex) } cond.Value = condition.Value.ValueString() } @@ -706,12 +708,17 @@ func fromCondition(condition client.Condition, ref Condition) (Condition, error) Value: types.StringNull(), Values: types.ListNull(types.StringType), } - if valueList, ok := condition.Value.([]interface{}); ok { - values, diags := basetypes.NewListValueFrom(context.Background(), types.StringType, valueList) - if diags.HasError() { - return c, fmt.Errorf("error converting values: %s - %s", diags[0].Summary(), diags[0].Detail()) + if isListOp(condition.Op) { + if valueList, ok := condition.Value.([]interface{}); ok { + values, diags := basetypes.NewListValueFrom(context.Background(), types.StringType, valueList) + if diags.HasError() { + return c, fmt.Errorf("error converting values: %s - %s", diags[0].Summary(), diags[0].Detail()) + } + c.Values = values + } else { + return c, fmt.Errorf("condition value is not a list") + } - c.Values = values } else { val := condition.Value.(string) c.Value = types.StringValue(val) @@ -902,7 +909,6 @@ func (f *FirewallConfig) toClient() (client.FirewallConfig, error) { } func (r *firewallConfigResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan FirewallConfig diags := req.Plan.Get(ctx, &plan) if resp.Diagnostics.HasError() { @@ -912,6 +918,7 @@ func (r *firewallConfigResource) Create(ctx context.Context, req resource.Create conf, err := plan.toClient() if err != nil { diags.AddError("failed to convert plan to client", err.Error()) + resp.Diagnostics.Append(diags...) return } @@ -969,6 +976,7 @@ func (r *firewallConfigResource) Update(ctx context.Context, req resource.Update conf, err := plan.toClient() if err != nil { diags.AddError("failed to convert plan to client", err.Error()) + resp.Diagnostics.Append(diags...) return }