dapr icon indicating copy to clipboard operation
dapr copied to clipboard

Proposal: HTTP Pipeline isn't executed in service-to-service invocation

Open ewassef opened this issue 4 years ago • 9 comments

In what area(s)?

/area runtime

What version of Dapr?

1.6

Expected Behavior

If I am setting an HTTP pipeline in a configuration and the service is an HTTP service. I expect the svc-to-svc invocation to go through this pipeline. This seems to be omitted due to the grpc channel used for the execution. This should invoke the local appchannel AFTER the grpc pipeline to keep the functionality

Actual Behavior

Direct invocation to the *-dapr service or from within the container will execute the pipeline, any svc to svc invocation will not

Steps to Reproduce the Problem

I have asked this question in Discord (https://discord.com/channels/778680217417809931/846943333824987156/959559729359753216) I have asked this question in StackOverflow (https://stackoverflow.com/questions/71711579/opa-middleware-with-dapr-not-invoking) I have a repo with repo scripts (https://github.com/ewassef/opa-dapr)

Release Note

RELEASE NOTE: FIX Bug in runtime. HTTP Pipeline invocation executes as designed

@yaron2 FYI

ewassef avatar Apr 05 '22 14:04 ewassef

Sidecar to Sidecar communication is always done via gRPC. You have to choice whether to call your sidecar with gRPC or HTTP, but for sidecar to sidecar communication it is gRPC only.

In a Kubernetes setting Dapr HTTP communication is only intra-pod. Are you using the Dapr middleware? https://docs.dapr.io/developing-applications/middleware/

berndverst avatar Apr 06 '22 18:04 berndverst

@berndverst pls take a look at the repo above. This is using the sample from the dapr.io website and a service calling another service that has an HTTP Pipeline configured.

ewassef avatar Apr 06 '22 20:04 ewassef

@yaron2 @berndverst any luck?

ewassef avatar Apr 15 '22 19:04 ewassef

I am facing the same problem.

Configured 2 services: Service A and Service B where service A is talking to service B Service B is configured with a DAPR middle ware. This middle ware is an OPA configuration witch is checking an AD access token.

When making direct calls to the service B from an other pod or outside of the cluster the middle-ware gets called. : | Service A | --> | dapr ---> service B| With these I have to give a valid JWT token to be able to access Service B.

When I make the call via dapr:
| service A -> dapr | ---> | dapr ---> service B |
the middle ware is not called. It only relies on the dapr configuration to allow or deny access. I does not mather if my JWT token is valid or not.

360build avatar May 20 '22 08:05 360build

That's correct. I support changing this to the behavior suggested here, where middleware executed on the app channel where Dapr invokes the app.

Triaged into 1.9.

yaron2 avatar May 25 '22 14:05 yaron2

For gRPC, the HTTP middleware should not apply. We tried making a mapping between the HTTP and gRPC protocols and it falls short. Each middleware component should be protocol specific IMO.

artursouza avatar May 31 '22 17:05 artursouza

That's correct. I support changing this to the behavior suggested here, where middleware executed on the app channel where Dapr invokes the app.

Triaged into 1.9.

Could you please share the solution of this change in high level in advance?

We tried below way, it works, but don't know if it's a reasonable way: forward request from gRPC server to HTTP Server(where HTTP middleware be executed) Before the change: | service A -> dapr | ---> | dapr gRPC server---> service B | After the change: | service A -> dapr | ---> | dapr gRPC server-> HTTP Server---> service B |

Thanks

JackLiao2014 avatar Jul 25 '22 03:07 JackLiao2014

That's correct. I support changing this to the behavior suggested here, where middleware executed on the app channel where Dapr invokes the app. Triaged into 1.9.

Could you please share the solution of this change in high level in advance?

We tried below way, it works, but don't know if it's a reasonable way: forward request from gRPC server to HTTP Server(where HTTP middleware be executed) Before the change: | service A -> dapr | ---> | dapr gRPC server---> service B | After the change: | service A -> dapr | ---> | dapr gRPC server-> HTTP Server---> service B |

Thanks

How did you forward the request from gRPC to the HTTP server, and how did you translate the gRPC call to an HTTP1.1 call?

yaron2 avatar Jul 26 '22 02:07 yaron2

That's correct. I support changing this to the behavior suggested here, where middleware executed on the app channel where Dapr invokes the app. Triaged into 1.9.

Could you please share the solution of this change in high level in advance? We tried below way, it works, but don't know if it's a reasonable way: forward request from gRPC server to HTTP Server(where HTTP middleware be executed) Before the change: | service A -> dapr | ---> | dapr gRPC server---> service B | After the change: | service A -> dapr | ---> | dapr gRPC server-> HTTP Server---> service B | Thanks

How did you forward the request from gRPC to the HTTP server, and how did you translate the gRPC call to an HTTP1.1 call?

We made some changes by referring to this: https://github.com/dapr/dapr/blob/master/pkg/channel/http/http_channel.go#L142 The "http channel" is used to "InvokeMethod invokes user code via HTTP."

We change it from "invokes user code via HTTP" to "invokes HTTP Server via HTTP"

JackLiao2014 avatar Jul 27 '22 09:07 JackLiao2014

Folks, I've discussed this issue with few people, and we ended up with four different proposals (non-exhaustive) that can be evaluated.

Currently our http middleware pipeline is executed in the context of Incoming HTTP request for Dapr Sidecar, hence it is executed based on used Dapr API Protocol, if it is HTTP so the HTTP middleware Pipeline will be executed.

For the context of this issue, we're talking about having middlewares that always executes based on the App Protocol used.

So, if the app uses HTTP Protocol the HTTP middleware pipeline should always execute regardless of used Dapr API Protocol, either gRPC or HTTP, service-to-service invocation or not.

So the first solution would be having a different type of pipeline that will be executed on the App Channel.

App Channel Middlewares

| service A -> dapr | ---> | dapr gRPC API --> HTTP App Channel --> App Channel Middlewares --> service B |

image

Configuration will look like:

apiVersion: dapr.io/v1alpha1
kind: Configuration
metadata:
  name: opa-test
spec:
  appHttpPipeline:
    handlers:
    - name: opa-policy
      type: middleware.http.opa

An important thing to say is that this is another type of pipeline (but same middleware implementation), because changing the current one might cause breaking changes in some of them since not every received http request is forwarded to the app itself.

Advantages

  1. Simplicity
  2. Reuse our already created middlewares and put them here

Disadvantages

  1. It is good to have middleware being executed as close to the edge as possible to avoid unnecessary computation (i.e. authorization/throttling/caching middlewares)
  2. Requires distinction between app channel middlewares and API middlewares.

gRPC Middleware | HTTP Pipeline Executor

| service A -> dapr | ---> | dapr gRPC API --> dapr gRPC HTTP Pipeline Executor --> App Channel --> service B |

Another option is having a gRPC middleware that parses the received gRPC request and convert to a valid http request to used as a context for http middlewares. It requires identifying incoming http requests within a gRPC Middleware (which might be difficult).

This would require mapping from gRPC Request to HTTP Request and the inverse as well, from HTTP Response to gRPC response.

image

Advantages

  1. Same http middleware pipeline could be applied (?)
  2. Executed on request edge and avoid unnecessary computation

Disadvantages

  1. Mapping from gRPC to HTTP is not so straightforward (even if the gRPC request contain http information)

Protocol agnostic Middleware

| service A -> dapr | ---> | dapr gRPC API --> dapr Protocol-Agnostic Middleware --> App Channel --> service B |

This was an evaluated alternative for this, but if you look deeper, having a protocol agnostic middleware is an advantage in terms of implementation, but in use it ends up being the same, because usually the same app doesn't run in the context of http and grpc and will end up having to translate grpc to http anyways.

image

The idea is having a middleware.opa that receives both - depending on the Dapr API Protocol used, http and grpc context and knows how to parse and apply validations/logic.

apiVersion: dapr.io/v1alpha1
kind: Configuration
metadata:
  name: opa-test
spec:
  middlewarePipeline:
    handlers:
    - name: opa-policy
      type: middleware.opa

Execute on Service A Outbound Request

| service A -> dapr HTTP API --> HTTP Middleware Pipeline | ---> | dapr gRPC API --> App Channel --> service B |

image

The idea here is to apply the same middlewares but for outbound requests from any app.

Advantages

  1. The request would never hit the Service B

Disadvantages

  1. Requires the ServiceB to have a configuration for outbound pipelines explicitly for Service A

The configuration will look like:

apiVersion: dapr.io/v1alpha1
kind: Configuration
metadata:
  name: opa-test
spec:
  egressHttpPipeline:
    serviceA: # Something goes here to allow identifying which is the target service
      handlers:
      - name: opa-policy
        type: middleware.opa

cc: @yaron2 @artursouza thoughts?

mcandeia avatar Aug 30 '22 14:08 mcandeia

  1. App Channel Middlewares - I think this can unblock many scenarios like adapting a payload to a particular service. I think we should implement this in 1.9
  2. gRPC Middleware | HTTP Pipeline Executor - I found this confusing to understand. Option 1 is easier.
  3. Protocol agnostic Middleware - we have been that route before (trying to converge HTTP and gRPC protocols) and it did not work for service to service invocation. We ended up deprecating that. I would not try this again.
  4. Execute on Service A Outbound Request - I would rather implement solution 1 because this solution can also impact our internal dapr-to-dapr communication and I don't think users should mess with that (any impact on mtls, for example?).

artursouza avatar Sep 02 '22 18:09 artursouza

After further investigation, I found that it is not so simple to reuse our already created middlewares because they rely on fasthttp as server middleware. Despite being very similar, we will probably have to update our middleware to provide a more http-server agnostic interface.

see here.

So the advantage of reusing our Http middlewares might be not possible given the current state but totally possible in a world where we unify the interfaces for middlewares

mcandeia avatar Sep 02 '22 19:09 mcandeia

OK. Let's fix that too as part of this work. So, the middlewares can be agnostic of framework.

artursouza avatar Sep 02 '22 20:09 artursouza

OK. Let's fix that too as part of this work. So, the middlewares can be agnostic of framework.

As I suggested to @mcandeia, we need to enable middleware for service to service invocation, however it was intended from the beginning that middleware applies on the Dapr public HTTP server only, it was never meant to be used for s2s.

yaron2 avatar Sep 03 '22 01:09 yaron2

Does makes sense to you having app channel middlewares separate from dapr api middlewares @yaron2 ?

mcandeia avatar Sep 04 '22 18:09 mcandeia

Fyi: Despite the fact that I mentioned, it could be possible to reuse our middlewares with some mapping from fasthttp request to fasthttp requestctx, I found some code doing the same for the native net.http

However, I continue to support the middlewares to be http-server agnostic(by providing a simple interface for most of cases), this would make things easier for other related tasks such as pluggable components reusing our created middlewares

mcandeia avatar Sep 04 '22 18:09 mcandeia

Does makes sense to you having app channel middlewares separate from dapr api middlewares @yaron2 ?

I would choose this option as a last resort. Separate middleware means more friction for contributors and more maintenance and testing burden for maintainers, and for end users this means less options because there is no reusability.

yaron2 avatar Sep 04 '22 19:09 yaron2

I would choose this option as a last resort. Separate middleware means more friction for contributors and more maintenance and testing burden for maintainers, and for end users this means less options because there is no reusability.

Yes, the idea is actually make it work with the current middlewares without any further modification - at least not triggered by this demand, what I'm trying to validate now is the separation of concerns: two different places where middlewares can be executed, but the implementation still the same,

You can see a POC here

The configuration below show how it should look like:

apiVersion: dapr.io/v1alpha1
kind: Configuration
metadata:
  name: opa-test
spec:
  appHttpPipeline:
    handlers:
    - name: opa-policy
      type: middleware.http.opa

I think our middlewares should be reusable as our primary premisse to make this work well, even if it requires to provide an unified interface to manage/interact with the request context.

But seems to me that this work of providing a unified interface for middlewares (API and app channel) could be postponed given that we can map between a fasthttp.Request => fasthttp.RequestCtx

this issue would make things easier.

mcandeia avatar Sep 04 '22 19:09 mcandeia

Hey @ewassef I just opened a PR in your sample branch with the current proposal for the issue this PR, could you check?

please keep in mind that's not meant to be used in production yet, it's just a preview from my own fork

mcandeia avatar Sep 09 '22 13:09 mcandeia