You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by GitBox <gi...@apache.org> on 2022/12/11 05:21:02 UTC

[GitHub] [shenyu] neo-shirley opened a new pull request, #4259: Update ShenyuExtConfiguration.java

neo-shirley opened a new pull request, #4259:
URL: https://github.com/apache/shenyu/pull/4259

   此处注入的 new RemoteAddressResolver() 会使得 org/apache/shenyu/springboot/starter/gateway/ShenyuConfiguration.java:162的ForwardedRemoteAddressResolver注入失效。 
   这与官方文档的说明不符，官网文档显示在Apache ShenYu网关中获取IP的自带实现为：org.apache.shenyu.web.forward.ForwardedRemoteAddressResolver。
   
   <!-- Describe your PR here; eg. Fixes #issueNo -->
   
   <!--
   Thank you for proposing a pull request. This template will guide you through the essential steps necessary for a pull request.
   -->
   Make sure that:
   
   - [ ] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [ ] You submit test cases (unit or integration tests) that back your changes.
   - [ ] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] neo-shirley commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
neo-shirley commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1045364073


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -50,7 +51,6 @@ public ShenyuResult<?> shenyuResult() {
     @Bean
     @ConditionalOnMissingBean(value = RemoteAddressResolver.class, search = SearchStrategy.ALL)
     public RemoteAddressResolver remoteAddressResolver() {
-        return new RemoteAddressResolver() {
-        };
+        return new ForwardedRemoteAddressResolver(1);

Review Comment:
   yes，you can delete this code。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1047949971


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -20,6 +20,7 @@
 import org.apache.shenyu.plugin.api.RemoteAddressResolver;
 import org.apache.shenyu.plugin.api.result.DefaultShenyuResult;
 import org.apache.shenyu.plugin.api.result.ShenyuResult;
+import org.apache.shenyu.web.forward.ForwardedRemoteAddressResolver;

Review Comment:
   remove this import



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1045362590


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -50,7 +51,6 @@ public ShenyuResult<?> shenyuResult() {
     @Bean
     @ConditionalOnMissingBean(value = RemoteAddressResolver.class, search = SearchStrategy.ALL)
     public RemoteAddressResolver remoteAddressResolver() {
-        return new RemoteAddressResolver() {
-        };
+        return new ForwardedRemoteAddressResolver(1);

Review Comment:
   this code alreay in ShenyuConfiguration.java. 
   
   can remove this ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] neo-shirley commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
neo-shirley commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1047076954


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -50,7 +51,6 @@ public ShenyuResult<?> shenyuResult() {
     @Bean
     @ConditionalOnMissingBean(value = RemoteAddressResolver.class, search = SearchStrategy.ALL)
     public RemoteAddressResolver remoteAddressResolver() {
-        return new RemoteAddressResolver() {
-        };
+        return new ForwardedRemoteAddressResolver(1);

Review Comment:
   ok



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 commented on PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#issuecomment-1354061957

   hi， pls check ci，
   
   > ShenyuExtConfigurationTest.testRemoteAddressResolver:58->lambda$testRemoteAddressResolver$1:59 » NoSuchBeanDefinition


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] key9527 commented on pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
key9527 commented on PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#issuecomment-1365718647

   > hi， pls check ci，
   > 
   > > ShenyuExtConfigurationTest.testRemoteAddressResolver:58->lambda$testRemoteAddressResolver$1:59 » NoSuchBeanDefinition
   
   ok,  All checks have passed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1046593589


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -50,7 +51,6 @@ public ShenyuResult<?> shenyuResult() {
     @Bean
     @ConditionalOnMissingBean(value = RemoteAddressResolver.class, search = SearchStrategy.ALL)
     public RemoteAddressResolver remoteAddressResolver() {
-        return new RemoteAddressResolver() {
-        };
+        return new ForwardedRemoteAddressResolver(1);

Review Comment:
   you can remove this (modify this pr)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 merged pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 merged PR #4259:
URL: https://github.com/apache/shenyu/pull/4259


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] codecov-commenter commented on pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#issuecomment-1345792386

   # [Codecov](https://codecov.io/gh/apache/shenyu/pull/4259?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4259](https://codecov.io/gh/apache/shenyu/pull/4259?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed308ca) into [master](https://codecov.io/gh/apache/shenyu/commit/f79710d668966dcfbcc4a47dbbc0e44faef16233?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f79710d) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4259      +/-   ##
   ============================================
   - Coverage     69.28%   69.27%   -0.02%     
     Complexity     7345     7345              
   ============================================
     Files           993      993              
     Lines         27970    27970              
     Branches       2476     2476              
   ============================================
   - Hits          19379    19375       -4     
   - Misses         7123     7127       +4     
     Partials       1468     1468              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shenyu/pull/4259?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...enyu/web/configuration/ShenyuExtConfiguration.java](https://codecov.io/gh/apache/shenyu/pull/4259/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXdlYi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hlbnl1L3dlYi9jb25maWd1cmF0aW9uL1NoZW55dUV4dENvbmZpZ3VyYXRpb24uamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [...che/shenyu/sync/data/http/HttpSyncDataService.java](https://codecov.io/gh/apache/shenyu/pull/4259/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXN5bmMtZGF0YS1jZW50ZXIvc2hlbnl1LXN5bmMtZGF0YS1odHRwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGVueXUvc3luYy9kYXRhL2h0dHAvSHR0cFN5bmNEYXRhU2VydmljZS5qYXZh) | `85.71% <0.00%> (-4.09%)` | :arrow_down: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] key9527 commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
key9527 commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1047956275


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -20,6 +20,7 @@
 import org.apache.shenyu.plugin.api.RemoteAddressResolver;
 import org.apache.shenyu.plugin.api.result.DefaultShenyuResult;
 import org.apache.shenyu.plugin.api.result.ShenyuResult;
+import org.apache.shenyu.web.forward.ForwardedRemoteAddressResolver;

Review Comment:
   sorry，my fault,  i forget this import code，i have deleted this，please merge again。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4259: Update ShenyuExtConfiguration.java

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4259:
URL: https://github.com/apache/shenyu/pull/4259#discussion_r1047949971


##########
shenyu-web/src/main/java/org/apache/shenyu/web/configuration/ShenyuExtConfiguration.java:
##########
@@ -20,6 +20,7 @@
 import org.apache.shenyu.plugin.api.RemoteAddressResolver;
 import org.apache.shenyu.plugin.api.result.DefaultShenyuResult;
 import org.apache.shenyu.plugin.api.result.ShenyuResult;
+import org.apache.shenyu.web.forward.ForwardedRemoteAddressResolver;

Review Comment:
   remove this ForwardedRemoteAddressResolver



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org