feat(core): Add SSH key generation #6006

flipswitchingmonkey · 2023-04-18T16:43:40Z

No description provided.

github-actions · 2023-04-18T16:44:10Z

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

If fixing bug, added test to cover scenario.
If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

codecov · 2023-04-18T17:00:15Z

Codecov Report

Patch coverage: 55.96% and project coverage change: +0.17 🎉

Comparison is base (cd89489) 18.49% compared to head (d4aeac8) 18.67%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6006      +/-   ##
==========================================
+ Coverage   18.49%   18.67%   +0.17%     
==========================================
  Files        2563     2582      +19     
  Lines      116018   116479     +461     
  Branches    18118    18178      +60     
==========================================
+ Hits        21458    21751     +293     
- Misses      93930    94090     +160     
- Partials      630      638       +8

Impacted Files	Coverage Δ
packages/cli/src/License.ts	`62.19% <0.00%> (ø)`
packages/cli/src/Server.ts	`0.00% <0.00%> (ø)`
...ironment/versionControl/versionControlHelper.ee.ts	`0.00% <0.00%> (ø)`
packages/cli/src/sso/saml/saml.service.ee.ts	`29.67% <0.00%> (ø)`
...trol/middleware/versionControlEnabledMiddleware.ts	`38.46% <38.46%> (ø)`
...nments/versionControl/versionControl.service.ee.ts	`66.66% <66.66%> (ø)`
...nvironments/versionControl/versionControlHelper.ts	`78.26% <78.26%> (ø)`
...nts/versionControl/versionControl.controller.ee.ts	`90.00% <90.00%> (ø)`
...s/cli/src/environments/versionControl/constants.ts	`100.00% <100.00%> (ø)`
.../versionControl/types/versionControlPreferences.ts	`100.00% <100.00%> (ø)`

... and 31 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

krynble · 2023-04-19T09:19:33Z

packages/cli/src/Server.ts

+		// set up the initial environment
+		if (isVersionControlLicensed()) {
+			try {
+				await Container.get(VersionControlService).init();


Correct me if I'm wrong, but you want o have the service being initialised in 2 different situations:

When it's already set up, you just load or

When setting up the feature (i.e. connecting a repo) then you init the service

I'm asking because conditional init could lead to issues like requiring a user restart after enabling a license for the feature to work

That's true. Same for saml, which I will change as well.

github-actions · 2023-04-19T09:19:46Z

✅ All Cypress E2E specs passed

krynble · 2023-04-19T09:22:30Z

packages/cli/src/environments/versionControl/versionControl.service.ee.ts

+			where: { key: VERSION_CONTROL_PREFERENCES_DB_KEY },
+		});
+		if (loadedPrefs) {
+			const prefs = jsonParse<VersionControlPreferences>(loadedPrefs.value);


To be 100% safe, you can add this in a try...catch block

krynble · 2023-04-19T09:24:44Z

packages/cli/src/environments/versionControl/versionControl.service.ee.ts

+			loadedPrefs.value = settingsValue;
+			result = await Db.collections.Settings.save(loadedPrefs);
+		} else {
+			result = await Db.collections.Settings.save({


I'm not 100% sure, but considering the key is the primary key, I guess typeorm can correctly insert or update by simply calling save

krynble · 2023-04-19T09:25:56Z

packages/cli/src/environments/versionControl/versionControlHelper.ts

+
+export function isVersionControlLicensed() {
+	const license = Container.get(License);
+	return license.isVersionControlEnabled();


This got a bit confusing, we're using licensed and enabled to clarify this. Maybe rename the License class method to isVersionControlLicensed and we use this naming convention, WDYT?

I agree. I used Enabled because it seems like that was the convention so far within the license class. Will rename it.

github-actions · 2023-04-19T09:26:38Z

✅ All Cypress E2E specs passed

github-actions · 2023-04-19T11:29:56Z

✅ All Cypress E2E specs passed

github-actions · 2023-04-19T11:39:35Z

✅ All Cypress E2E specs passed

krynble · 2023-04-19T14:31:28Z

packages/cli/src/environments/versionControl/versionControl.service.ee.ts

+		};
+	}
+
+	async generateAndSaveKeyPair(keyType: 'ed25519' | 'rsa' = 'ed25519') {


I'm guessing this function could check if a key already exists and throw an error in case it does - or at least in the future, check if there is a connected repo.

If a repo is connected, we want to prevent generating new keys at random WDYT?

This is really just a temporary helper function for now - once it is used in the context of the actual saving of settings, it will be adjusted to check. Right now there is just nothing to check yet and I wanted to keeps this PR small and just contain the SSH key related functions.

github-actions · 2023-04-19T14:31:40Z

✅ All Cypress E2E specs passed

github-actions · 2023-04-19T14:35:14Z

✅ All Cypress E2E specs passed

github-actions · 2023-04-19T15:43:50Z

✅ All Cypress E2E specs passed

* master: (199 commits) feat(editor): Add disable template experiment (#5963) feat(core): Upgrade google-timezones-json to use the correct timezone for Sao Paulo (#6042) fix(Code Node): Update vm2 to address CVE-2023-30547 (#6039) docs: Add proprietary license text (no-changelog) (#6038) test(n8n Node): Unit tests (no-changelog) refactor: Accumulate `loadOptions` from all node versions to validate (no-changelog) (#6014) Update CHANGELOG.md feat: Add variables e2e tests (no-changelog) (#6027) fix(editor): Fix typo in SSO upgrade link (#6031) fix(editor): Add correct add variable button message when no variables created (no-changelog) (#6028) docs: Add api notice to credentials for google sheets nodes (no-changelog) (#6024) fix(Notion Node): Update credential test to not require user permissions (#6022) fix(editor): Clean up demo and template callouts from workflows page (#6023) fix(editor): Fix memory leak in Node Detail View by correctly unsubscribing from event buses (#6021) fix(editor): SettingsSidebar should disconnect from push when navigating away (#6025) fix(editor): Use fake timers in useDebounce.test.ts to make the test less flaky (no-changelog) (#6029) docs: Update the info URL for updating n8n (no-changelog) (#6018) fix(core): Improve domain and url matching for extractDomain and extractUrl (#6010) feat(core): Add SSH key generation (#6006) fix(editor): Update SSO upgrade link (#6016) ... # Conflicts: # packages/editor-ui/src/components/WorkflowShareModal.ee.vue # packages/editor-ui/src/stores/workflows.ts # packages/editor-ui/src/views/NodeView.vue

* basic prefs and ssh key generation * review change * cleanup save * lint fix

janober · 2023-05-02T10:16:20Z

Got released with [email protected]

flipswitchingmonkey added 2 commits April 18, 2023 18:42

basic prefs and ssh key generation

6332410

Merge branch 'master' into pay-380-add-ssh-key-packages

4f703be

flipswitchingmonkey marked this pull request as ready for review April 18, 2023 16:44

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Apr 18, 2023

krynble reviewed Apr 19, 2023

View reviewed changes

krynble suggested changes Apr 19, 2023

View reviewed changes

review change

8d54370

flipswitchingmonkey requested a review from krynble April 19, 2023 11:49

flipswitchingmonkey added 2 commits April 19, 2023 16:19

cleanup save

ebbfef6

lint fix

d4aeac8

krynble reviewed Apr 19, 2023

View reviewed changes

flipswitchingmonkey requested a review from krynble April 19, 2023 14:35

krynble approved these changes Apr 19, 2023

View reviewed changes

flipswitchingmonkey merged commit 71ed1f4 into master Apr 19, 2023

flipswitchingmonkey deleted the pay-380-add-ssh-key-packages branch April 19, 2023 15:46

sunilrr pushed a commit to fl-g6/qp-n8n that referenced this pull request Apr 24, 2023

feat(core): Add SSH key generation (n8n-io#6006)

0e3293f

* basic prefs and ssh key generation * review change * cleanup save * lint fix

janober added the Released label May 2, 2023

feat(core): Add SSH key generation #6006

feat(core): Add SSH key generation #6006

Uh oh!

Conversation

flipswitchingmonkey commented Apr 18, 2023

Uh oh!

github-actions bot commented Apr 18, 2023

Uh oh!

codecov bot commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

janober commented May 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Apr 18, 2023 •

edited

Loading