From 6fc53c830efd39abe8e248671274307c9d958ebd Mon Sep 17 00:00:00 2001 From: strekm Date: Thu, 19 Sep 2019 14:06:50 +0200 Subject: [PATCH] Multiple rules vs (#55) --- config/samples/valid.yaml | 16 +++- .../api_controller_integration_test.go | 4 +- internal/processing/helpers.go | 53 +----------- internal/processing/processing.go | 84 +++++++++++++++---- internal/processing/processing_test.go | 71 +++++++--------- 5 files changed, 112 insertions(+), 116 deletions(-) diff --git a/config/samples/valid.yaml b/config/samples/valid.yaml index f01f81da5..958c4f96c 100644 --- a/config/samples/valid.yaml +++ b/config/samples/valid.yaml @@ -146,7 +146,7 @@ spec: port: 8080 host: foo.bar rules: - - path: /.img + - path: /img methods: ["GET"] mutators: [] accessStrategies: @@ -154,9 +154,19 @@ spec: config: trusted_issuers: ["http://dex.kyma.local"] required_scope: [] - - path: /.headers + - path: /headers methods: ["GET"] accessStrategies: - handler: oauth2_introspection config: - required_scope: ["foo"] \ No newline at end of file + required_scope: ["foo"] + - path: /favicon + methods: ["GET"] + accessStrategies: + - handler: allow + - path: /status/.* + methods: ["GET"] + accessStrategies: + - handler: noop + mutators: + - handler: noop \ No newline at end of file diff --git a/controllers/api_controller_integration_test.go b/controllers/api_controller_integration_test.go index 058246f0b..4ba9b0e01 100644 --- a/controllers/api_controller_integration_test.go +++ b/controllers/api_controller_integration_test.go @@ -288,7 +288,7 @@ var _ = Describe("APIRule Controller", func() { Expect(vs.Spec.Gateways).To(HaveLen(1)) Expect(vs.Spec.Gateways[0]).To(Equal(testGatewayURL)) //Spec.HTTP - Expect(vs.Spec.HTTP).To(HaveLen(1)) + Expect(vs.Spec.HTTP).To(HaveLen(2)) ////// HTTP.Match[] Expect(vs.Spec.HTTP[0].Match).To(HaveLen(1)) /////////// Match[].URI @@ -296,7 +296,7 @@ var _ = Describe("APIRule Controller", func() { Expect(vs.Spec.HTTP[0].Match[0].URI.Exact).To(BeEmpty()) Expect(vs.Spec.HTTP[0].Match[0].URI.Prefix).To(BeEmpty()) Expect(vs.Spec.HTTP[0].Match[0].URI.Suffix).To(BeEmpty()) - Expect(vs.Spec.HTTP[0].Match[0].URI.Regex).To(Equal("/.*")) + Expect(vs.Spec.HTTP[0].Match[0].URI.Regex).To(Equal("/img")) Expect(vs.Spec.HTTP[0].Match[0].Scheme).To(BeNil()) Expect(vs.Spec.HTTP[0].Match[0].Method).To(BeNil()) Expect(vs.Spec.HTTP[0].Match[0].Authority).To(BeNil()) diff --git a/internal/processing/helpers.go b/internal/processing/helpers.go index 051e300e7..b234e24e8 100644 --- a/internal/processing/helpers.go +++ b/internal/processing/helpers.go @@ -7,14 +7,9 @@ import ( "github.com/kyma-incubator/api-gateway/internal/builders" rulev1alpha1 "github.com/ory/oathkeeper-maester/api/v1alpha1" k8sMeta "k8s.io/apimachinery/pkg/apis/meta/v1" - networkingv1alpha3 "knative.dev/pkg/apis/istio/v1alpha3" ) func prepareAccessRule(api *gatewayv1alpha1.APIRule, ar *rulev1alpha1.Rule, rule gatewayv1alpha1.Rule, ruleInd int, accessStrategies []*rulev1alpha1.Authenticator) *rulev1alpha1.Rule { - ar.ObjectMeta.OwnerReferences = []k8sMeta.OwnerReference{generateOwnerRef(api)} - ar.ObjectMeta.Name = fmt.Sprintf("%s-%s-%d", api.ObjectMeta.Name, *api.Spec.Service.Name, ruleInd) - ar.ObjectMeta.Namespace = api.ObjectMeta.Namespace - return builders.AccessRule().From(ar). Spec(builders.AccessRuleSpec().From(generateAccessRuleSpec(api, rule, accessStrategies))). Get() @@ -46,30 +41,12 @@ func generateAccessRuleSpec(api *gatewayv1alpha1.APIRule, rule gatewayv1alpha1.R Mutators(builders.Mutators().From(rule.Mutators)).Get() } -func generateVirtualService(api *gatewayv1alpha1.APIRule, destinationHost string, destinationPort uint32, path string) *networkingv1alpha3.VirtualService { - //TODO implement support for "allow" & oathkeeper authn (many http objects) - virtualServiceName := fmt.Sprintf("%s-%s", api.ObjectMeta.Name, *api.Spec.Service.Name) - ownerRef := generateOwnerRef(api) - return builders.VirtualService(). - Name(virtualServiceName). - Namespace(api.ObjectMeta.Namespace). - Owner(builders.OwnerReference().From(&ownerRef)). - Spec( - builders.VirtualServiceSpec(). - Host(*api.Spec.Service.Host). - Gateway(*api.Spec.Gateway). - HTTP(builders.HTTPRoute(). - Match(builders.MatchRequest().URI().Regex("/.*")). - Route(builders.RouteDestination().Host(destinationHost).Port(destinationPort)))). - Get() -} - func isSecured(rule gatewayv1alpha1.Rule) bool { if len(rule.Mutators) > 0 { return true } for _, strat := range rule.AccessStrategies { - if strat.Name != "noop" { + if strat.Name != "allow" { return true } } @@ -85,31 +62,3 @@ func generateOwnerRef(api *gatewayv1alpha1.APIRule) k8sMeta.OwnerReference { Controller(true). Get() } - -func generateObjectMeta(api *gatewayv1alpha1.APIRule) k8sMeta.ObjectMeta { - ownerRef := generateOwnerRef(api) - return *builders.ObjectMeta(). - Name(fmt.Sprintf("%s-%s", api.ObjectMeta.Name, *api.Spec.Service.Name)). - Namespace(api.ObjectMeta.Namespace). - OwnerReference(builders.OwnerReference().From(&ownerRef)). - Get() -} - -func prepareVirtualService(api *gatewayv1alpha1.APIRule, vs *networkingv1alpha3.VirtualService, destinationHost string, destinationPort uint32, path string) *networkingv1alpha3.VirtualService { - //TODO implement support for "allow" & oathkeeper authn (many http objects) - virtualServiceName := fmt.Sprintf("%s-%s", api.ObjectMeta.Name, *api.Spec.Service.Name) - ownerRef := generateOwnerRef(api) - - return builders.VirtualService().From(vs). - Name(virtualServiceName). - Namespace(api.ObjectMeta.Namespace). - Owner(builders.OwnerReference().From(&ownerRef)). - Spec( - builders.VirtualServiceSpec(). - Host(*api.Spec.Service.Host). - Gateway(*api.Spec.Gateway). - HTTP(builders.HTTPRoute(). - Match(builders.MatchRequest().URI().Regex("/.*")). - Route(builders.RouteDestination().Host(destinationHost).Port(destinationPort)))). - Get() -} diff --git a/internal/processing/processing.go b/internal/processing/processing.go index 244dc0ba0..fea29cc28 100644 --- a/internal/processing/processing.go +++ b/internal/processing/processing.go @@ -3,6 +3,7 @@ package processing import ( "context" "fmt" + "github.com/kyma-incubator/api-gateway/internal/builders" "github.com/go-logr/logr" gatewayv1alpha1 "github.com/kyma-incubator/api-gateway/api/v1alpha1" @@ -37,26 +38,19 @@ func NewFactory(vsClient *istioClient.VirtualService, arClient *oryClient.Access // Run ? func (f *Factory) Run(ctx context.Context, api *gatewayv1alpha1.APIRule) error { - var destinationHost string - var destinationPort uint32 var err error for i, rule := range api.Spec.Rules { if isSecured(rule) { - destinationHost = f.oathkeeperSvc - destinationPort = f.oathkeeperSvcPort - } else { - destinationHost = fmt.Sprintf("%s.%s.svc.cluster.local", *api.Spec.Service.Name, api.ObjectMeta.Namespace) - destinationPort = *api.Spec.Service.Port - } - // Create one AR per path - err = f.processAR(ctx, api, api.Spec.Rules[i], i, rule.AccessStrategies) - if err != nil { - return err + // Create one AR per path + err = f.processAR(ctx, api, api.Spec.Rules[i], i, rule.AccessStrategies) + if err != nil { + return err + } } } // Compile list of paths, create one VS - err = f.processVS(ctx, api, destinationHost, destinationPort) + err = f.processVS(ctx, api) if err != nil { return err } @@ -103,17 +97,17 @@ func (f *Factory) getAccessRule(ctx context.Context, api *gatewayv1alpha1.APIRul return ar, nil } -func (f *Factory) processVS(ctx context.Context, api *gatewayv1alpha1.APIRule, destinationHost string, destinationPort uint32) error { +func (f *Factory) processVS(ctx context.Context, api *gatewayv1alpha1.APIRule) error { oldVS, err := f.getVirtualService(ctx, api) if err != nil { return err } if oldVS != nil { - newVS := prepareVirtualService(api, oldVS, destinationHost, destinationPort, api.Spec.Rules[0].Path) + newVS := f.prepareVirtualService(api, oldVS) return f.updateVirtualService(ctx, newVS) } - vs := generateVirtualService(api, destinationHost, destinationPort, api.Spec.Rules[0].Path) + vs := f.generateVirtualService(api) return f.createVirtualService(ctx, vs) } @@ -138,3 +132,61 @@ func (f *Factory) processAR(ctx context.Context, api *gatewayv1alpha1.APIRule, r } return nil } + +func (f *Factory) prepareVirtualService(api *gatewayv1alpha1.APIRule, vs *networkingv1alpha3.VirtualService) *networkingv1alpha3.VirtualService { + vsSpecBuilder := builders.VirtualServiceSpec() + vsSpecBuilder.Host(*api.Spec.Service.Host) + vsSpecBuilder.Gateway(*api.Spec.Gateway) + + for _, rule := range api.Spec.Rules { + httpRouteBuilder := builders.HTTPRoute() + + if isSecured(rule) { + httpRouteBuilder.Route(builders.RouteDestination().Host(f.oathkeeperSvc).Port(f.oathkeeperSvcPort)) + } else { + destinationHost := fmt.Sprintf("%s.%s.svc.cluster.local", *api.Spec.Service.Name, api.ObjectMeta.Namespace) + httpRouteBuilder.Route(builders.RouteDestination().Host(destinationHost).Port(*api.Spec.Service.Port)) + } + + httpRouteBuilder.Match(builders.MatchRequest().URI().Regex(rule.Path)) + vsSpecBuilder.HTTP(httpRouteBuilder) + } + + vsBuilder := builders.VirtualService(). + From(vs). + Spec(vsSpecBuilder) + + return vsBuilder.Get() +} + +func (f *Factory) generateVirtualService(api *gatewayv1alpha1.APIRule) *networkingv1alpha3.VirtualService { + virtualServiceName := fmt.Sprintf("%s-%s", api.ObjectMeta.Name, *api.Spec.Service.Name) + ownerRef := generateOwnerRef(api) + + vsSpecBuilder := builders.VirtualServiceSpec() + vsSpecBuilder.Host(*api.Spec.Service.Host) + vsSpecBuilder.Gateway(*api.Spec.Gateway) + + for _, rule := range api.Spec.Rules { + httpRouteBuilder := builders.HTTPRoute() + + if isSecured(rule) { + httpRouteBuilder.Route(builders.RouteDestination().Host(f.oathkeeperSvc).Port(f.oathkeeperSvcPort)) + } else { + destinationHost := fmt.Sprintf("%s.%s.svc.cluster.local", *api.Spec.Service.Name, api.ObjectMeta.Namespace) + httpRouteBuilder.Route(builders.RouteDestination().Host(destinationHost).Port(*api.Spec.Service.Port)) + } + + httpRouteBuilder.Match(builders.MatchRequest().URI().Regex(rule.Path)) + vsSpecBuilder.HTTP(httpRouteBuilder) + } + + vsBuilder := builders.VirtualService(). + Name(virtualServiceName). + Namespace(api.ObjectMeta.Namespace). + Owner(builders.OwnerReference().From(&ownerRef)) + + vsBuilder.Spec(vsSpecBuilder) + + return vsBuilder.Get() +} diff --git a/internal/processing/processing_test.go b/internal/processing/processing_test.go index 9a3f2bd89..f2da2b6da 100644 --- a/internal/processing/processing_test.go +++ b/internal/processing/processing_test.go @@ -79,14 +79,15 @@ func TestCreateVS_NoOp(t *testing.T) { strategies := []*rulev1alpha1.Authenticator{ { Handler: &rulev1alpha1.Handler{ - Name: "noop", + Name: "allow", }, }, } apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) + f := &Factory{oathkeeperSvcPort: 1234, oathkeeperSvc: "fake.oathkeeper"} - vs := generateVirtualService(apiRule, serviceName+"."+apiNamespace+".svc.cluster.local", servicePort, apiPath) + vs := f.generateVirtualService(apiRule) assert.Equal(len(vs.Spec.Gateways), 1) assert.Equal(vs.Spec.Gateways[0], apiGateway) @@ -133,8 +134,9 @@ func TestCreateVS_JWT(t *testing.T) { } apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) + f := &Factory{oathkeeperSvcPort: 4455, oathkeeperSvc: "test-oathkeeper"} - vs := generateVirtualService(apiRule, "test-oathkeeper", 4455, apiPath) + vs := f.generateVirtualService(apiRule) assert.Equal(len(vs.Spec.Gateways), 1) assert.Equal(vs.Spec.Gateways[0], apiGateway) @@ -183,13 +185,9 @@ func TestPrepareAR_JWT(t *testing.T) { apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) oldAR := generateAccessRule(apiRule, apiRule.Spec.Rules[0], 0, []*rulev1alpha1.Authenticator{strategies[0]}) - oldAR.ObjectMeta.Generation = int64(15) - oldAR.ObjectMeta.Name = "mst" - + oldAROnwerRef := oldAR.OwnerReferences[0] newAR := prepareAccessRule(apiRule, oldAR, apiRule.Spec.Rules[0], 0, []*rulev1alpha1.Authenticator{strategies[0]}) - assert.Equal(newAR.ObjectMeta.Generation, int64(15)) - assert.Equal(len(newAR.Spec.Authenticators), 1) assert.Equal(newAR.Spec.Authenticators[0].Name, "jwt") assert.NotEmpty(newAR.Spec.Authenticators[0].Config) @@ -207,11 +205,12 @@ func TestPrepareAR_JWT(t *testing.T) { assert.Equal(newAR.ObjectMeta.Name, apiName+"-"+serviceName+"-0") assert.Equal(newAR.ObjectMeta.Namespace, apiNamespace) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].APIVersion, apiAPIVersion) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Kind, apiKind) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Name, apiName) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].UID, apiUID) + assert.Equal(len(newAR.ObjectMeta.OwnerReferences), 1) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].APIVersion, oldAROnwerRef.APIVersion) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Kind, oldAROnwerRef.Kind) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Name, oldAROnwerRef.Name) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].UID, oldAROnwerRef.UID) } func TestGenerateAR_JWT(t *testing.T) { @@ -283,7 +282,8 @@ func TestGenerateVS_OAUTH(t *testing.T) { } apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) - vs := generateVirtualService(apiRule, "test-oathkeeper", 4455, apiRule.Spec.Rules[0].Path) + f := &Factory{oathkeeperSvcPort: 4455, oathkeeperSvc: "test-oathkeeper"} + vs := f.generateVirtualService(apiRule) assert.Equal(len(vs.Spec.Gateways), 1) assert.Equal(vs.Spec.Gateways[0], apiGateway) @@ -328,15 +328,11 @@ func TestPrepareVS_OAUTH(t *testing.T) { } apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) + f := &Factory{oathkeeperSvcPort: 4455, oathkeeperSvc: "test-oathkeeper"} - oldVS := generateVirtualService(apiRule, "test-oathkeeper", 4455, apiRule.Spec.Rules[0].Path) - - oldVS.ObjectMeta.Generation = int64(15) - oldVS.ObjectMeta.Name = "mst" - - newVS := prepareVirtualService(apiRule, oldVS, "test-oathkeeper", 4455, apiRule.Spec.Rules[0].Path) - - assert.Equal(newVS.ObjectMeta.Generation, int64(15)) + oldVS := f.generateVirtualService(apiRule) + oldVSOnwerRef := oldVS.OwnerReferences[0] + newVS := f.prepareVirtualService(apiRule, oldVS) assert.Equal(len(newVS.Spec.Gateways), 1) assert.Equal(newVS.Spec.Gateways[0], apiGateway) @@ -351,14 +347,11 @@ func TestPrepareVS_OAUTH(t *testing.T) { assert.Equal(int(newVS.Spec.HTTP[0].Route[0].Destination.Port.Number), 4455) assert.Equal(newVS.Spec.HTTP[0].Match[0].URI.Regex, apiPath) - assert.Equal(newVS.ObjectMeta.Name, apiName+"-"+serviceName) - assert.Equal(newVS.ObjectMeta.Namespace, apiNamespace) - - assert.Equal(newVS.ObjectMeta.OwnerReferences[0].APIVersion, apiAPIVersion) - assert.Equal(newVS.ObjectMeta.OwnerReferences[0].Kind, apiKind) - assert.Equal(newVS.ObjectMeta.OwnerReferences[0].Name, apiName) - assert.Equal(newVS.ObjectMeta.OwnerReferences[0].UID, apiUID) - + assert.Equal(len(newVS.ObjectMeta.OwnerReferences), 1) + assert.Equal(newVS.ObjectMeta.OwnerReferences[0].APIVersion, oldVSOnwerRef.APIVersion) + assert.Equal(newVS.ObjectMeta.OwnerReferences[0].Kind, oldVSOnwerRef.Kind) + assert.Equal(newVS.ObjectMeta.OwnerReferences[0].Name, oldVSOnwerRef.Name) + assert.Equal(newVS.ObjectMeta.OwnerReferences[0].UID, oldVSOnwerRef.UID) } func TestGenerateAR_OAUTH(t *testing.T) { @@ -430,14 +423,9 @@ func TestPreapreAR_OAUTH(t *testing.T) { apiRule := getAPIRuleFor(strategies, []*rulev1alpha1.Mutator{}) oldAR := generateAccessRule(apiRule, apiRule.Spec.Rules[0], 0, strategies) - - oldAR.ObjectMeta.Generation = int64(15) - oldAR.ObjectMeta.Name = "mst" - + oldAROnwerRef := oldAR.OwnerReferences[0] newAR := prepareAccessRule(apiRule, oldAR, apiRule.Spec.Rules[0], 0, strategies) - assert.Equal(newAR.ObjectMeta.Generation, int64(15)) - assert.Equal(len(newAR.Spec.Authenticators), 1) assert.NotEmpty(newAR.Spec.Authenticators[0].Config) assert.Equal(newAR.Spec.Authenticators[0].Name, "oauth2_introspection") @@ -452,12 +440,9 @@ func TestPreapreAR_OAUTH(t *testing.T) { assert.Equal(newAR.Spec.Upstream.URL, "http://example-service.some-namespace.svc.cluster.local:8080") - assert.Equal(newAR.ObjectMeta.Name, apiName+"-"+serviceName+"-0") - assert.Equal(newAR.ObjectMeta.Namespace, apiNamespace) - - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].APIVersion, apiAPIVersion) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Kind, apiKind) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Name, apiName) - assert.Equal(newAR.ObjectMeta.OwnerReferences[0].UID, apiUID) - + assert.Equal(len(newAR.ObjectMeta.OwnerReferences), 1) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].APIVersion, oldAROnwerRef.APIVersion) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Kind, oldAROnwerRef.Kind) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].Name, oldAROnwerRef.Name) + assert.Equal(newAR.ObjectMeta.OwnerReferences[0].UID, oldAROnwerRef.UID) }