Skip to content

Commit

Permalink
Merge pull request #20 from jakkab/support-multiple-mutators
Browse files Browse the repository at this point in the history 
Support multiple mutators
  • Loading branch information
Piotr Mścichowski authored Aug 9, 2019
2 parents f384d51 + 2a6d779 commit 5fbc214
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 133 deletions.
32 changes: 18 additions & 14 deletions api/v1alpha1/rule_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type RuleSpec struct {
Match *Match `json:"match"`
Authenticators []*Authenticator `json:"authenticators,omitempty"`
Authorizer *Authorizer `json:"authorizer,omitempty"`
Mutator *Mutator `json:"mutator,omitempty"`
Mutators []*Mutator `json:"mutators,omitempty"`
}

// Validation defines the validation state of Rule
Expand Down Expand Up @@ -152,30 +152,34 @@ func (rl RuleList) FilterNotValid() RuleList {
// ValidateWith uses provided validation configuration to check whether the rule have proper handlers set. Nil is a valid handler.
func (r Rule) ValidateWith(config validation.Config) error {

if r.Spec.Authenticators != nil {
var invalidHandlers []string

var invalidAuthenticators []string
if r.Spec.Authenticators != nil {
for _, authenticator := range r.Spec.Authenticators {
if valid := config.IsAuthenticatorValid(authenticator.Name); !valid {
invalidAuthenticators = append(invalidAuthenticators, authenticator.Name)
invalidHandlers = append(invalidHandlers, fmt.Sprintf("authenticator/%s", authenticator.Name))
}
}

if invalidAuthenticators != nil {
return fmt.Errorf("invalid authenticators: %s", invalidAuthenticators)
}
}

if r.Spec.Authorizer != nil {
if valid := config.IsAuthorizerValid(r.Spec.Authorizer.Name); !valid {
return fmt.Errorf("authorizer: %s is invalid", r.Spec.Authorizer.Name)
invalidHandlers = append(invalidHandlers, fmt.Sprintf("authorizer/%s", r.Spec.Authorizer.Name))
}
}
if r.Spec.Mutator != nil {
if valid := config.IsMutatorValid(r.Spec.Mutator.Name); !valid {
return fmt.Errorf("mutator: %s is invalid", r.Spec.Mutator.Name)

if r.Spec.Mutators != nil {
for _, m := range r.Spec.Mutators {
if valid := config.IsMutatorValid(m.Name); !valid {
invalidHandlers = append(invalidHandlers, m.Name)
}
}
}

if len(invalidHandlers) != 0 {
return fmt.Errorf("invalid handlers: %s, please check the configuration", invalidHandlers)
}

return nil
}

Expand All @@ -193,8 +197,8 @@ func (r Rule) ToRuleJSON() *RuleJSON {
if ruleJSON.Authorizer == nil {
ruleJSON.Authorizer = &Authorizer{denyHandler}
}
if ruleJSON.Mutator == nil {
ruleJSON.Mutator = &Mutator{noopHandler}
if ruleJSON.Mutators == nil {
ruleJSON.Mutators = []*Mutator{{noopHandler}}
}

if ruleJSON.Upstream.PreserveHost == nil {
Expand Down
181 changes: 97 additions & 84 deletions api/v1alpha1/rule_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package v1alpha1

import (
"fmt"
"github.com/ory/oathkeeper-maester/internal/validation"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -51,16 +52,24 @@ var (
"authorizer": {
"handler": "deny"
},
"mutator": {
"handler": "handler2",
"config": {
"key1": [
"val1",
"val2",
"val3"
]
"mutators": [
{
"handler": "handler1",
"config": {
"key1": "val1"
}
},
{
"handler": "handler2",
"config": {
"key1": [
"val1",
"val2",
"val3"
]
}
}
}
]
},
{
"upstream": {
Expand Down Expand Up @@ -96,9 +105,11 @@ var (
"authorizer": {
"handler": "deny"
},
"mutator": {
"handler": "noop"
}
"mutators": [
{
"handler": "noop"
}
]
},
{
"upstream": {
Expand All @@ -124,9 +135,11 @@ var (
"key1": "val1"
}
},
"mutator": {
"handler": "noop"
}
"mutators": [
{
"handler": "noop"
}
]
}
]`

Expand Down Expand Up @@ -179,7 +192,7 @@ func TestToOathkeeperRules(t *testing.T) {
newBoolPtr(true),
[]*Authenticator{&Authenticator{h1}},
nil,
&Mutator{h2})
[]*Mutator{{h1}, {h2}})

rule2 := newRule(
"foo2",
Expand Down Expand Up @@ -217,74 +230,77 @@ func TestToOathkeeperRules(t *testing.T) {

func TestToRuleJson(t *testing.T) {

t.Run("Should convert a Rule to JSON Rule", func(t *testing.T) {

var actual *RuleJSON
var testHandler = newHandler("test-handler", "")
var testRule = newRule(
"r1",
"test",
"https://upstream.url",
"https://match.this/url",
newStringPtr("/strip/me"),
nil,
nil,
nil,
nil)

t.Run("If no handlers have been specified, it should generate an ID and add default values for missing handlers", func(t *testing.T) {

//when
actual = testRule.ToRuleJSON()

//then
assert.Equal(t, "r1.test", actual.ID)

assertHasDefaultAuthenticator(t, actual)

require.NotNil(t, actual.RuleSpec.Authorizer)
assert.Equal(t, denyHandler, actual.RuleSpec.Authorizer.Handler)

require.NotNil(t, actual.RuleSpec.Mutator)
assert.Equal(t, noopHandler, actual.RuleSpec.Mutator.Handler)

assert.False(t, *actual.RuleSpec.Upstream.PreserveHost)
})

t.Run("If one handler has been provided, it should generate an ID, rewrite the provided handler and add default values for missing handlers", func(t *testing.T) {

//given
testRule.Spec.Mutator = &Mutator{testHandler}
assert := assert.New(t)

var actual *RuleJSON
var testHandler = newHandler("test-handler", "")
var testHandler2 = newHandler("test-handler2", "")

for k, tc := range []struct {
desc string
r *Rule
extra func(json *RuleJSON)
}{

{
"If no handlers have been specified, it should generate an ID and add default values for missing handlers",
newStaticRule(nil, nil, nil),
func(r *RuleJSON) {
assert.Equal(unauthorizedHandler, r.Authenticators[0].Handler)
assert.Equal(denyHandler, r.Authorizer.Handler)
assert.Equal(noopHandler, r.Mutators[0].Handler)
},
},
{
"If one handler type has been provided, it should generate an ID, rewrite the provided handler and add default values for missing handlers",
newStaticRule(nil, nil, []*Mutator{{testHandler}}),
func(r *RuleJSON) {
assert.Equal(unauthorizedHandler, r.Authenticators[0].Handler)
assert.Equal(denyHandler, r.Authorizer.Handler)
assert.Equal(testHandler, r.Mutators[0].Handler)
},
},
{
"If all handler types are defined, it should generate an ID and rewrite the handlers",
newStaticRule([]*Authenticator{{testHandler}}, &Authorizer{testHandler}, []*Mutator{{testHandler}}),
func(r *RuleJSON) {
assert.Equal(testHandler, r.Authenticators[0].Handler)
assert.Equal(testHandler, r.Authorizer.Handler)
assert.Equal(testHandler, r.Mutators[0].Handler)
},
},
{
"if multiple authentication and/or mutation handlers have been provided, it should rewrite all of them",
newStaticRule([]*Authenticator{{testHandler}, {testHandler2}}, nil, []*Mutator{{testHandler}, {testHandler2}}),
func(r *RuleJSON) {
assert.Equal(testHandler, r.Authenticators[0].Handler)
assert.Equal(testHandler2, r.Authenticators[1].Handler)
assert.Equal(testHandler, r.Mutators[0].Handler)
assert.Equal(testHandler2, r.Mutators[1].Handler)
assert.Equal(denyHandler, r.Authorizer.Handler)
},
},
} {
t.Run(fmt.Sprintf("case %d: %s", k, tc.desc), func(t *testing.T) {

//when
actual = testRule.ToRuleJSON()
actual = tc.r.ToRuleJSON()

//then
assert.Equal(t, "r1.test", actual.ID)

assertHasDefaultAuthenticator(t, actual)
assert.Equal("r1.test", actual.ID)

require.NotNil(t, actual.RuleSpec.Authenticators)
require.NotNil(t, actual.RuleSpec.Authorizer)
assert.Equal(t, denyHandler, actual.RuleSpec.Authorizer.Handler)

require.NotNil(t, actual.RuleSpec.Mutator)
assert.Equal(t, testHandler, actual.RuleSpec.Mutator.Handler)
})
require.NotNil(t, actual.RuleSpec.Mutators)

t.Run("If all handlers are defined, it should generate an ID and rewrite the entire spec", func(t *testing.T) {
require.NotEmpty(t, actual.RuleSpec.Authenticators)
require.NotEmpty(t, actual.RuleSpec.Mutators)

//given
testRule.Spec.Authenticators = []*Authenticator{{testHandler}}
testRule.Spec.Authorizer = &Authorizer{testHandler}

//when
actual = testRule.ToRuleJSON()
//testcase-specific assertions
tc.extra(actual)

//then
assert.Equal(t, "r1.test", actual.ID)
assert.Equal(t, testRule.Spec, actual.RuleSpec)
})
})
}
}

func TestValidateWith(t *testing.T) {
Expand Down Expand Up @@ -324,7 +340,7 @@ func TestValidateWith(t *testing.T) {
//given
rule.Spec.Authenticators = []*Authenticator{{testHandler}}
rule.Spec.Authorizer = &Authorizer{testHandler}
rule.Spec.Mutator = &Mutator{testHandler}
rule.Spec.Mutators = []*Mutator{{testHandler}}

//when
validationError = rule.ValidateWith(validationConfig)
Expand Down Expand Up @@ -373,7 +389,11 @@ func TestFilterNotValid(t *testing.T) {
})
}

func newRule(name, namespace, upstreamURL, matchURL string, stripURLPath *string, preserveURLHost *bool, authenticators []*Authenticator, authorizer *Authorizer, mutator *Mutator) *Rule {
func newStaticRule(authenticators []*Authenticator, authorizer *Authorizer, mutators []*Mutator) *Rule {
return newRule("r1", "test", "", "", newStringPtr(""), newBoolPtr(false), authenticators, authorizer, mutators)
}

func newRule(name, namespace, upstreamURL, matchURL string, stripURLPath *string, preserveURLHost *bool, authenticators []*Authenticator, authorizer *Authorizer, mutators []*Mutator) *Rule {

spec := RuleSpec{
Upstream: &Upstream{
Expand All @@ -387,7 +407,7 @@ func newRule(name, namespace, upstreamURL, matchURL string, stripURLPath *string
},
Authenticators: authenticators,
Authorizer: authorizer,
Mutator: mutator,
Mutators: mutators,
}

return &Rule{
Expand Down Expand Up @@ -431,10 +451,3 @@ func newBoolPtr(b bool) *bool {
func newStringPtr(s string) *string {
return &s
}

func assertHasDefaultAuthenticator(t *testing.T, actual *RuleJSON) {
require.NotNil(t, actual.RuleSpec.Authenticators)
require.NotEmpty(t, actual.RuleSpec.Authenticators)
require.Len(t, actual.RuleSpec.Authenticators, 1)
assert.Equal(t, unauthorizedHandler, actual.RuleSpec.Authenticators[0].Handler)
}
14 changes: 10 additions & 4 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,16 @@ func (in *RuleSpec) DeepCopyInto(out *RuleSpec) {
*out = new(Authorizer)
(*in).DeepCopyInto(*out)
}
if in.Mutator != nil {
in, out := &in.Mutator, &out.Mutator
*out = new(Mutator)
(*in).DeepCopyInto(*out)
if in.Mutators != nil {
in, out := &in.Mutators, &out.Mutators
*out = make([]*Mutator, len(*in))
for i := range *in {
if (*in)[i] != nil {
in, out := &(*in)[i], &(*out)[i]
*out = new(Mutator)
(*in).DeepCopyInto(*out)
}
}
}
}

Expand Down
26 changes: 14 additions & 12 deletions config/crd/bases/oathkeeper.ory.sh_rules.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,18 +427,20 @@ spec:
- url
- methods
type: object
mutator:
properties:
config:
description: Config configures the handler. Configuration keys vary
per handler.
type: object
handler:
description: Name is the name of a handler
type: string
required:
- handler
type: object
mutators:
items:
properties:
config:
description: Config configures the handler. Configuration keys
vary per handler.
type: object
handler:
description: Name is the name of a handler
type: string
required:
- handler
type: object
type: array
upstream:
properties:
preserveHost:
Expand Down
7 changes: 6 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@ go 1.12
require (
github.com/avast/retry-go v2.4.1+incompatible
github.com/bitly/go-simplejson v0.5.0
github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 // indirect
github.com/go-logr/logr v0.1.0
github.com/onsi/ginkgo v1.8.0
github.com/onsi/gomega v1.5.0
github.com/ory/oathkeeper-k8s-controller v0.0.1-beta.2
github.com/stretchr/testify v1.3.0
golang.org/x/net v0.0.0-20190620200207-3b0461eec859 // indirect
golang.org/x/sys v0.0.0-20190429190828-d89cdac9e872 // indirect
golang.org/x/text v0.3.2 // indirect
golang.org/x/tools v0.0.0-20190501045030-23463209683d // indirect
gopkg.in/yaml.v2 v2.2.2 // indirect
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible
sigs.k8s.io/controller-runtime v0.2.0-beta.2
sigs.k8s.io/kind v0.4.0 // indirect
)
Loading

0 comments on commit 5fbc214

Please sign in to comment.