Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple mutators #20

Merged
merged 1 commit into from
Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
32 changes: 18 additions & 14 deletions api/v1alpha1/rule_types.go
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
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
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
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
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