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

test(core): Fix multiple parallel private key checks (no-changelog) #9012

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Apr 2, 2024

Prevent migration MoveSshKeysToDatabase1711390882123 from failing during multiple parallel private key checks during test runs, leading to this.cipher.encrypt(undefined).

  ● Metrics › should not return default metrics only when disabled

    TypeError: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined

      14 | 		const [key, iv] = this.getKeyAndIv(salt);
      15 | 		const cipher = createCipheriv('aes-256-cbc', key, iv);
    > 16 | 		const encrypted = cipher.update(typeof data === 'string' ? data : JSON.stringify(data));
         | 		                         ^
      17 | 		return Buffer.concat([RANDOM_BYTES, salt, encrypted, cipher.final()]).toString('base64');
      18 | 	}
      19 |

      at Cipher.encrypt (../core/src/Cipher.ts:16:28)
      at MoveSshKeysToDatabase1711390882123.up (src/databases/migrations/common/1711390882123-MoveSshKeysToDatabase.ts:50:37)
      at MoveSshKeysToDatabase1711390882123.up (src/databases/utils/migrationHelpers.ts:186:5)
      at MigrationExecutor.executePendingMigrations (../../node_modules/.pnpm/@n8n+typeorm@0.3.20-7_@sentry+node@7.87.0_ioredis@5.3.2_mysql2@2.3.3_pg@8.11.3_sqlite3@5.1.7/node_modules/src/migration/MigrationExecutor.ts:336:17)
      at DataSource.runMigrations (../../node_modules/.pnpm/@n8n+typeorm@0.3.20-7_@sentry+node@7.87.0_ioredis@5.3.2_mysql2@2.3.3_pg@8.11.3_sqlite3@5.1.7/node_modules/src/data-source/DataSource.ts:404:13)
      at Object.migrate (src/Db.ts:80:2)
      at Object.init (test/integration/shared/testDb.ts:38:2)
      at Object.<anonymous> (test/integration/shared/utils/testServer.ts:93:3)

@ivov ivov requested a review from a team as a code owner April 2, 2024 07:19
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Apr 2, 2024
Copy link

cypress bot commented Apr 5, 2024

2 flaky tests on run #4605 ↗︎

0 343 12 0 Flakiness 2

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Project: n8n Commit: 45dc1f265e
Status: Passed Duration: 03:50 💡
Started: Apr 5, 2024 3:34 PM Ended: Apr 5, 2024 3:38 PM
Flakiness  5-ndv.cy.ts • 1 flaky test

View Output Video

Test Artifacts
NDV > should not retrieve remote options when required params throw errors Test Replay Screenshots Video
Flakiness  24-ndv-paired-item.cy.ts • 1 flaky test

View Output Video

Test Artifacts
NDV > resolves expression with default item when input node is not parent, while still pairing items Test Replay Screenshots Video

Review all test suite changes for PR #9012 ↗︎

Copy link
Contributor

github-actions bot commented Apr 5, 2024

✅ All Cypress E2E specs passed

Copy link
Contributor

github-actions bot commented Apr 5, 2024

✅ All Cypress E2E specs passed

@ivov ivov merged commit ae3f164 into master Apr 5, 2024
55 checks passed
@ivov ivov deleted the test-fix-multiple-parallel-private-key-checks branch April 5, 2024 15:45
@janober
Copy link
Member

janober commented Apr 10, 2024

Got released with n8n@1.37.0

@mbakgun
Copy link
Contributor

mbakgun commented Apr 10, 2024

Hello guys,

After upgrading to the latest version of n8n, my Docker environment became stuck and is not booting up.

Error :

2024-04-10T20:53:02.036277549Z 2024-04-10T20:53:02.035Z | warn     | Migrations in progress, please do NOT stop the process. "{ file: 'migrationHelpers.js', function: 'logMigrationStart' }"
2024-04-10T20:53:02.036636033Z 2024-04-10T20:53:02.036Z | info     | Starting migration MoveSshKeysToDatabase1711390882123 "{ file: 'migrationHelpers.js', function: 'logMigrationStart' }"
2024-04-10T20:53:02.039543609Z Migration "MoveSshKeysToDatabase1711390882123" failed, error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'key = 'features.sourceControl.sshKeys'' at line 1
2024-04-10T20:53:02.040756987Z 2024-04-10T20:53:02.040Z | error    | Error: There was an error running database migrations "{ file: 'LoggerProxy.js', function: 'exports.error' }"
2024-04-10T20:53:02.041059367Z 2024-04-10T20:53:02.040Z | error    | QueryFailedError: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'key = 'features.sourceControl.sshKeys'' at line 1 "{ file: 'LoggerProxy.js', function: 'exports.error' }"
2024-04-10T20:53:06.209095258Z User settings loaded from: /root/.n8n/config

Versions :

  • MySQL 8.0.32
  • N8N 1.37.0 (Even downgrading is not effective and isn't working at all.)

Any recommendations are appreciated. (Please except migrate to the PostgreSQL) 🙏
@ivov , @despairblue

@netroy
Copy link
Member

netroy commented Apr 10, 2024

thanks for letting us know.
we'll fix this and release a patch tomorrow.
until then please downgrade 🙏🏽.

@jalvesbsolus
Copy link

@netroy

await runQuery(`DELETE ${settings} WHERE WHERE key = 'features.sourceControl.sshKeys';`);
I think it is "where" duplicated

@netroy
Copy link
Member

netroy commented Apr 10, 2024

Yeah. the query is invalid because of the double WHERE, but also invalid for MySQL because of the wrong quotes.

One issue here is that we deprecated MySQL support almost a year ago, and don't test on MySQL as extensively anymore.
But since this is a quick fix, we'll sort this out tomorrow morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants