From af3c54258c6f979f004691846dd1e3ae533b4bdb Mon Sep 17 00:00:00 2001 From: Rachel Date: Mon, 5 Feb 2024 22:26:30 +0000 Subject: [PATCH 1/3] revisions to sdk api for traffic policy following our discussion --- CHANGELOG.md | 10 ++ VERSION | 2 +- config/policy.go | 196 +++-------------------------- config/policy_test.go | 130 ++++++------------- go.mod | 2 +- go.sum | 2 + trafficpolicy/policy.go | 91 ++++++++++++++ trafficpolicy/policy_test.go | 234 +++++++++++++++++++++++++++++++++++ 8 files changed, 399 insertions(+), 268 deletions(-) create mode 100644 trafficpolicy/policy.go create mode 100644 trafficpolicy/policy_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b162c75..9951b1d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## 1.8.1 +Enhancements: + +- Provides access to structs for building a Traffic Policy configuration + +Breaking changes: + +- Renames pre-release option `WithPolicyConfig` to `WithPolicyString` +- Changes signature of `WithPolicy` option to accept the newly exposed structs for building a Traffic Policy configuration + ## 1.8.0 - Adds the `WithPolicy` and `WithPolicyConfig` options for applying a Traffic Policy to an endpoint. diff --git a/VERSION b/VERSION index 27f9cd32..a8fdfda1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.8.0 +1.8.1 diff --git a/config/policy.go b/config/policy.go index 5d0ceb60..200854b2 100644 --- a/config/policy.go +++ b/config/policy.go @@ -2,69 +2,22 @@ package config import ( "encoding/json" - "fmt" "golang.ngrok.com/ngrok/internal/pb" + "golang.ngrok.com/ngrok/trafficpolicy" ) -type policy struct { - Inbound inboundRules `json:"inbound,omitempty"` - Outbound outboundRules `json:"outbound,omitempty"` -} - -type inboundRules []policyRule -type outboundRules []policyRule - -type policyRule struct { - Name string `json:"name,omitempty"` - Expressions []string `json:"expressions,omitempty"` - Actions []action `json:"actions"` -} -type action struct { - Type string `json:"type"` - Config json.RawMessage `json:"config,omitempty"` -} - -type PolicyActionOption = option[*action] -type PolicyRuleOption = option[*policyRule] -type PolicyRuleSetOption = option[*[]policyRule] -type PolicyOption = option[*policy] - -// Supports conversion to a json string -type JsonConvertible interface { - ToJSON() string -} - -func (p *policy) ToJSON() string { - return toJSON(p) -} - -func (p policyRule) ToJSON() string { - return toJSON(p) -} - -func (p action) ToJSON() string { - return toJSON(p) -} - -func toJSON(o any) string { - bytes, err := json.Marshal(o) +type policy trafficpolicy.Policy +type rule trafficpolicy.Rule +type action trafficpolicy.Action - if err != nil { - panic(fmt.Sprintf("failed to convert to json with error: %s", err.Error())) - } - - return string(bytes) -} - -// WithPolicyConfig configures this edge with the provided policy configuration -// passed as a json string and overwrites any prevously-set traffic policy +// WithPolicyString configures this edge with the provided policy configuration +// passed as a json string and overwrites any previously-set traffic policy // https://ngrok.com/docs/http/traffic-policy -func WithPolicyConfig(jsonStr string) interface { +func WithPolicyString(jsonStr string) interface { HTTPEndpointOption TLSEndpointOption TCPEndpointOption - JsonConvertible } { p := &policy{} if err := json.Unmarshal([]byte(jsonStr), p); err != nil { @@ -74,130 +27,17 @@ func WithPolicyConfig(jsonStr string) interface { return p } -// WithPolicy configures this edge with the given traffic and overwrites any +// WithPolicy configures this edge with the given traffic policy and overwrites any // previously-set traffic policy // https://ngrok.com/docs/http/traffic-policy/ -func WithPolicy(opts ...PolicyOption) interface { +func WithPolicy(tp trafficpolicy.Policy) interface { HTTPEndpointOption TLSEndpointOption TCPEndpointOption - JsonConvertible -} { - cfg := &policy{} - applyOpts(cfg, opts...) - - return cfg -} - -// WithInboundRules adds the provided policy rules to the inbound -// set of the given policy. -// The order in which policies are specified is observed. -func WithInboundRules(opts ...PolicyRuleSetOption) PolicyOption { - rules := []policyRule{} - applyOpts(&rules, opts...) - - return inboundRules(rules) -} - -// WithOutboundRules adds the provided policy to be outbound -// set of the given policy. -// The order in which policies are specified is observed. -func WithOutboundRules(opts ...PolicyRuleSetOption) PolicyOption { - rules := []policyRule{} - applyOpts(&rules, opts...) - - return outboundRules(rules) -} - -// WithPolicyRule provides a policy rule built from the given options. -func WithPolicyRule(opts ...PolicyRuleOption) interface { - PolicyRuleSetOption - JsonConvertible } { - pr := policyRule{} - applyOpts(&pr, opts...) + p := policy(tp) - return pr -} - -// WithPolicyName sets the provided name on a policy rule. -func WithPolicyName(n string) PolicyRuleOption { - return optionFunc[*policyRule]( - func(r *policyRule) { - r.Name = n - }) -} - -// WithPolicyExpression appends the provided CEL statement to a policy rule's expressions. -func WithPolicyExpression(e string) PolicyRuleOption { - return optionFunc[*policyRule]( - func(r *policyRule) { - r.Expressions = append(r.Expressions, e) - }) -} - -// WithPolicyAction appends the provided action to the set of the policy rule. -// The order the actions are specified is observed. -func WithPolicyAction(opts ...PolicyActionOption) interface { - PolicyRuleOption - JsonConvertible -} { - a := action{} - applyOpts(&a, opts...) - - return a -} - -// WithActionType sets the provided type for this action. Type must be specified. -func WithPolicyActionType(t string) PolicyActionOption { - return optionFunc[*action](func(a *action) { a.Type = t }) -} - -// WithConfig sets the provided json string as the configuration for this action -func WithPolicyActionConfig(cfg string) PolicyActionOption { - return optionFunc[*action]( - func(a *action) { - a.Config = []byte(cfg) - }) -} - -// supports inbound rules as an a policy option -func (ib inboundRules) apply(p *policy) { - p.Inbound = append(p.Inbound, ib...) -} - -// supports outbound rules as a policy option -func (ib outboundRules) apply(p *policy) { - p.Outbound = append(p.Outbound, ib...) -} - -// supports policy rule as an option of a collection of -// rules, which can be used for inbound or outbound -func (pr policyRule) apply(r *[]policyRule) { - *r = append(*r, pr) -} - -// supports action as an option of a policy rule -func (a action) apply(p *policyRule) { - p.Actions = append(p.Actions, a) -} - -// an option that is applicable to the specified type -type option[T any] interface { - apply(T) -} - -type optionFunc[T any] func(T) - -func (f optionFunc[T]) apply(a T) { - f(a) -} - -// applies the set of options to the specified target -func applyOpts[T any](target T, opts ...option[T]) { - for _, o := range opts { - o.apply(target) - } + return &p } func (p *policy) ApplyTLS(opts *tlsOptions) { @@ -218,12 +58,12 @@ func (p *policy) toProtoConfig() *pb.MiddlewareConfiguration_Policy { } inbound := make([]*pb.MiddlewareConfiguration_PolicyRule, len(p.Inbound)) for i, inP := range p.Inbound { - inbound[i] = inP.toProtoConfig() + inbound[i] = rule(inP).toProtoConfig() } outbound := make([]*pb.MiddlewareConfiguration_PolicyRule, len(p.Outbound)) for i, outP := range p.Outbound { - outbound[i] = outP.toProtoConfig() + outbound[i] = rule(outP).toProtoConfig() } return &pb.MiddlewareConfiguration_Policy{ Inbound: inbound, @@ -231,18 +71,22 @@ func (p *policy) toProtoConfig() *pb.MiddlewareConfiguration_Policy { } } -func (pr policyRule) toProtoConfig() *pb.MiddlewareConfiguration_PolicyRule { +func (pr rule) toProtoConfig() *pb.MiddlewareConfiguration_PolicyRule { actions := make([]*pb.MiddlewareConfiguration_PolicyAction, len(pr.Actions)) for i, act := range pr.Actions { - actions[i] = act.toProtoConfig() + actions[i] = action(act).toProtoConfig() } return &pb.MiddlewareConfiguration_PolicyRule{Name: pr.Name, Expressions: pr.Expressions, Actions: actions} } func (a action) toProtoConfig() *pb.MiddlewareConfiguration_PolicyAction { + var cfgBytes []byte = nil + if a.Config != "" { + cfgBytes = []byte(a.Config) + } return &pb.MiddlewareConfiguration_PolicyAction{ Type: a.Type, - Config: []byte(a.Config), + Config: cfgBytes, } } diff --git a/config/policy_test.go b/config/policy_test.go index 31803943..f0df071f 100644 --- a/config/policy_test.go +++ b/config/policy_test.go @@ -7,6 +7,7 @@ import ( "golang.ngrok.com/ngrok/internal/pb" "golang.ngrok.com/ngrok/internal/tunnel/proto" + "golang.ngrok.com/ngrok/trafficpolicy" ) func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, @@ -29,32 +30,42 @@ func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, name: "with policy", opts: optsFunc( WithPolicy( - WithInboundRules( - WithPolicyRule( - WithPolicyName("denyPUT"), - WithPolicyExpression("req.Method == 'PUT'"), - WithPolicyAction(WithPolicyActionType("deny")), - ), - WithPolicyRule( - WithPolicyName("logFooHeader"), - WithPolicyExpression("'foo' in req.Headers"), - WithPolicyAction( - WithPolicyActionType("log"), - WithPolicyActionConfig(`{"metadata": {"key": "val"}}`), - ), - ), - ), - WithOutboundRules( - WithPolicyRule( - WithPolicyName("InternalErrorWhenFailed"), - WithPolicyExpression("res.StatusCode <= 0"), - WithPolicyExpression("res.StatusCode >= 300"), - WithPolicyAction( - WithPolicyActionType("custom-response"), - WithPolicyActionConfig(`"status_code": 500}`), - ), - ), - ), + trafficpolicy.Policy{ + Inbound: []trafficpolicy.Rule{ + { + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []trafficpolicy.Action{ + {Type: "deny"}, + }, + }, + { + Name: "logFooHeader", + Expressions: []string{"'foo' in req.Headers"}, + Actions: []trafficpolicy.Action{ + { + Type: "log", + Config: `{"metadata": {"key": "val"}}`, + }, + }, + }, + }, + Outbound: []trafficpolicy.Rule{ + { + Name: "InternalErrorWhenFailed", + Expressions: []string{ + "res.StatusCode <= '0'", + "res.StatusCode >= '300'", + }, + Actions: []trafficpolicy.Action{ + { + Type: "custom-response", + Config: `"status_code": 500}`, + }, + }, + }, + }, + }, ), ), expectOpts: func(t *testing.T, opts *O) { @@ -68,9 +79,9 @@ func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, }, }, { - name: "with policy config", + name: "with policy string", opts: optsFunc( - WithPolicyConfig(` + WithPolicyString(` { "inbound":[ { @@ -99,7 +110,7 @@ func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, require.NotNil(t, actual) require.Len(t, actual.Inbound, 2) require.Equal(t, "denyPut", actual.Inbound[0].Name) - require.Equal(t, actual.Inbound[0].Actions, []*pb.MiddlewareConfiguration_PolicyAction{{Type: "deny"}}) + require.Equal(t, []*pb.MiddlewareConfiguration_PolicyAction{{Type: "deny"}}, actual.Inbound[0].Actions) require.Len(t, actual.Outbound, 1) require.Len(t, actual.Outbound[0].Expressions, 2) }, @@ -123,64 +134,3 @@ func TestPolicy(t *testing.T) { return h.Policy }) } - -func TestPolicyToJSON(t *testing.T) { - t.Run("Convert whole policy to json", func(t *testing.T) { - cfg := WithPolicy( - WithInboundRules( - WithPolicyRule( - WithPolicyName("deny put requests"), - WithPolicyExpression("req.Method == 'PUT'"), - WithPolicyAction( - WithPolicyActionType("deny"))), - WithPolicyRule( - WithPolicyName("log 'foo' header"), - WithPolicyExpression("'foo' in req.Headers"), - WithPolicyAction( - WithPolicyActionType("log"), - WithPolicyActionConfig(`{"metadata":{"key":"val"}}`)))), - WithOutboundRules( - WithPolicyRule( - WithPolicyName("return 500 when response not success"), - WithPolicyExpression("res.StatusCode <= 0"), - WithPolicyExpression("res.StatusCode >= 300"), - WithPolicyAction( - WithPolicyActionType("custom-response"), - WithPolicyActionConfig(`{"status_code":500}`))))) - - json := cfg.ToJSON() - - result := WithPolicyConfig(json) - - require.Equal(t, cfg, result) - }) - - t.Run("convert policy rule to json", func(t *testing.T) { - expected := `{"name":"denyPut","expressions":["req.Method == 'PUT'"],"actions":[{"type":"deny","config":{"status_code":401}}]}` - - policy := WithPolicyRule( - WithPolicyName("denyPut"), - WithPolicyExpression("req.Method == 'PUT'"), - WithPolicyAction( - WithPolicyActionType("deny"), - WithPolicyActionConfig(`{"status_code":401}`), - ), - ) - - result := policy.ToJSON() - - require.Equal(t, expected, result) - }) - - t.Run("convert action to json", func(t *testing.T) { - expected := `{"type":"deny","config":{"status_code":401}}` - action := WithPolicyAction( - WithPolicyActionType("deny"), - WithPolicyActionConfig(`{"status_code":401}`), - ) - - result := action.ToJSON() - - require.Equal(t, expected, result) - }) -} diff --git a/go.mod b/go.mod index 1a180ea5..22d21cb5 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( golang.org/x/net v0.15.0 golang.org/x/sync v0.3.0 google.golang.org/protobuf v1.31.0 + gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -22,5 +23,4 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.12.0 // indirect golang.org/x/term v0.12.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 6f156dcd..09526631 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,7 @@ github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaS github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE= +github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ= github.com/inconshreveable/log15 v3.0.0-testing.3+incompatible h1:zaX5fYT98jX5j4UhO/WbfY8T1HkgVrydiDMC9PWqGCo= github.com/inconshreveable/log15 v3.0.0-testing.3+incompatible/go.mod h1:cOaXtrgN4ScfRrD9Bre7U1thNq5RtJ8ZoP4iXVGRj6o= github.com/inconshreveable/log15/v3 v3.0.0-testing.5 h1:h4e0f3kjgg+RJBlKOabrohjHe47D3bbAB9BgMrc3DYA= @@ -25,6 +26,7 @@ go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN8 golang.ngrok.com/muxado/v2 v2.0.0 h1:bu9eIDhRdYNtIXNnqat/HyMeHYOAbUH55ebD7gTvW6c= golang.ngrok.com/muxado/v2 v2.0.0/go.mod h1:wzxJYX4xiAtmwumzL+QsukVwFRXmPNv86vB8RPpOxyM= golang.org/x/crypto v0.13.0 h1:mvySKfSWJ+UKUii46M40LOvyWfN0s2U+46/jDd0e6Ck= +golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E= diff --git a/trafficpolicy/policy.go b/trafficpolicy/policy.go new file mode 100644 index 00000000..ca6dcdbb --- /dev/null +++ b/trafficpolicy/policy.go @@ -0,0 +1,91 @@ +package trafficpolicy + +import ( + "bytes" + "encoding/json" + + "gopkg.in/yaml.v3" +) + +type Policy struct { + // the ordered set of rules that apply to inbound traffic + Inbound []Rule `json:"inbound,omitempty" yaml:"inbound,omitempty"` + // the ordered set of rules that apply to outbound traffic + Outbound []Rule `json:"outbound,omitempty" yaml:"outbound,omitempty"` +} + +type Rule struct { + // the name of the traffic policy rule + Name string `json:"name,omitempty" yaml:"name,omitempty"` + // the set of CEL expressions used to determine if this rule is applicable + Expressions []string `json:"expressions,omitempty" yaml:"expressions,omitempty"` + // the ordered set of actions that should take effect against the traffic + Actions []Action `json:"actions" yaml:"actions"` +} + +type Action struct { + // the type of action that should be used + Type string `json:"type" yaml:"type"` + // the configuration for the specified action type written as a json string + Config ActionConfigString `json:"config,omitempty" yaml:"config,omitempty"` +} + +type ActionConfigString string + +func (s ActionConfigString) MarshalJSON() ([]byte, error) { + raw := json.RawMessage([]byte(s)) + + return json.Marshal(raw) +} + +func (cfg *ActionConfigString) UnmarshalJSON(data []byte) error { + s := ActionConfigString(data) + *cfg = s + return nil +} + +func (p Policy) MarshalJSON() (string, error) { + return marshalJSON(p) +} + +func (p Rule) MarshalJSON() (string, error) { + return marshalJSON(p) +} + +func (p Action) MarshalJSON() (string, error) { + return marshalJSON(p) +} + +func marshalJSON(o any) (string, error) { + b := new(bytes.Buffer) + enc := json.NewEncoder(b) + enc.SetEscapeHTML(false) + + if err := enc.Encode(o); err != nil { + return "", err + } + + return b.String(), nil +} + +func (p Policy) MarshalYAML() (string, error) { + return marshalJSON(p) +} + +func (p Rule) MarshalYAML() (string, error) { + return marshalJSON(p) +} + +func (p Action) MarshalYAML() (string, error) { + return marshalJSON(p) +} + +func marshalYAML(o any) (string, error) { + bytes, err := yaml.Marshal(o) + + if err != nil { + return "", err + } + + return string(bytes), nil +} diff --git a/trafficpolicy/policy_test.go b/trafficpolicy/policy_test.go new file mode 100644 index 00000000..822ae803 --- /dev/null +++ b/trafficpolicy/policy_test.go @@ -0,0 +1,234 @@ +package trafficpolicy + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPolicyToJSON(t *testing.T) { + t.Run("Convert whole policy to json", func(t *testing.T) { + expected := ` + { + "inbound": [ + { + "name":"denyPUT", + "expressions":["req.Method == 'PUT'"], + "actions":[ + { + "type":"deny" + } + ] + }, + { + "name":"logFooHeader", + "expressions":["'foo' in req.Headers"], + "actions":[ + { + "type":"log", + "config":{ + "metadata":{ + "key":"val" + } + } + } + ] + } + ], + "outbound": [ + { + "name":"InternalErrorWhenFailed", + "expressions":["res.StatusCode <= '0'", "res.StatusCode >= '300'"], + "actions":[ + { + "type":"custom-response", + "config":{ + "status_code":500 + } + } + ] + } + ] + }` + cfg := Policy{ + Inbound: []Rule{ + { + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + {Type: "deny"}, + }, + }, + { + Name: "logFooHeader", + Expressions: []string{"'foo' in req.Headers"}, + Actions: []Action{ + { + Type: "log", + Config: `{"metadata":{"key":"val"}}`, + }, + }, + }, + }, + Outbound: []Rule{ + { + Name: "InternalErrorWhenFailed", + Expressions: []string{ + "res.StatusCode <= '0'", + "res.StatusCode >= '300'", + }, + Actions: []Action{ + { + Type: "custom-response", + Config: `{"status_code":500}`, + }, + }, + }, + }, + } + + json, err := cfg.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, expected, json) + }) + + t.Run("convert policy rule to json", func(t *testing.T) { + expected := `{"name":"denyPUT","expressions":["req.Method == 'PUT'"],"actions":[{"type":"deny","config":{"status_code":401}}]}` + + policy := Rule{ + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + { + Type: "deny", + Config: `{"status_code": 401}`, + }, + }, + } + + result, err := policy.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, expected, result) + }) + + t.Run("convert action to json", func(t *testing.T) { + expected := `{"type":"deny","config":{"status_code":401}}` + action := Action{ + + Type: "deny", + Config: `{"status_code": 401}`, + } + + result, err := action.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, expected, result) + }) +} + +func TestPolicyToYAML(t *testing.T) { + t.Run("Convert whole policy to yaml", func(t *testing.T) { + expected := ` + inbound: + - name: "denyPUT" + expressions: ["req.Method == 'PUT'"] + actions: + - type: "deny" + - name: "logFooHeader" + expressions: ["'foo' in req.Headers"] + actions: + - type: "log" + config: + metadata: + key: "val" + outbound: + - name: "InternalErrorWhenFailed" + expressions: + - "res.StatusCode <= '0'" + - "res.StatusCode >= '300'" + actions: + - type: "custom-response" + config: + status_code: 500` + cfg := Policy{ + Inbound: []Rule{ + { + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + {Type: "deny"}, + }, + }, + { + Name: "logFooHeader", + Expressions: []string{"'foo' in req.Headers"}, + Actions: []Action{ + { + Type: "log", + Config: `{"metadata":{"key":"val"}}`, + }, + }, + }, + }, + Outbound: []Rule{ + { + Name: "InternalErrorWhenFailed", + Expressions: []string{ + "res.StatusCode <= '0'", + "res.StatusCode >= '300'", + }, + Actions: []Action{ + { + Type: "custom-response", + Config: `{"status_code":500}`, + }, + }, + }, + }, + } + + yaml, err := cfg.MarshalYAML() + require.NoError(t, err) + require.YAMLEq(t, expected, yaml) + }) + + t.Run("convert policy rule to json", func(t *testing.T) { + expected := ` + name: "denyPUT" + expressions: ["req.Method == 'PUT'"] + actions: + - type: "deny" + config: + status_code: 401` + + policy := Rule{ + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + { + Type: "deny", + Config: `{"status_code": 401}`, + }, + }, + } + + result, err := policy.MarshalYAML() + require.NoError(t, err) + require.YAMLEq(t, expected, result) + }) + + t.Run("convert action to json", func(t *testing.T) { + expected := ` + type: "deny" + config: + status_code: 401` + action := Action{ + + Type: "deny", + Config: `{"status_code": 401}`, + } + + result, err := action.MarshalYAML() + require.NoError(t, err) + require.YAMLEq(t, expected, result) + }) +} From 424304973efe435e03afa17fa35db482a815b248 Mon Sep 17 00:00:00 2001 From: Rachel Date: Mon, 5 Feb 2024 23:52:22 +0000 Subject: [PATCH 2/3] fix yaml marshal --- trafficpolicy/policy.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/trafficpolicy/policy.go b/trafficpolicy/policy.go index ca6dcdbb..e6b25b24 100644 --- a/trafficpolicy/policy.go +++ b/trafficpolicy/policy.go @@ -3,6 +3,7 @@ package trafficpolicy import ( "bytes" "encoding/json" + "errors" "gopkg.in/yaml.v3" ) @@ -39,11 +40,24 @@ func (s ActionConfigString) MarshalJSON() ([]byte, error) { } func (cfg *ActionConfigString) UnmarshalJSON(data []byte) error { + if cfg == nil { + return errors.New("ActionConfigString: UnmarshalJSON on nil pointer") + } + s := ActionConfigString(data) *cfg = s return nil } +func (s ActionConfigString) MarshalYAML() (interface{}, error) { + var innerMap map[string]interface{} + if err := yaml.Unmarshal([]byte(s), &innerMap); err != nil { + return nil, err + } + + return innerMap, nil +} + func (p Policy) MarshalJSON() (string, error) { return marshalJSON(p) } @@ -69,15 +83,15 @@ func marshalJSON(o any) (string, error) { } func (p Policy) MarshalYAML() (string, error) { - return marshalJSON(p) + return marshalYAML(p) } func (p Rule) MarshalYAML() (string, error) { - return marshalJSON(p) + return marshalYAML(p) } func (p Action) MarshalYAML() (string, error) { - return marshalJSON(p) + return marshalYAML(p) } func marshalYAML(o any) (string, error) { From b96ce892617ce12aece0cd1369e141cd32d9c669 Mon Sep 17 00:00:00 2001 From: Rachel Date: Tue, 6 Feb 2024 18:13:21 +0000 Subject: [PATCH 3/3] addressing feedback: renames and constructors. Action config changed to map type. --- config/policy.go | 25 ++-- config/policy_test.go | 19 +-- go.mod | 3 +- go.sum | 2 + policy/policy.go | 124 ++++++++++++++++++++ {trafficpolicy => policy}/policy_test.go | 143 ++++++++++++++++++++--- trafficpolicy/policy.go | 105 ----------------- 7 files changed, 282 insertions(+), 139 deletions(-) create mode 100644 policy/policy.go rename {trafficpolicy => policy}/policy_test.go (58%) delete mode 100644 trafficpolicy/policy.go diff --git a/config/policy.go b/config/policy.go index 200854b2..bc964181 100644 --- a/config/policy.go +++ b/config/policy.go @@ -2,14 +2,16 @@ package config import ( "encoding/json" + "errors" + "fmt" "golang.ngrok.com/ngrok/internal/pb" - "golang.ngrok.com/ngrok/trafficpolicy" + po "golang.ngrok.com/ngrok/policy" ) -type policy trafficpolicy.Policy -type rule trafficpolicy.Rule -type action trafficpolicy.Action +type policy po.Policy +type rule po.Rule +type action po.Action // WithPolicyString configures this edge with the provided policy configuration // passed as a json string and overwrites any previously-set traffic policy @@ -30,14 +32,14 @@ func WithPolicyString(jsonStr string) interface { // WithPolicy configures this edge with the given traffic policy and overwrites any // previously-set traffic policy // https://ngrok.com/docs/http/traffic-policy/ -func WithPolicy(tp trafficpolicy.Policy) interface { +func WithPolicy(p po.Policy) interface { HTTPEndpointOption TLSEndpointOption TCPEndpointOption } { - p := policy(tp) + ret := policy(p) - return &p + return &ret } func (p *policy) ApplyTLS(opts *tlsOptions) { @@ -82,8 +84,13 @@ func (pr rule) toProtoConfig() *pb.MiddlewareConfiguration_PolicyRule { func (a action) toProtoConfig() *pb.MiddlewareConfiguration_PolicyAction { var cfgBytes []byte = nil - if a.Config != "" { - cfgBytes = []byte(a.Config) + if len(a.Config) > 0 { + var err error + cfgBytes, err = json.Marshal(a.Config) + + if err != nil { + panic(errors.New(fmt.Sprintf("failed to parse action configuration due to error: %s", err.Error()))) + } } return &pb.MiddlewareConfiguration_PolicyAction{ Type: a.Type, diff --git a/config/policy_test.go b/config/policy_test.go index f0df071f..bb0b27ad 100644 --- a/config/policy_test.go +++ b/config/policy_test.go @@ -7,7 +7,7 @@ import ( "golang.ngrok.com/ngrok/internal/pb" "golang.ngrok.com/ngrok/internal/tunnel/proto" - "golang.ngrok.com/ngrok/trafficpolicy" + po "golang.ngrok.com/ngrok/policy" ) func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, @@ -30,37 +30,37 @@ func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, name: "with policy", opts: optsFunc( WithPolicy( - trafficpolicy.Policy{ - Inbound: []trafficpolicy.Rule{ + po.Policy{ + Inbound: []po.Rule{ { Name: "denyPUT", Expressions: []string{"req.Method == 'PUT'"}, - Actions: []trafficpolicy.Action{ + Actions: []po.Action{ {Type: "deny"}, }, }, { Name: "logFooHeader", Expressions: []string{"'foo' in req.Headers"}, - Actions: []trafficpolicy.Action{ + Actions: []po.Action{ { Type: "log", - Config: `{"metadata": {"key": "val"}}`, + Config: map[string]any{"metadata": map[string]any{"key": "val"}}, }, }, }, }, - Outbound: []trafficpolicy.Rule{ + Outbound: []po.Rule{ { Name: "InternalErrorWhenFailed", Expressions: []string{ "res.StatusCode <= '0'", "res.StatusCode >= '300'", }, - Actions: []trafficpolicy.Action{ + Actions: []po.Action{ { Type: "custom-response", - Config: `"status_code": 500}`, + Config: map[string]any{"status_code": 500}, }, }, }, @@ -113,6 +113,7 @@ func testPolicy[T tunnelConfigPrivate, O any, OT any](t *testing.T, require.Equal(t, []*pb.MiddlewareConfiguration_PolicyAction{{Type: "deny"}}, actual.Inbound[0].Actions) require.Len(t, actual.Outbound, 1) require.Len(t, actual.Outbound[0].Expressions, 2) + require.Equal(t, []byte(`{"status_code":500}`), actual.Outbound[0].Actions[0].Config) }, }, } diff --git a/go.mod b/go.mod index 22d21cb5..4ff6e98e 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( golang.org/x/net v0.15.0 golang.org/x/sync v0.3.0 google.golang.org/protobuf v1.31.0 - gopkg.in/yaml.v3 v3.0.1 + gopkg.in/yaml.v2 v2.4.0 ) require ( @@ -23,4 +23,5 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.12.0 // indirect golang.org/x/term v0.12.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 09526631..0f3d73dd 100644 --- a/go.sum +++ b/go.sum @@ -43,5 +43,7 @@ google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/policy/policy.go b/policy/policy.go new file mode 100644 index 00000000..7de5af54 --- /dev/null +++ b/policy/policy.go @@ -0,0 +1,124 @@ +package policy + +import ( + "bytes" + "encoding/json" + "fmt" + + "gopkg.in/yaml.v2" +) + +type Policy struct { + // the ordered set of rules that apply to inbound traffic + Inbound []Rule `json:"inbound,omitempty" yaml:"inbound,omitempty"` + // the ordered set of rules that apply to outbound traffic + Outbound []Rule `json:"outbound,omitempty" yaml:"outbound,omitempty"` +} + +type Rule struct { + // the name of the traffic policy rule + Name string `json:"name,omitempty" yaml:"name,omitempty"` + // the set of CEL expressions used to determine if this rule is applicable + Expressions []string `json:"expressions,omitempty" yaml:"expressions,omitempty"` + // the ordered set of actions that should take effect against the traffic + Actions []Action `json:"actions" yaml:"actions"` +} + +type Action struct { + // the type of action that should be used + Type string `json:"type" yaml:"type"` + // the configuration for the specified action type written as a json string + Config map[string]any `json:"config,omitempty" yaml:"config,omitempty"` +} + +// converts the policy to a json string +func (p Policy) JSON() (string, error) { + return marshalJSON(p) +} + +// converts the policy to a yaml string +func (p Policy) YAML() (string, error) { + return marshalYAML(p) +} + +// creates a rule from the specified string in json or yaml format +func NewRuleFromString(input string) (Rule, error) { + r := Rule{} + err := unmarshal(input, &r) + + return r, err +} + +// creates a rule from the specified string in json or yaml format and panics if invalid +func MustRuleFromString(input string) Rule { + r := Rule{} + if err := unmarshal(input, &r); err != nil { + panic(fmt.Sprintf("failed to create rule from specified string due to error: %s", err.Error())) + } + + return r +} + +// converts the rule to a json string +func (p Rule) JSON() (string, error) { + return marshalJSON(p) +} + +// converts the rule to a yaml string +func (p Rule) YAML() (string, error) { + return marshalYAML(p) +} + +// creates an action from the specified string in json or yaml format +func NewActionFromString(input string) (Action, error) { + a := Action{} + err := unmarshal(input, &a) + + return a, err +} + +// creates an action from the specified string in json or yaml format and panics if invalid +func MustActionFromString(input string) Action { + a := Action{} + if err := unmarshal(input, &a); err != nil { + panic(fmt.Sprintf("failed to create action from specified string due to error: %s", err.Error())) + } + + return a +} + +// converts the action to a json string +func (p Action) JSON() (string, error) { + return marshalJSON(p) +} + +// converts the action to a yaml string +func (p Action) YAML() (string, error) { + return marshalYAML(p) +} + +func marshalJSON(o any) (string, error) { + b := new(bytes.Buffer) + enc := json.NewEncoder(b) + enc.SetEscapeHTML(false) + + if err := enc.Encode(o); err != nil { + return "", err + } + + return b.String(), nil +} + +func marshalYAML(o any) (string, error) { + bytes, err := yaml.Marshal(o) + + if err != nil { + return "", err + } + + return string(bytes), nil +} + +func unmarshal(input string, typ any) error { + return yaml.UnmarshalStrict([]byte(input), typ) +} diff --git a/trafficpolicy/policy_test.go b/policy/policy_test.go similarity index 58% rename from trafficpolicy/policy_test.go rename to policy/policy_test.go index 822ae803..f6638e27 100644 --- a/trafficpolicy/policy_test.go +++ b/policy/policy_test.go @@ -1,4 +1,4 @@ -package trafficpolicy +package policy import ( "testing" @@ -65,7 +65,7 @@ func TestPolicyToJSON(t *testing.T) { Actions: []Action{ { Type: "log", - Config: `{"metadata":{"key":"val"}}`, + Config: map[string]any{"metadata": map[string]any{"key": "val"}}, }, }, }, @@ -80,14 +80,14 @@ func TestPolicyToJSON(t *testing.T) { Actions: []Action{ { Type: "custom-response", - Config: `{"status_code":500}`, + Config: map[string]any{"status_code": 500}, }, }, }, }, } - json, err := cfg.MarshalJSON() + json, err := cfg.JSON() require.NoError(t, err) require.JSONEq(t, expected, json) }) @@ -101,12 +101,12 @@ func TestPolicyToJSON(t *testing.T) { Actions: []Action{ { Type: "deny", - Config: `{"status_code": 401}`, + Config: map[string]any{"status_code": 401}, }, }, } - result, err := policy.MarshalJSON() + result, err := policy.JSON() require.NoError(t, err) require.JSONEq(t, expected, result) }) @@ -116,10 +116,10 @@ func TestPolicyToJSON(t *testing.T) { action := Action{ Type: "deny", - Config: `{"status_code": 401}`, + Config: map[string]any{"status_code": 401}, } - result, err := action.MarshalJSON() + result, err := action.JSON() require.NoError(t, err) require.JSONEq(t, expected, result) }) @@ -164,7 +164,7 @@ func TestPolicyToYAML(t *testing.T) { Actions: []Action{ { Type: "log", - Config: `{"metadata":{"key":"val"}}`, + Config: map[string]any{"metadata": map[string]any{"key": "val"}}, }, }, }, @@ -179,14 +179,14 @@ func TestPolicyToYAML(t *testing.T) { Actions: []Action{ { Type: "custom-response", - Config: `{"status_code":500}`, + Config: map[string]any{"status_code": 500}, }, }, }, }, } - yaml, err := cfg.MarshalYAML() + yaml, err := cfg.YAML() require.NoError(t, err) require.YAMLEq(t, expected, yaml) }) @@ -206,12 +206,12 @@ func TestPolicyToYAML(t *testing.T) { Actions: []Action{ { Type: "deny", - Config: `{"status_code": 401}`, + Config: map[string]any{"status_code": 401}, }, }, } - result, err := policy.MarshalYAML() + result, err := policy.YAML() require.NoError(t, err) require.YAMLEq(t, expected, result) }) @@ -224,11 +224,124 @@ func TestPolicyToYAML(t *testing.T) { action := Action{ Type: "deny", - Config: `{"status_code": 401}`, + Config: map[string]any{"status_code": 401}, } - result, err := action.MarshalYAML() + result, err := action.YAML() require.NoError(t, err) require.YAMLEq(t, expected, result) }) } + +func TestFromString(t *testing.T) { + t.Run("rule from json", func(t *testing.T) { + input := `{"name":"denyPUT","expressions":["req.Method == 'PUT'"],"actions":[{"type":"deny","config":{"status_code":401}}]}` + expected := Rule{ + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + { + Type: "deny", + Config: map[string]any{"status_code": 401}, + }, + }, + } + + result, err := NewRuleFromString(input) + + require.NoError(t, err) + require.Equal(t, expected, result) + }) + + t.Run("new rule from yaml", func(t *testing.T) { + input := ` + name: "denyPUT" + expressions: ["req.Method == 'PUT'"] + actions: + - type: "deny" + config: + status_code: 401` + + expected := Rule{ + Name: "denyPUT", + Expressions: []string{"req.Method == 'PUT'"}, + Actions: []Action{ + { + Type: "deny", + Config: map[string]any{"status_code": 401}, + }, + }, + } + + result, err := NewRuleFromString(input) + + require.NoError(t, err) + require.Equal(t, expected, result) + }) + + t.Run("convert action to json", func(t *testing.T) { + input := `{"type":"deny","config":{"status_code":401}}` + expected := Action{ + + Type: "deny", + Config: map[string]any{"status_code": 401}, + } + + result, err := NewActionFromString(input) + + require.NoError(t, err) + require.Equal(t, expected, result) + }) + + t.Run("action from yaml", func(t *testing.T) { + input := ` + type: "deny" + config: + status_code: 401` + expected := Action{ + + Type: "deny", + Config: map[string]any{"status_code": 401}, + } + + result, err := NewActionFromString(input) + + require.NoError(t, err) + require.Equal(t, expected, result) + }) + + t.Run("must action to json", func(t *testing.T) { + input := `{"type":"deny","config":{"status_code":401}}` + expected := Action{ + + Type: "deny", + Config: map[string]any{"status_code": 401}, + } + + result := MustActionFromString(input) + + require.Equal(t, expected, result) + }) + + t.Run("must action from yaml", func(t *testing.T) { + input := ` + type: "deny" + config: + status_code: 401` + expected := Action{ + + Type: "deny", + Config: map[string]any{"status_code": 401}, + } + + result := MustActionFromString(input) + + require.Equal(t, expected, result) + }) + + t.Run("must action from invalid", func(t *testing.T) { + input := `invalid: val` + + require.Panics(t, func() { MustActionFromString(input) }) + }) +} diff --git a/trafficpolicy/policy.go b/trafficpolicy/policy.go deleted file mode 100644 index e6b25b24..00000000 --- a/trafficpolicy/policy.go +++ /dev/null @@ -1,105 +0,0 @@ -package trafficpolicy - -import ( - "bytes" - "encoding/json" - "errors" - - "gopkg.in/yaml.v3" -) - -type Policy struct { - // the ordered set of rules that apply to inbound traffic - Inbound []Rule `json:"inbound,omitempty" yaml:"inbound,omitempty"` - // the ordered set of rules that apply to outbound traffic - Outbound []Rule `json:"outbound,omitempty" yaml:"outbound,omitempty"` -} - -type Rule struct { - // the name of the traffic policy rule - Name string `json:"name,omitempty" yaml:"name,omitempty"` - // the set of CEL expressions used to determine if this rule is applicable - Expressions []string `json:"expressions,omitempty" yaml:"expressions,omitempty"` - // the ordered set of actions that should take effect against the traffic - Actions []Action `json:"actions" yaml:"actions"` -} - -type Action struct { - // the type of action that should be used - Type string `json:"type" yaml:"type"` - // the configuration for the specified action type written as a json string - Config ActionConfigString `json:"config,omitempty" yaml:"config,omitempty"` -} - -type ActionConfigString string - -func (s ActionConfigString) MarshalJSON() ([]byte, error) { - raw := json.RawMessage([]byte(s)) - - return json.Marshal(raw) -} - -func (cfg *ActionConfigString) UnmarshalJSON(data []byte) error { - if cfg == nil { - return errors.New("ActionConfigString: UnmarshalJSON on nil pointer") - } - - s := ActionConfigString(data) - *cfg = s - return nil -} - -func (s ActionConfigString) MarshalYAML() (interface{}, error) { - var innerMap map[string]interface{} - if err := yaml.Unmarshal([]byte(s), &innerMap); err != nil { - return nil, err - } - - return innerMap, nil -} - -func (p Policy) MarshalJSON() (string, error) { - return marshalJSON(p) -} - -func (p Rule) MarshalJSON() (string, error) { - return marshalJSON(p) -} - -func (p Action) MarshalJSON() (string, error) { - return marshalJSON(p) -} - -func marshalJSON(o any) (string, error) { - b := new(bytes.Buffer) - enc := json.NewEncoder(b) - enc.SetEscapeHTML(false) - - if err := enc.Encode(o); err != nil { - return "", err - } - - return b.String(), nil -} - -func (p Policy) MarshalYAML() (string, error) { - return marshalYAML(p) -} - -func (p Rule) MarshalYAML() (string, error) { - return marshalYAML(p) -} - -func (p Action) MarshalYAML() (string, error) { - return marshalYAML(p) -} - -func marshalYAML(o any) (string, error) { - bytes, err := yaml.Marshal(o) - - if err != nil { - return "", err - } - - return string(bytes), nil -}