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

fix(weaver): upgraded Corda dependencies to overcome Log4j vulnerability #2980

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Conversation

VRamakrishna
Copy link
Contributor

  • Updated Corda package dependencies from 4.8 to 4.8.11, in which the vulnerability was fixed.
  • Upgraded Gradle from 5.6.4 to 7.5 to manage the Corda updates.
  • Downloaded Clikt dependency directly from Maven Central to avoid variant ambiguities in Corda Simple Application.
  • Fixed formatting in shell scripts to ensure that the shell command directive was in Line 1.

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

Copy link
Contributor

@sandeepnRES sandeepnRES left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@VRamakrishna
Copy link
Contributor Author

I need to fix a failing unit test. Am testing the fix out now (requires minor upgrades to the Kotlin and Corda plugin versions). Will merge after I see that test (and other Weaver tests) pass.

@petermetz
Copy link
Contributor

I need to fix a failing unit test. Am testing the fix out now (requires minor upgrades to the Kotlin and Corda plugin versions). Will merge after I see that test (and other Weaver tests) pass.

@VRamakrishna Sure thing, take your time!

@petermetz
Copy link
Contributor

@VRamakrishna If the changes in those two commits that you have in the PR right now are broken separately, then please s quash them together. The guiding principle being that if you check out any specific commit from the main branch then the build should be working regardless of which commit you've picked.

@VRamakrishna VRamakrishna enabled auto-merge (squash) January 19, 2024 06:28
@sandeepnRES sandeepnRES disabled auto-merge January 19, 2024 06:29
@sandeepnRES sandeepnRES enabled auto-merge (rebase) January 19, 2024 06:29
@VRamakrishna
Copy link
Contributor Author

@petermetz I fixed the Weaver tests and squashed the commits, but there is still a blocking error in "Cactus CI / yarn_codegen":
image

See https://github.com/hyperledger/cacti/actions/runs/7580285585/job/20645864623?pr=2980 for error logs. I don't believe my changes have anything to do with this. Can you check?

@outSH
Copy link
Contributor

outSH commented Jan 19, 2024

@petermetz I fixed the Weaver tests and squashed the commits, but there is still a blocking error in "Cactus CI / yarn_codegen": image

See https://github.com/hyperledger/cacti/actions/runs/7580285585/job/20645864623?pr=2980 for error logs. I don't believe my changes have anything to do with this. Can you check?

@VRamakrishna The following files are getting changed by yarn run codegen (don't ask me why, it seem that changes in gradle depencies in weaver bubbles up to other projects in monorepo):

 M examples/cactus-example-carbon-accounting-business-logic-plugin/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M examples/cactus-example-supply-chain-business-logic-plugin/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M extensions/cactus-plugin-object-store-ipfs/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-cmd-api-server/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-core-api/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-consortium-manual/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-htlc-eth-besu-erc20/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-aws-sm/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-azure-kv/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-google-sm/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-memory-wasm/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-memory/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-keychain-vault/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-ledger-connector-corda/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-ledger-connector-fabric/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat
 M packages/cactus-plugin-odap-hermes/src/main/kotlin/generated/openapi/kotlin-client/gradlew.bat

coedgen on main works fine so this must be caused by changes here

@VRamakrishna
Copy link
Contributor Author

VRamakrishna commented Jan 19, 2024

@outSH Thanks for pointing that out. I suspect my .gitattributes change may be behind this. Let me revise that and see.

(Update) The test now passes after I reverted the change.

@VRamakrishna VRamakrishna disabled auto-merge January 19, 2024 15:41
@VRamakrishna VRamakrishna enabled auto-merge (rebase) January 22, 2024 07:56
@VRamakrishna VRamakrishna disabled auto-merge January 22, 2024 07:56
@VRamakrishna VRamakrishna enabled auto-merge (rebase) January 22, 2024 11:29
@VRamakrishna
Copy link
Contributor Author

@petermetz @outSH I had to fix some conflicts in a file that was modified by another PR 3 days ago, but after doing so, I'm getting an error in the yarn_codegen workflow again. This time is throws an error in a specific package which I never touched. The .gitattributes file was also not changed. So I don't know how to debug this. Can you help? The link to the failing workflow: https://github.com/hyperledger/cacti/actions/runs/7610446051/job/20723809720?pr=2980.

@petermetz
Copy link
Contributor

@petermetz @outSH I had to fix some conflicts in a file that was modified by another PR 3 days ago, but after doing so, I'm getting an error in the yarn_codegen workflow again. This time is throws an error in a specific package which I never touched. The .gitattributes file was also not changed. So I don't know how to debug this. Can you help? The link to the failing workflow: https://github.com/hyperledger/cacti/actions/runs/7610446051/job/20723809720?pr=2980.

@VRamakrishna Some changes got snuck in that triggered the codegen race conditions again.

For now the way I've been dealing with it myself is to just re-try that one specific job and usually it goes through the second time but if not the third time did it so far so I never had to do it a forth time.
Also, make sure not to retry the entire CI execution, just the specific job:

image

Updated Corda package dependencies from 4.8 to 4.8.11, in which the vulnerability was fixed.
Upgraded Gradle from 5.6.4 to 7.5 to manage the Corda updates.
Downloaded Clikt dependency directly from Maven Central to avoid variant ambiguities in Corda Simple Application.
Upgraded Kotlin and Corda plugin versions to make Weaver Interop CordApp unit tests pass.
Fixed formatting in shell scripts to ensure that the shell command directive was in Line 1.

Signed-off-by: VRamakrishna <[email protected]>
@VRamakrishna VRamakrishna merged commit 76f0c68 into hyperledger-cacti:main Jan 23, 2024
127 of 147 checks passed
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.

4 participants