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

feat(router): enable net.src.* and net.dst.* in http expression #11950

Merged
merged 10 commits into from
Jan 12, 2024

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Nov 8, 2023

Summary

KAG-2963
KAG-3032

Checklist

Full changelog

  • [Implement ...]

Issue reference

Fix #[issue number]

@chronolaw chronolaw force-pushed the feat/remove_net_port_in_expression branch from 2b2741d to 9357315 Compare November 17, 2023 14:15
Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this deprecation is more properly done from the ATC router side. What do you think?

kong/router/expressions.lua Outdated Show resolved Hide resolved
kong/router/expressions.lua Outdated Show resolved Hide resolved
kong/router/atc.lua Outdated Show resolved Hide resolved
@StarlightIbuki
Copy link
Contributor

Also, we should split this into 2 PRs, to let the title be precise.

@chronolaw
Copy link
Contributor Author

I feel this deprecation is more properly done from the ATC router side. What do you think?

No, atc-router library has no schema definition, we only define schema here.

@chronolaw
Copy link
Contributor Author

chronolaw commented Nov 20, 2023

Also, we should split this into 2 PRs, to let the title be precise.

Perhaps we can not do this, adding net.src.* and net.dst.* will cause the change of cache key at the same time.

@dndx
Copy link
Member

dndx commented Nov 27, 2023

@StarlightIbuki Please review again.

We need a temporary solution for deprecation. Considering the limited use case of expressions, I doubt we need to make the migration that fancy. I'm okay with a solution that doesn't understand the syntax.

@dndx dndx force-pushed the feat/remove_net_port_in_expression branch from 105f1e9 to 2ed23bc Compare November 27, 2023 06:21
Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor suggestion on performance. Otherwise good enough.

kong/router/expressions.lua Outdated Show resolved Hide resolved
kong/router/expressions.lua Show resolved Hide resolved
@chronolaw chronolaw force-pushed the feat/remove_net_port_in_expression branch from 2ed23bc to 9a354b7 Compare November 27, 2023 09:07
@chronolaw chronolaw added the pr/discussion This PR is being debated. Probably just a few details. label Nov 29, 2023
@chronolaw chronolaw marked this pull request as draft November 29, 2023 08:38
@chronolaw chronolaw changed the title feat(router): enable net.src.* and net.dst.* in expression feat(router): enable net.src.* and net.dst.* in http expression Jan 9, 2024
chronolaw and others added 10 commits January 11, 2024 17:43
convert net.port

basic hash

match src_ip/port

cache key

remove net.port

verify_expression

verify expression more

verify_expression

verify_expression

change basic fields

stream exec()

http select()

test net.dst.port

expression work with net.port

deprecate_net_port_and_rework_cache_algo.yml

tune tests

fix stream tests

lint fix

net.src.ip & net.dst.ip

schema validation tests

more tests

spy.on for log warning

finally restore spy.on

gsub net.port ==

fix lint error

verify_expression

ngx.re.gsub

NET_PORT_REPLACE

changelog

regex optimization

changelog

revert some code
@dndx dndx force-pushed the feat/remove_net_port_in_expression branch from f0699ac to 609c59c Compare January 11, 2024 09:43
@flrgh
Copy link
Contributor

flrgh commented Jan 11, 2024

Is this ready to merge, or are we waiting for any reason?

@dndx dndx merged commit 560fdfb into master Jan 12, 2024
23 checks passed
@dndx dndx deleted the feat/remove_net_port_in_expression branch January 12, 2024 07:42
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

chronolaw added a commit that referenced this pull request Jan 23, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
chronolaw added a commit that referenced this pull request Jan 24, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
chronolaw added a commit that referenced this pull request Feb 26, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
chronolaw added a commit that referenced this pull request Feb 26, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
chronolaw added a commit that referenced this pull request Mar 4, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
locao pushed a commit that referenced this pull request Mar 5, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
ADD-SP pushed a commit that referenced this pull request Mar 7, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
ADD-SP pushed a commit that referenced this pull request Mar 7, 2024
…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants