Skip to content

Commit

Permalink
fix(mojaloop/#3096): unable to upgrade knex dependency on centralLedg…
Browse files Browse the repository at this point in the history
…er (#939)

fix(mojaloop/#3096): unable to upgrade knex dependency on centralLedger - mojaloop/project#3096
- added UV_THREADPOOL_SIZE config to CI command for running integration tests
- upgraded knex to latest version

The following issue mojaloop/project#3112 was created to investigate as to why the Integration Tests are so unstable when then Event Handlers are executing in-line. For the time being the above PR clearly separates the process which resolves the stability issue for the time being.

The following changes were made as a work-around:
- Integration Tests will no longer start the Event Handlers in-line. You will need to start the Central-Ledger Service as a seperate process (see updated documentation for more information)
- Documentation for Integration Tests updated to reflect the above requirement
- CI Job updated to start the Central-Ledger Service as a seperate Process prior to running Integration Tests
- Added comments to where the commented out code for the initialization of the in-line Event Handlers referencing the above story.
- Also made some changes to the Handler tests to handle failures instead of stalling the Integration Tests

chore: maintenance updates
- fixed audit issues
- added missing kafka topics from docker-compose init kafka container to ensure the CL Service startup is clean
  • Loading branch information
mdebarros authored Feb 3, 2023
1 parent 168fed9 commit c376af8
Show file tree
Hide file tree
Showing 9 changed files with 323 additions and 151 deletions.
49 changes: 47 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ defaults_Dependencies: &defaults_Dependencies |
apk --no-cache add openssh-client
apk add --no-cache -t build-dependencies make gcc g++ python3 libtool autoconf automake jq
apk add --no-cache -t openssl ncurses coreutils libgcc linux-headers grep util-linux binutils findutils
apk add --no-cache curl
npm config set unsafe-perm true
npm install -g node-gyp

Expand Down Expand Up @@ -221,12 +222,56 @@ jobs:
## wait for services to be up and running
npm run wait-4-docker
- run:
name: Run the integration tests
name: Install dependencies to check health services
command: |
sudo apt install curl
- run:
name: Rebuild any dependencies
command: |
npm rebuild
npm run test:int
- run:
name: Run migration scripts
command: |
npm run migrate
- run:
name: Run the integration tests
command: |
## Run Service in background
## This is a temporary work-around until the following issue can be addressed: https://github.com/mojaloop/project/issues/3112
echo "Starting Service in the background"
npm start > ./test/results/cl-service.log &
## Store PID for cleanup
echo $! > /tmp/int-test-service.pid
PID=$(cat /tmp/int-test-service.pid)
echo "Service started with Process ID=$PID"
## Check Service Health
echo "Waiting for Service to be healthy"
bash .circleci/curl-retry-cl-health.sh
## Lets wait a few seconds to ensure that Kafka handlers are rebalanced
echo "Waiting ${WAIT_FOR_REBALANCE}s for Kafka Re-balancing..." && sleep $WAIT_FOR_REBALANCE
## Start integration tests
echo "Running Integration Tests"
npm run test:int:skipMigrate
## Kill service
echo "Stopping Service with Process ID=$PID"
kill $(cat /tmp/int-test-service.pid)
environment:
ENDPOINT_URL: http://localhost:4545/notification
UV_THREADPOOL_SIZE: 12
WAIT_FOR_REBALANCE: 20
TEST_INT_RETRY_COUNT: 20
TEST_INT_RETRY_DELAY: 2
TEST_INT_REBALANCE_DELAY: 20000
- run:
name: Cleanup Docker
command: |
## Shutdown Docker containers
docker compose down -v
- store_artifacts:
path: ./test/results
prefix: test
Expand Down
33 changes: 33 additions & 0 deletions .circleci/curl-retry-cl-health.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/bin/bash

## Script is to wait-retry with a number of retries to determine if the Central Ledger health end-point is healthy or not

# Central Ledger Health end-point for integration tests
url="http://localhost:3001/health"

# Number of retries
retries=10

# Sleep between retries
sleepwait=1

# Counter for retries
count=0

while [ $count -lt $retries ]
do
response=$(curl -s -o /dev/null -w "%{http_code}" $url)
if [ $response -eq 200 ]; then
echo "Successful response: $response"
break
else
echo "Response: $response. Retrying..."
((count++))
sleep $sleepwait
fi
done

if [ $count -eq $retries ]; then
echo "Failed after $retries attempts."
exit 1
fi
8 changes: 1 addition & 7 deletions .ncurc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,5 @@ reject: [
# TODO: New versions from 2.2.0 onwards introduce a newer incompatible version of the ILP-Packet that is not compatible with the Mojaloop Specification
"ilp-packet",
# TODO: v6+ (ref: https://github.com/sindresorhus/get-port/releases/tag/v6.0.0) is an ESM library and thus not compatible with CommonJS. Future story needed to resolve.
"get-port",
# There seems to be an issue with seeding or migration with integration tests
# that lead to some tests failing down the line.
# Specifically test case "update transfer state to COMMITTED by FULFIL request"
# "savePayeeTransferResponse::failure Cannot read properties of undefined (reading 'settlementWindowId')"
# TODO: More investigation
"knex"
"get-port"
]
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,27 @@ If you want to run integration tests in a repetitive manner, you can startup the
npm run wait-4-docker
```

Start the Central-Ledger Service in the background, capturing the Process ID, so we can kill it when we are done. Alternatively you could also start the process in a separate terminal. This is a temporary work-around until the following issue can be addressed: https://github.com/mojaloop/project/issues/3112.

```bash
npm start > cl-service.log &
echo $! > /tmp/int-test-service.pid
```

You can access the Central-Ledger Service log in another terminal with `tail -f cl-service.log`.

Run the Integration Tests

```bash
npm run test:int
```

Kill the background Central-Ledger Service

```bash
kill $(cat /tmp/int-test-service.pid)
```

- Running inside docker

Start containers required for Integration Tests, including a `central-ledger` container which will be used as a proxy shell.
Expand Down
13 changes: 6 additions & 7 deletions audit-ci.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@
"GHSA-9c47-m6qq-7p4h", // https://github.com/advisories/GHSA-9c47-m6qq-7p4h
// TODO: Upgrade jsonwebtoken in the central-services-shared lib --> https://github.com/mojaloop/project/issues/3097
"GHSA-qwph-4952-7xr6", // https://github.com/advisories/GHSA-qwph-4952-7xr6
"GHSA-hjrf-2m68-5959", // https://github.com/advisories/GHSA-hjrf-2m68-5959
"GHSA-27h2-hvpr-p74q", // https://github.com/advisories/GHSA-27h2-hvpr-p74q
// Unable to upgrade Knex due to the following issue:
// # There seems to be an issue with seeding or migration with integration tests
// # that lead to some tests failing down the line.
// # Specifically test case "update transfer state to COMMITTED by FULFIL request"
// # "savePayeeTransferResponse::failure Cannot read properties of undefined (reading 'settlementWindowId')"
// TODO: More investigation --> https://github.com/mojaloop/project/issues/3096
"GHSA-4jv9-3563-23j3" // https://github.com/advisories/GHSA-4jv9-3563-23j3
// Knex dependency has been upgraded to v2.4x as advised by this advisory. Not sure why its still reporting it as an issue?
// TODO: Investigate as to why this is still being reported even though Knex was upgraded! :(
"GHSA-4jv9-3563-23j3", // https://github.com/advisories/GHSA-4jv9-3563-23j3
// TODO: To be investigated
"GHSA-rc47-6667-2j5j", // https://github.com/advisories/GHSA-rc47-6667-2j5j
]
}
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ services:
kafka-topics.sh --bootstrap-server kafka:29092 --create --if-not-exists --topic topic-transfer-position --replication-factor 1 --partitions 1
kafka-topics.sh --bootstrap-server kafka:29092 --create --if-not-exists --topic topic-transfer-fulfil --replication-factor 1 --partitions 1
kafka-topics.sh --bootstrap-server kafka:29092 --create --if-not-exists --topic topic-notification-event --replication-factor 1 --partitions 1
kafka-topics.sh --bootstrap-server kafka:29092 --create --if-not-exists --topic topic-transfer-get --replication-factor 1 --partitions 1
kafka-topics.sh --bootstrap-server kafka:29092 --create --if-not-exists --topic topic-admin-transfer --replication-factor 1 --partitions 1
echo -e 'Successfully created the following topics:'
kafka-topics.sh --bootstrap-server kafka:29092 --list
"
Expand Down
28 changes: 14 additions & 14 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
"test:xunit": "npx tape 'test/unit/**/**.test.js' | tap-xunit > ./test/results/xunit.xml",
"test:coverage": "npx nyc --reporter=lcov --reporter=text-summary tapes -- 'test/unit/**/**.test.js'",
"test:coverage-check": "npm run test:coverage && nyc check-coverage",
"test:int": "npm run migrate && TST_RETRY_COUNT=20 TST_RETRY_TIMEOUT=3 UV_THREADPOOL_SIZE=12 npx tape 'test/integration/**/*.test.js' | tap-spec",
"test:int": "npm run migrate && npx tape 'test/integration/**/*.test.js' | tap-spec",
"test:int:skipMigrate": "npx tape 'test/integration/**/*.test.js' | tap-spec",
"migrate": "npm run migrate:latest && npm run seed:run",
"migrate:latest": "npx knex $npm_package_config_knex migrate:latest",
"migrate:create": "npx knex migrate:make $npm_package_config_knex",
Expand Down Expand Up @@ -77,7 +78,7 @@
},
"dependencies": {
"@hapi/good": "9.0.1",
"@hapi/hapi": "21.2.0",
"@hapi/hapi": "21.2.1",
"@hapi/inert": "7.0.0",
"@hapi/joi": "17.1.1",
"@hapi/vision": "7.0.0",
Expand Down Expand Up @@ -108,7 +109,7 @@
"hapi-auth-bearer-token": "8.0.0",
"hapi-swagger": "15.0.0",
"ilp-packet": "2.2.0",
"knex": "2.3.0",
"knex": "2.4.2",
"lodash": "4.17.21",
"moment": "2.29.4",
"rc": "1.2.8",
Expand Down
Loading

0 comments on commit c376af8

Please sign in to comment.