diff --git a/api/v1alpha1/rule_types.go b/api/v1alpha1/rule_types.go index f9da522..14c1a8a 100644 --- a/api/v1alpha1/rule_types.go +++ b/api/v1alpha1/rule_types.go @@ -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 @@ -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 } @@ -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 { diff --git a/api/v1alpha1/rule_types_test.go b/api/v1alpha1/rule_types_test.go index e03da79..62c0515 100644 --- a/api/v1alpha1/rule_types_test.go +++ b/api/v1alpha1/rule_types_test.go @@ -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" @@ -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": { @@ -96,9 +105,11 @@ var ( "authorizer": { "handler": "deny" }, - "mutator": { - "handler": "noop" - } + "mutators": [ + { + "handler": "noop" + } + ] }, { "upstream": { @@ -124,9 +135,11 @@ var ( "key1": "val1" } }, - "mutator": { - "handler": "noop" - } + "mutators": [ + { + "handler": "noop" + } + ] } ]` @@ -179,7 +192,7 @@ func TestToOathkeeperRules(t *testing.T) { newBoolPtr(true), []*Authenticator{&Authenticator{h1}}, nil, - &Mutator{h2}) + []*Mutator{{h1}, {h2}}) rule2 := newRule( "foo2", @@ -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) { @@ -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) @@ -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{ @@ -387,7 +407,7 @@ func newRule(name, namespace, upstreamURL, matchURL string, stripURLPath *string }, Authenticators: authenticators, Authorizer: authorizer, - Mutator: mutator, + Mutators: mutators, } return &Rule{ @@ -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) -} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2cf2883..934e4c6 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -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) + } + } } } diff --git a/config/crd/bases/oathkeeper.ory.sh_rules.yaml b/config/crd/bases/oathkeeper.ory.sh_rules.yaml index d29461d..b346a49 100644 --- a/config/crd/bases/oathkeeper.ory.sh_rules.yaml +++ b/config/crd/bases/oathkeeper.ory.sh_rules.yaml @@ -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: diff --git a/go.mod b/go.mod index a0c1f57..cce2a1e 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index 23037c3..2252f74 100644 --- a/go.sum +++ b/go.sum @@ -115,6 +115,7 @@ github.com/onsi/gomega v1.5.0 h1:izbySO9zDPmjJ8rDjLvkA2zJHIo+HkYXHnf7eN7SSyo= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/ory/oathkeeper-k8s-controller v0.0.1-beta.2 h1:F7Q3FyYAjWBvViwWhYoSQJ+u931n1fePbMS5Hmwu8Nw= github.com/ory/oathkeeper-k8s-controller v0.0.1-beta.2/go.mod h1:nfAR0sttL71teMB2Cv8QHfLZYOQKto3SEt6oKgjwWbQ= +github.com/ory/oathkeeper-maester v0.0.1-beta.3/go.mod h1:wmb9va/b8djBJtIFhhZgQjJI7OZjCYW+kLyks6OuyUs= github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c h1:MUyE44mTvnI5A0xrxIxaMqoWFzPfQvtE2IWUollMDMs= github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c/go.mod h1:VyrYX9gd7irzKovcSS6BIIEwPRkP2Wm2m9ufcdFSJ34= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -134,6 +135,7 @@ github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.2.2 h1:J7U/N7eRtzjhs26d6GqMh2HBuXP8/Z64Densiiieafo= github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= +github.com/sirupsen/logrus v1.4.1 h1:GL2rEmy6nsikmW0r8opw9JIRScdMF5hA8cOYLH7In1k= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= diff --git a/tests/integration/rules_test.go b/tests/integration/rules_test.go index 679b2fc..64429bb 100644 --- a/tests/integration/rules_test.go +++ b/tests/integration/rules_test.go @@ -10,8 +10,8 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" oathkeeperv1alpha1 "github.com/ory/oathkeeper-maester/api/v1alpha1" + "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -398,15 +398,17 @@ func getRule1Json() string { "authorizer": { "handler": "allow" }, - "mutator": { - "handler": "header", - "config": { - "headers": { - "X-User": "{{ print .Subject }}", - "X-Some-Arbitrary-Data": "{{ print .Extra.some.arbitrary.data }}" + "mutators": [ + { + "handler": "header", + "config": { + "headers": { + "X-User": "{{ print .Subject }}", + "X-Some-Arbitrary-Data": "{{ print .Extra.some.arbitrary.data }}" + } } - } - } + } + ] } } ` @@ -443,12 +445,14 @@ func getRule2Json() string { "required_resource": "my:resource:foobar:foo:1234" } }, - "mutator": { - "handler": "id_token", - "config": { - "aud": ["audience1", "audience2"] + "mutators": [ + { + "handler": "id_token", + "config": { + "aud": ["audience1", "audience2"] + } } - } + ] } } ` diff --git a/tests/integration/validation.go b/tests/integration/validation.go index eba4045..ddb072f 100644 --- a/tests/integration/validation.go +++ b/tests/integration/validation.go @@ -12,14 +12,14 @@ import ( //`actual` is a representation of an entry from the ConfigMap handled by the Controller func validateRuleEquals(actual *json.Json, expected *json.Json) { Expect(actual).To(Equal(expected)) - expectOnlyKeys(actual, "id", "upstream", "match", "authenticators", "authorizer", "mutator") + expectOnlyKeys(actual, "id", "upstream", "match", "authenticators", "authorizer", "mutators") expectString(actual, "id") compareUpstreams(actual.Get("upstream"), expected.Get("upstream")) compareMatches(actual.Get("match"), expected.Get("match")) - compareAuthenticators(actual.Get("authenticators"), expected.Get("authenticators")) + compareHandlerArrays(actual.Get("authenticators"), expected.Get("authenticators")) compareHandlers(actual.Get("authorizer"), expected.Get("authorizer")) - compareHandlers(actual.Get("mutator"), expected.Get("mutator")) + compareHandlerArrays(actual.Get("mutators"), expected.Get("mutators")) } func compareUpstreams(actual *json.Json, expected *json.Json) { @@ -36,7 +36,7 @@ func compareMatches(actual *json.Json, expected *json.Json) { expectStringArray(actual, "methods") } -func compareAuthenticators(actual *json.Json, expected *json.Json) { +func compareHandlerArrays(actual *json.Json, expected *json.Json) { //both are equal Expect(actual).To(Equal(expected))