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

IDefinedAggregator can't handle error codes from downstream requests #890

Closed
jasongm86 opened this issue May 15, 2019 · 0 comments
Closed

Comments

@jasongm86
Copy link
Contributor

jasongm86 commented May 15, 2019

Expected Behavior / New Feature

I'm using a custom aggregator and have authentication on the downstream requests. I'm struggling to find a way to handle authentication errors because the IDefinedAggregator takes a list of type DownstreamResponse which in the case where the downstream request is unauthenticated, these responses will be null and this isn't handled before .Aggregate() is called on the custom defined aggregator (for good reason I suppose).

Actual Behavior / Motivation for New Feature

I'd either expect to be able to handle the Ocelot error codes from the DownstreamContext in the custom aggregator or in the event that one or more of the downstream requests is unauthenticated, that UserDefinedResponseAggregator handles that before passing the responses to the aggregator. The former would obviously be better but I'm looking for opinions because it looks to me like anyone using a custom aggregator with authentication should have run into this problem, although I can't see any previous issues on GitHub. Happy to contribute and implement if I get some feedback and validation so raising this issue in the first instance.

Steps to Reproduce the Problem

{
  "ReRoutes": [
    {
      "DownstreamPathTemplate": "whoami/firstname",
      "UpstreamPathTemplate": "/firstname",
      "UpstreamHttpMethod": [
        "Get"
      ],
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "someservice",
          "Port": 80
        }
      ],
      "Key": "FirstName",
      "AuthenticationOptions": {
        "AuthenticationProviderKey": "JwtBearer"
      }
    },
    {
      "DownstreamPathTemplate": "whoami/lastname",
      "UpstreamPathTemplate": "/lastname",
      "UpstreamHttpMethod": [
        "Get"
      ],
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "someservice",
          "Port": 80
        }
      ],
      "Key": "LastName",
      "AuthenticationOptions": {
        "AuthenticationProviderKey": "JwtBearer"
      }
    }
  ],
  "Aggregates": [
    {
      "ReRouteKeys": [
        "FirstName",
        "LastName"
      ],
      "UpstreamPathTemplate": "/whoami",
      "Aggregator": "MyDefinedAggregator"
    }
  ],
  "GlobalConfiguration": {
    "RequestIdKey": "OcRequestId"
  }
}```
@jasongm86 jasongm86 changed the title IDefinedAggregator can't handle error codes for downstream requests IDefinedAggregator can't handle error codes from downstream requests May 15, 2019
thiagoloureiro pushed a commit that referenced this issue May 20, 2019
…tream requests (#892)

* Release/13.2.0 (#834)

* Fix formatting in getting started page (#752)

* updated release docs (#745)

* Update README.md (#756)

Fixed typo "Ocleot"

* Fixed typo there => their (#763)

* Some Typo fixes (#765)

* Typo algorythm => algorithm (#764)

* Typo querystring => query string (#766)

* Typo usual => usually (#767)

* Typos (#768)

* kubernetes provider (#772)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* fix issue #661 for Advanced aggregations (#704)

* Add Advanced Aggregation Feature

* fix overwrite error

* distinct data for better performance

* remove constructor parameter

* fix tests issue

* fix tests

* fix tests issue

* Add UnitTest and AcceptanceTest

* fix responseKeys typo

* Update SimpleJsonResponseAggregator.cs

* change port

* Fix code example for SSL Errors (#780)

DangerousAcceptAnyServerCertificateValidator has to be set to "true" to disable certification validation, not "false".

* Changed wording for ease of reading (#776)

Just some wording changes for clarification.

* Ignore response content if null (fix #785) (#786)

* fix bug #791 (#795)

* Update loadbalancer.rst (#796)

* UriBuilder - remove leading question mark #747 (#794)

* Update qualityofservice.rst (#801)

Tiny typo

* K8s package (#804)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* feat: publish package Ocelot.Provider.Kubernetes

* Okta integration (#807)

Okta integration

* update cliamsParser (#798)

* update cliamsParser

* update using

* IOcelotBuilder opens the IMvcCoreBuilder property for easy customization (#790)

* IOcelotBuilder opens the IMvcCoreBuilder property for easy customization

* Adjustment code

* nuget package (#809)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* feat: publish package Ocelot.Provider.Kubernetes

* feat : nuget package

* fix: Namesapce Spelling wrong

* fix:Namesapce Spelling Wrong

* Fix: errors when using rate limiting (#811)

* Fix: errors when using rate limiting
Add: QuotaExceededError class for requesting too much
Add: QuotaExceededError error code
Add: Add an error when limit is reached
Reflact: Extract GetResponseMessage method for getting default or configured response message for requ

* Fix: modify check_we_have_considered_all_errors_in_these_tests for adding a new OcelotErrorCode

* added missing COPY csproj files (#821)

* Add note on In-Process hosting (#816)

When using ASP.NET Core 2.2 with In-Process hosting in IIS it's important to use .UseIIS() instead of .UseIISIntegration().

* Fix bug: (#810)

If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* Fixed Dockerfile (missing Kubernetes)

* Revert "Fix bug: (#810)" (#823)

This reverts commit 19c80af.

* remove duplicate `IHttpRequester` register (#819)

* remove duplicate `IHttpRequester` register

* reserve the first

* fix HttpRequesterMiddleware does not call next bug (#830)

call next so that we can do something with the response, such as add some custom header etc...

* Removed Packing to fix issues, will be sorted out after create a nuget package on Nuget.Org (#831)

* Allows access to unpass node (#825)

* Fix bug:
If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* fix bug:
If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* Updated FluentValidations Nuget Package (#833)

* Removed Warnings

* Make the full DownstreamContext available to user defined aggregators

This allows error codes to be handled
thiagoloureiro added a commit that referenced this issue May 20, 2019
* Fixed Format Issue for Kubernetes ServiceDiscoveryProvider

* Fixes broken links (#858)

* Fix link to issue 262

* Fixes broken link to issue 340

* Fixed broken link to issue 340 (#857)

* Update information for Okta Authorization (#853)

* +dynamic claim variables (#855)

incl. tests

* IOcelotPipelineBuilder.Use(): Return IOcelotPipelineBuilder (#875)

Fixes #685

* Fix UpstreamHost checking when reroutes duplicate validation (#864)

* Format json in reame (#877)

Format json file in AdministrationApi ReadMe

* kubernetes use in cluster (#882)

* refactor :kubernetes use in cluster

* feat:delete KubeClient

* add more flexible method to config ocelot pipeline (#880)

* update k8s doc & samples (#885)

* refactor :kubernetes use in cluster

* feat:delete KubeClient

* feat :  update k8s doc & samples

* Update kubernetes.rst

* Fix/issue666 (#889)

* cache key now can generate from query string for request with Get Methods and request content for requests with post methods

* MD5Helper Added. OutputCacheMiddleware now can generate cache key using method, url and content

* unit test created for CacheKeyGenerator

* CacheKeyGenerator Registered in OcelotBuilder as singletone

* Fix issue #890 IDefinedAggregator can't handle error codes from downstream requests (#892)

* Release/13.2.0 (#834)

* Fix formatting in getting started page (#752)

* updated release docs (#745)

* Update README.md (#756)

Fixed typo "Ocleot"

* Fixed typo there => their (#763)

* Some Typo fixes (#765)

* Typo algorythm => algorithm (#764)

* Typo querystring => query string (#766)

* Typo usual => usually (#767)

* Typos (#768)

* kubernetes provider (#772)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* fix issue #661 for Advanced aggregations (#704)

* Add Advanced Aggregation Feature

* fix overwrite error

* distinct data for better performance

* remove constructor parameter

* fix tests issue

* fix tests

* fix tests issue

* Add UnitTest and AcceptanceTest

* fix responseKeys typo

* Update SimpleJsonResponseAggregator.cs

* change port

* Fix code example for SSL Errors (#780)

DangerousAcceptAnyServerCertificateValidator has to be set to "true" to disable certification validation, not "false".

* Changed wording for ease of reading (#776)

Just some wording changes for clarification.

* Ignore response content if null (fix #785) (#786)

* fix bug #791 (#795)

* Update loadbalancer.rst (#796)

* UriBuilder - remove leading question mark #747 (#794)

* Update qualityofservice.rst (#801)

Tiny typo

* K8s package (#804)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* feat: publish package Ocelot.Provider.Kubernetes

* Okta integration (#807)

Okta integration

* update cliamsParser (#798)

* update cliamsParser

* update using

* IOcelotBuilder opens the IMvcCoreBuilder property for easy customization (#790)

* IOcelotBuilder opens the IMvcCoreBuilder property for easy customization

* Adjustment code

* nuget package (#809)

* feat: Kubernetes ServiceDiscoveryProvider

* 编写k8s测试例子

* feat:fix kube config

* feat: remove port

* feat : complete the k8s test

* feat :  add kubeserviceDiscovery test

* feat : add kube provider unittest

* feat :add kubetnetes docs

how to use ocelot with kubetnetes docs

* keep the configuration as simple as possible, no qos, no cache

* fix: use http

* add PollingKubeServiceDiscovery

* feat : refactor logger

* feat : add  pollkube docs

* feat:Remove unnecessary code

* feat : code-block json

* feat: publish package Ocelot.Provider.Kubernetes

* feat : nuget package

* fix: Namesapce Spelling wrong

* fix:Namesapce Spelling Wrong

* Fix: errors when using rate limiting (#811)

* Fix: errors when using rate limiting
Add: QuotaExceededError class for requesting too much
Add: QuotaExceededError error code
Add: Add an error when limit is reached
Reflact: Extract GetResponseMessage method for getting default or configured response message for requ

* Fix: modify check_we_have_considered_all_errors_in_these_tests for adding a new OcelotErrorCode

* added missing COPY csproj files (#821)

* Add note on In-Process hosting (#816)

When using ASP.NET Core 2.2 with In-Process hosting in IIS it's important to use .UseIIS() instead of .UseIISIntegration().

* Fix bug: (#810)

If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* Fixed Dockerfile (missing Kubernetes)

* Revert "Fix bug: (#810)" (#823)

This reverts commit 19c80af.

* remove duplicate `IHttpRequester` register (#819)

* remove duplicate `IHttpRequester` register

* reserve the first

* fix HttpRequesterMiddleware does not call next bug (#830)

call next so that we can do something with the response, such as add some custom header etc...

* Removed Packing to fix issues, will be sorted out after create a nuget package on Nuget.Org (#831)

* Allows access to unpass node (#825)

* Fix bug:
If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* fix bug:
If the registered Consul node is unexpectedly down and not restarted immediately, other services should continue to find the registered service.

* Updated FluentValidations Nuget Package (#833)

* Removed Warnings

* Make the full DownstreamContext available to user defined aggregators

This allows error codes to be handled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant