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

Unable to match serviceId and routeId #389

Closed
ZoraZora59 opened this issue Oct 22, 2020 · 20 comments
Closed

Unable to match serviceId and routeId #389

ZoraZora59 opened this issue Oct 22, 2020 · 20 comments
Labels

Comments

@ZoraZora59
Copy link

rateLimit version 2.4.1
Here is my yml configuration.

ratelimit:
    key-prefix: ${spring.cloud.client.ip-address}:apiserv_rate_limit
    enabled: true
    repository: REDIS
    behind-proxy: true
    default-policy-list: #optional - will apply unless specific policy exists
      - limit: 10 
        quota: 1000 
        refresh-interval: 2000 
        type: 
          - url
    policy-list:
      cloud-currencyBackend:
        - limit: 10
          quota: 1000
          refresh-interval: 60
          type: #optional
            - url=/backend/currency/**
routes:
    cloud-currencyBackend:
      path: /backend/currency/**
      serviceId: currency-service
      stripPrefix: false

Every time while RateLimitProperties.getPolicies(key) execute , the key will be routeId likes /backend/currency/**.
My policyList is serviceId : targetPolicy likes cloud-currencyBackend: some policy. Therefore the result of this method will always be defaultPolicyList.

How can I make this work ?

@ZoraZora59 ZoraZora59 added the bug label Oct 22, 2020
@github-actions
Copy link

Hello @ZoraZora59 ,thank you for submitting an issue!

@ZoraZora59
Copy link
Author

<dependency>
    <groupId>com.marcosbarbero.cloud</groupId>
    <artifactId>spring-cloud-zuul-ratelimit</artifactId>
    <version>2.4.1.RELEASE</version>
</dependency>

@ZoraZora59
Copy link
Author

Hi @lchayoun , could you please offer me some help?

@ZoraZora59
Copy link
Author

Please @marcosbarbero . I wanne commit a fix branch for this issue if no available solution for it,.

@lchayoun
Copy link
Collaborator

lchayoun commented Oct 22, 2020

is cloud-currencyBackend the name of the service that is serving the /backend/currency/** endpoints?

@ZoraZora59
Copy link
Author

is cloud-currencyBackend the name of the service that is serving the /backend/currency/** endpoints?

I tried “currency-service” but it doesn’t work as well.

@marcosbarbero
Copy link
Owner

@ZoraZora59 thanks for the feedback, but I'm a little confused, what's the service name? Does the request work without the rate-limiting?

@ZoraZora59
Copy link
Author

@ZoraZora59 thanks for the feedback, but I'm a little confused, what's the service name? Does the request work without the rate-limiting?

The application name of my service is "currency-service". It marked as "cloud-currencyBackend" in "zuul.routes".

@ZoraZora59
Copy link
Author

I found a method named policy at calss AbstractRateLimitFilter.

    protected List<Policy> policy(Route route, HttpServletRequest request) {
        String routeId = Optional.ofNullable(route).map(Route::getId).orElse(null);
        return properties.getPolicies(routeId).stream()
            .filter(policy -> applyPolicy(request, route, policy))
            .collect(Collectors.toList());
    }

I guess it might should be

    protected List<Policy> policy(Route route, HttpServletRequest request) {
        String routeId = Optional.ofNullable(route).map(Route::getLocation).orElse(null);
        return properties.getPolicies(routeId).stream()
            .filter(policy -> applyPolicy(request, route, policy))
            .collect(Collectors.toList());
    }

Notice the second row , Route::getId replaced into Route::getLocation.

@ZoraZora59
Copy link
Author

@ZoraZora59 thanks for the feedback, but I'm a little confused, what's the service name? Does the request work without the rate-limiting?

The request of test is "curl localhost:8080/backend/currency/sleep100ms".

@marcosbarbero
Copy link
Owner

Can you share a project reproducing the problem?
Also, I noticed you have it configured like this:

- url=/backend/currency/**

If you want to use URL patterns you should do it like this:

- url_pattern=/backend/currency/*

@ZoraZora59
Copy link
Author

ZoraZora59 commented Oct 23, 2020

Can you share a project reproducing the problem?
Also, I noticed you have it configured like this:

- url=/backend/currency/**

If you want to use URL patterns you should do it like this:

- url_pattern=/backend/currency/*

I switch to version 2.0.6.RELEASE , and set yml as follow:

  ratelimit:
    key-prefix: ${spring.cloud.client.ip-address}:apiserv_rate_limit
    enabled: true
    repository: REDIS
    behind-proxy: true
    default-policy-list: #optional - will apply unless specific policy exists
      - limit: 10
        quota: 1000 
        refresh-interval: 2000
        type:
          - url
    policy-list:
      currency-service:
        - limit: 5 #optional - request number limit per refresh interval window
          quota: 1000 #optional - request time limit per refresh interval window (in seconds)
          refresh-interval: 60 #default value (in seconds)
          type: #optional
            - type: URL
              matcher: /backend/currency/

After I update source code and install in my local mvn repository, it worked.

@ZoraZora59
Copy link
Author

Can you share a project reproducing the problem?
Also, I noticed you have it configured like this:

- url=/backend/currency/**

If you want to use URL patterns you should do it like this:

- url_pattern=/backend/currency/*

To reproducing the problem , here is part of my project config.

My own service yml as follow :

server:
  port: 2333
spring:
  application:
    name: currency-service

Configured in api-zuul as follow :

server:
  port: 8080
spring:
  application:
    name: api-zuul
zuul:
  ratelimit:
    key-prefix: ${spring.cloud.client.ip-address}:apiserv_rate_limit
    enabled: true
    repository: REDIS
    behind-proxy: true
    default-policy-list: 
      - limit: 10
        quota: 1000 
        refresh-interval: 2000 
        type:
          - url
    policy-list:
      currency-service:
        - limit: 5
          quota: 1000
          refresh-interval: 60 
          type:
            - type: URL
              matcher: /backend/currency/
  routes:
    cloud-currencyBackend:
      path: /backend/currency/**
      serviceId: currency-service
      stripPrefix: false

@marcosbarbero
Copy link
Owner

The problem still exists on version 2.4.1.RELEASE?

@ZoraZora59
Copy link
Author

The problem still exists on version 2.4.1.RELEASE?

Yes.

@ZoraZora59
Copy link
Author

The problem still exists on version 2.4.1.RELEASE?

As the result of route.getId, this problem still exist.

@marcosbarbero
Copy link
Owner

What are the spring-cloud and spring-boot versions used by you?

@ZoraZora59
Copy link
Author

What are the spring-cloud and spring-boot versions used by you?

Boot version 2.0.3.RELEASE
Cloud version Finchley.RELEASE

@marcosbarbero
Copy link
Owner

The versions you're using are not compatible with the 2.4.1.RELEASE version, you need to upgrade it to the latest spring-boot and spring-cloud versions

@ZoraZora59
Copy link
Author

The versions you're using are not compatible with the 2.4.1.RELEASE version, you need to upgrade it to the latest spring-boot and spring-cloud versions

OK, I’ll try it on latest version. Thanks.

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

No branches or pull requests

3 participants