Ocelot icon indicating copy to clipboard operation
Ocelot copied to clipboard

#360 Routing based on request header

Open jlukawska opened this issue 4 years ago • 16 comments

Closes #360

Routing based on request header.

Proposed Changes

Additionally to routing by UpstreamPathTemplate, you can define UpstreamHeaderTemplates. Then to match a route, all the headers from this section must exist in the request headers.

The example:

    {
        "DownstreamPathTemplate": "/",
        "DownstreamScheme": "https",
        "DownstreamHostAndPorts": [
                {
                    "Host": "10.0.10.1",
                    "Port": 80,
                }
            ],
        "UpstreamPathTemplate": "/",
        "UpstreamHttpMethod": [ "Get" ],
        "UpstreamHeaderTemplates": {
            "country": "uk",
            "version": "v1"
        }
    }

In this case the route will be matched only if a request has both headers set to the configured values.

You can use placeholders in your UpstreamHeaderTemplates in the following way:

    {
        "DownstreamPathTemplate": "/{versionnumber}/api",
        "DownstreamScheme": "https",
        "DownstreamHostAndPorts": [
                {
                    "Host": "10.0.10.1",
                    "Port": 80,
                }
            ],
        "UpstreamPathTemplate": "/api",
        "UpstreamHttpMethod": [ "Get" ],
        "UpstreamHeaderTemplates": {
            "version": "{header:versionnumber}"
        }
    }

In this case the whole value for the request header version will be placed into the DownstreamPathTemplate. If you need, you can specify more complex upstream header template with placeholders like e.g. version-{header:version}_country-{header:country}.

Placeholders do not have to exist in DownstreamPathTemplate. This case may be used to require specific header no matter what the value it contains.

UpstreamHeaderTemplates section can be used also for Aggregates.

jlukawska avatar Aug 14 '20 19:08 jlukawska

Hi @Thanos06660 @jlukawska,

Thanks for this PR! This is something super useful :)

Do we know if this is going to be released soon?

Best, Fadil

fadil-khodabuccus-cko avatar Apr 08 '21 14:04 fadil-khodabuccus-cko

@fadil-khodabuccus-cko commented on Apr 8, 2021

I agree this is super useful and should be released

CorporateActionMan avatar Apr 01 '22 12:04 CorporateActionMan

Vote up! Any word on when this will be released?

stodolos avatar Aug 16 '22 20:08 stodolos

Hello.

Just wanted to check in to see if there is any progress to release this?

madebykrol avatar Jan 18 '23 17:01 madebykrol

Hi @jlukawska, Any news ?

akanmf avatar Apr 13 '23 06:04 akanmf

Hi, please accept it. We are really needing this feature and this is opened since 2020

felipepiresdejesus avatar Apr 13 '23 12:04 felipepiresdejesus

Hi @jlukawska, Any news ?

Unfortunately, I don't have rights to merge it. It seems that the Ocelot project has been abandoned by its owner :(

jlukawska avatar Apr 13 '23 13:04 jlukawska

@felipepiresdejesus commented on Apr 13, 2023:

Hi, please accept it. We are really needing this feature and this is opened since 2020

Don't worry, Felipe! :blush: Coming soon... :sunglasses:

Will you resolve all merge conflicts? 😉 It is a joke!

raman-m avatar Jul 17 '23 19:07 raman-m

Well... @jlukawska

Before resolving merge conflicts for this PR, could we work on develop branch in your fork, please? After merging PR 7 your repo has redundant merge commit. That was happen because you used "Merge commit" option while merging the PR. For canonic updates it is better to use fast-forwarding option (rebase).

Step 1

To fix that you need to remove current top-commit which is merge commit (it has 2 parents). Use this command:

git checkout develop
git reset --hard HEAD~1

Reset command will report: HEAD is now at 0af49617 Merge pull request #2 from ThreeMammals/develop

After that, try to Sync fork please? I hope it should help you to make canonic update of develop branch. I hope Sync Fork operation will apply rebasing.

Step 2

Because you have other merge commits in develop branch which don't belong to head repository, I expect Sync Fork will fail because of some reason. In this case it is better just to remove develop branch, and provide new Sync Fork to pull develop branch entirely.

Step 3

If step 2 failed (no the same top commit in develop), just delete develop branch using GitHub UI and make git fetch --all, and remove develop locally. After that use this script to pull develop from head (upstream) repository:

git remote add upstream https://github.com/ThreeMammals/Ocelot.git // add original repo as upstream remote
git fetch upstream                                                 // pull changes from original repo
git checkout -b develop upstream/develop                           // pull down develop branch locally
git push -u origin develop                                         // push develop branch to fork

Don't forget to make develop branch as default one.

Let me know upgrading results for develop branch, please! Thank you!

raman-m avatar Jul 18 '23 11:07 raman-m

The feature branch has been rebased onto ThreeMammals:develop! ✔️ All merge conflicts have been resolved successfully during rebase-operation! ✔️

@jlukawska I thought you need a help in conflicts resolution. We are now in the code review stage! 🎉

raman-m avatar Aug 02 '23 19:08 raman-m

@BrettJG As the issue #360 author, Could you share your opinion regarding this solution please?


@netren20000 @iderbyshev @pliolis @lalocarbone @Phiph @mrclayman @jrsanbornjr @sachinjagdale @SebastianMajewski @falcopr @brettwinters @RickJNash @PraveenValavan After your hot discussions for #360 in the past, I am inviting you to review this solution, plz. So, I will appreciate your feedbacks!

raman-m avatar Aug 04 '23 10:08 raman-m

@Thanos06660 @fadil-khodabuccus-cko @CorporateActionMan @stodolos @madebykrol @akanmf @felipepiresdejesus Welcome to code review session! I know you've been waiting for this feature for years, but let's improve the quality of the solution. 😉

raman-m avatar Aug 04 '23 11:08 raman-m

Hi, The pull request was approved but now there are conflicts

felipepiresdejesus avatar Nov 23 '23 01:11 felipepiresdejesus

@RaynaldM reviewed on Nov 23

Thanks for the review!

@ggnaegi @RaynaldM This PR is not planned for Oct and Nov milestones, but this PR is important one to be included to future Dec'23 milestone. If you want to lead this PR then I'll be glad... But we have to focus on Nov milestone now: which progress is 0%! Time is marching... I'll return to this PR back after Nov release...

raman-m avatar Nov 23 '23 14:11 raman-m

@felipepiresdejesus commented on Nov 23 The pull request was approved but now there are conflicts

Yes, the team knows about that. Thanks for pointing to this problem!

To fix conflicts more quickly you could ask the author @jlukawska to add you to her forked repo as collaborator, and then you'll be able to help her just using her feature branch. 😉

raman-m avatar Nov 23 '23 14:11 raman-m

Possible delivery in December...

raman-m avatar Nov 29 '23 17:11 raman-m