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

[ISSUE #8169] Upgrade spring-boot version to 2.6.6(2.x) #8184

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

liqipeng
Copy link
Contributor

Issue: #8169
参考 GHSA-36p3-wjmg-h94x ,升级到 Spring Boot 2.6.6 修复此问题

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch 3 times, most recently from 2a96bae to c793d8d Compare April 17, 2022 08:28
@liqipeng liqipeng marked this pull request as ready for review April 17, 2022 09:08
@KomachiSion
Copy link
Collaborator

循环依赖是哪里报的? 信息是什么?
我记得2.1版本的就已经是不允许循环依赖的了。

@liqipeng
Copy link
Contributor Author

循环依赖是哪里报的? 信息是什么? 我记得2.1版本的就已经是不允许循环依赖的了。

可以看下这个CI https://github.com/alibaba/nacos/runs/6051513004?check_suite_focus=true 日志里搜“circular reference”

The dependencies of some of the beans in the application context form a cycle:

   nacosAuthManager (field private com.alibaba.nacos.plugin.auth.impl.JwtTokenManager com.alibaba.nacos.plugin.auth.impl.NacosAuthManager.tokenManager)
┌─────┐
|  jwtTokenManager (field private com.alibaba.nacos.plugin.auth.impl.NacosAuthConfig com.alibaba.nacos.plugin.auth.impl.JwtTokenManager.nacosAuthConfig)
↑     ↓
|  nacosAuthConfig (field private com.alibaba.nacos.plugin.auth.impl.JwtTokenManager com.alibaba.nacos.plugin.auth.impl.NacosAuthConfig.tokenProvider)
└─────┘

当我手动调整代码让这个依赖解除后,发现又遇到了另外更复杂的循环依赖(印象中好像是5个类的依赖,代码未提交就撤销了,暂时不能提供具体的)。

issue里补充了一些信息 #8169 (comment) 这里也同步一下:

补充一下上面关于循环依赖:

  1. 循环依赖问题出现在 develop 分支,1.x分支未遇到循环依赖问题;
  2. 对于 develop 分支的循环依赖问题,已调整处理方式为在 SpringApplicationRunListener 里通过System.setProperty硬编码方式设置,见 fc0d3c4 ,这样就不需要增加额外配置影响后续升版 ;

@liqipeng
Copy link
Contributor Author

彻底解决循环依赖建议可以单独再搞个issue来处理。

@liqipeng
Copy link
Contributor Author

@KomachiSion

@KomachiSion
Copy link
Collaborator

彻底解决循环依赖建议可以单独再搞个issue来处理。

建议先单独pr解决循环依赖问题,再升级spring 版本

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch 2 times, most recently from 3b95939 to 24eb0fb Compare May 16, 2022 17:21
@liqipeng
Copy link
Contributor Author

彻底解决循环依赖建议可以单独再搞个issue来处理。

建议先单独pr解决循环依赖问题,再升级spring 版本

这个PR也rebase了一下,并解决了 JwtTokenManagerNacosAuthConfig的循环依赖。
另外还有循环依赖是在istio模块待解决:

org.springframework.beans.factory.UnsatisfiedDependencyException: 
Error creating bean with name 'nacosMcpService': Unsatisfied dependency expressed through field 'resourceManager'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: 
Error creating bean with name 'nacosResourceManager': Unsatisfied dependency expressed through field 'serviceInfoResourceWatcher'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: 
Error creating bean with name 'nacosServiceInfoResourceWatcher': Unsatisfied dependency expressed through field 'eventProcessor'; nested exception is org.springframework.beans.factory.BeanCurrentlyInCreationException: 
Error creating bean with name 'eventProcessor': Requested bean is currently in creation: Is there an unresolvable circular reference?

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch from 24eb0fb to 66d55c7 Compare May 16, 2022 18:07
@liqipeng
Copy link
Contributor Author

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

@KomachiSion
Copy link
Collaborator

请不要在一个PR中修复多个问题。

各个循环依赖可以单独提PR修复。

@KomachiSion
Copy link
Collaborator

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

我看了一下PR, 其中auth相关的循环依赖解决的没有问题,希望单独一个PR修复。
istio没有修复,依赖关闭循环依赖检测来弄,不太好,先由我来简单修复下。
关于OkHttp的去除,可以单独提交一个PR。

最后这些都修复完毕了,再提交PR来升级spring 版本。

@chenhao26-nineteen
Copy link
Collaborator

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

我看了一下PR, 其中auth相关的循环依赖解决的没有问题,希望单独一个PR修复。 istio没有修复,依赖关闭循环依赖检测来弄,不太好,先由我来简单修复下。 关于OkHttp的去除,可以单独提交一个PR。

最后这些都修复完毕了,再提交PR来升级spring 版本。

For the cyclic dependency of auth, I will refer to this PR and resubmit a pr

@liqipeng
Copy link
Contributor Author

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

我看了一下PR, 其中auth相关的循环依赖解决的没有问题,希望单独一个PR修复。 istio没有修复,依赖关闭循环依赖检测来弄,不太好,先由我来简单修复下。 关于OkHttp的去除,可以单独提交一个PR。

最后这些都修复完毕了,再提交PR来升级spring 版本。

OK,我拆分一下,让当前这个PR仅留下升级Spring版本号相关的部分。

@liqipeng
Copy link
Contributor Author

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

暂不太熟悉istio相关,已提了一个单独issue #8405,邀请协助解决

我看了一下PR, 其中auth相关的循环依赖解决的没有问题,希望单独一个PR修复。 istio没有修复,依赖关闭循环依赖检测来弄,不太好,先由我来简单修复下。 关于OkHttp的去除,可以单独提交一个PR。
最后这些都修复完毕了,再提交PR来升级spring 版本。

For the cyclic dependency of auth, I will refer to this PR and resubmit a pr

Tks

@liqipeng
Copy link
Contributor Author

liqipeng commented May 22, 2022

已拆分了两个PR:
#8422 remove commonOkHttp dependency(2.x)
#8425 solve circular reference between JwtTokenManager and NacosAuthConfig(2.x)

但是遇到develop分支好像出了什么问题,ci在GHA无法通过,而ci无法通过的测试在本地却是可以通过的,尝试排查但是暂未找到原因:(下面错误信息来自:https://github.com/alibaba/nacos/runs/6529660737?check_suite_focus=true)


Error:  Tests run: 15, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 12.124 s <<< FAILURE! - in com.alibaba.nacos.naming.core.v2.upgrade.UpgradeJudgementTest
Error:  testUpgradeCheckSucc(com.alibaba.nacos.naming.core.v2.upgrade.UpgradeJudgementTest)  Time elapsed: 0.808 s  <<< FAILURE!
java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertTrue(Assert.java:52)
	at com.alibaba.nacos.naming.core.v2.upgrade.UpgradeJudgementTest.testUpgradeCheckSucc(UpgradeJudgementTest.java:157)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)

需要先解决一下develop分支本身的ci的问题, @KomachiSion ,这个问题在其他PR也遇到了。

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch from 66d55c7 to fbe697e Compare May 23, 2022 17:02
@liqipeng
Copy link
Contributor Author

liqipeng commented May 23, 2022

@KomachiSion 此PR已进行rebase,rebase后循环依赖报错还是存在的:

The dependencies of some of the beans in the application context form a cycle:

   istioServer (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.server.IstioServer.nacosMcpService)
┌─────┐
|  nacosMcpService (field com.alibaba.nacos.istio.common.NacosResourceManager com.alibaba.nacos.istio.mcp.NacosMcpService.resourceManager)
↑     ↓
|  nacosResourceManager (field com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher com.alibaba.nacos.istio.common.NacosResourceManager.serviceInfoResourceWatcher)
↑     ↓
|  nacosServiceInfoResourceWatcher (field private com.alibaba.nacos.istio.common.EventProcessor com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher.eventProcessor)
↑     ↓
|  eventProcessor (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.common.EventProcessor.nacosMcpService)
└─────┘

详见:https://github.com/alibaba/nacos/runs/6559728106?check_suite_focus=true

@KomachiSion
Copy link
Collaborator

@KomachiSion 此PR已进行rebase,rebase后循环依赖报错还是存在的:

The dependencies of some of the beans in the application context form a cycle:

   istioServer (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.server.IstioServer.nacosMcpService)
┌─────┐
|  nacosMcpService (field com.alibaba.nacos.istio.common.NacosResourceManager com.alibaba.nacos.istio.mcp.NacosMcpService.resourceManager)
↑     ↓
|  nacosResourceManager (field com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher com.alibaba.nacos.istio.common.NacosResourceManager.serviceInfoResourceWatcher)
↑     ↓
|  nacosServiceInfoResourceWatcher (field private com.alibaba.nacos.istio.common.EventProcessor com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher.eventProcessor)
↑     ↓
|  eventProcessor (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.common.EventProcessor.nacosMcpService)
└─────┘

详见:https://github.com/alibaba/nacos/runs/6559728106?check_suite_focus=true

OK, I will recheck it.

@KomachiSion
Copy link
Collaborator

@KomachiSion 此PR已进行rebase,rebase后循环依赖报错还是存在的:

The dependencies of some of the beans in the application context form a cycle:

   istioServer (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.server.IstioServer.nacosMcpService)
┌─────┐
|  nacosMcpService (field com.alibaba.nacos.istio.common.NacosResourceManager com.alibaba.nacos.istio.mcp.NacosMcpService.resourceManager)
↑     ↓
|  nacosResourceManager (field com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher com.alibaba.nacos.istio.common.NacosResourceManager.serviceInfoResourceWatcher)
↑     ↓
|  nacosServiceInfoResourceWatcher (field private com.alibaba.nacos.istio.common.EventProcessor com.alibaba.nacos.istio.common.NacosServiceInfoResourceWatcher.eventProcessor)
↑     ↓
|  eventProcessor (field private com.alibaba.nacos.istio.mcp.NacosMcpService com.alibaba.nacos.istio.common.EventProcessor.nacosMcpService)
└─────┘

详见:https://github.com/alibaba/nacos/runs/6559728106?check_suite_focus=true

#8467 should fix it, You can retry again.

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch from fbe697e to 02bc35b Compare May 29, 2022 15:48
@liqipeng
Copy link
Contributor Author

昨天再次rebase,IT已经ok了,有CI错误,我再看看

@liqipeng liqipeng force-pushed the lqp/v2.x-develop_issue8169 branch from 02bc35b to 045154c Compare June 3, 2022 15:51
@KomachiSion KomachiSion merged commit 4c23846 into alibaba:develop Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants