diff --git a/.github/workflows/docker-base-image.yml b/.github/workflows/docker-base-image.yml index c32160e763..c2b5c9b6e6 100644 --- a/.github/workflows/docker-base-image.yml +++ b/.github/workflows/docker-base-image.yml @@ -20,26 +20,28 @@ jobs: - uses: actions/checkout@v4.1.1 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./docker/images/n8n-base/Dockerfile diff --git a/.github/workflows/docker-images-benchmark.yml b/.github/workflows/docker-images-benchmark.yml index b0aa6e997d..d4bf2f98a0 100644 --- a/.github/workflows/docker-images-benchmark.yml +++ b/.github/workflows/docker-images-benchmark.yml @@ -19,20 +19,22 @@ jobs: - uses: actions/checkout@v4.1.1 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./packages/@n8n/benchmark/Dockerfile diff --git a/.github/workflows/docker-images-custom.yml b/.github/workflows/docker-images-custom.yml new file mode 100644 index 0000000000..dfc9d77a03 --- /dev/null +++ b/.github/workflows/docker-images-custom.yml @@ -0,0 +1,83 @@ +name: Docker Custom Image CI +run-name: Build ${{ inputs.branch }} - ${{ inputs.user }} + +on: + workflow_dispatch: + inputs: + branch: + description: 'GitHub branch to create image off.' + required: true + tag: + description: 'Name of the docker tag to create.' + required: true + merge-master: + description: 'Merge with master.' + type: boolean + required: true + default: false + user: + description: '' + required: false + default: 'none' + start-url: + description: 'URL to call after workflow is kicked off.' + required: false + default: '' + success-url: + description: 'URL to call after Docker Image got built successfully.' + required: false + default: '' + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: Call Start URL - optionally + if: ${{ github.event.inputs.start-url != '' }} + run: curl -v -X POST -d 'url=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' ${{github.event.inputs.start-url}} || echo "" + shell: bash + + - name: Checkout + uses: actions/checkout@v4.1.1 + with: + ref: ${{ github.event.inputs.branch }} + + - name: Merge Master - optionally + if: github.event.inputs.merge-master + run: git remote add upstream https://github.com/n8n-io/n8n.git -f; git merge upstream/master --allow-unrelated-histories || echo "" + shell: bash + + - name: Set up QEMU + uses: docker/setup-qemu-action@v3.3.0 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3.8.0 + + - name: Login to GHCR + uses: docker/login-action@v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Build and push image to GHCR + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false + with: + context: . + file: ./docker/images/n8n-custom/Dockerfile + build-args: | + N8N_RELEASE_TYPE=development + platforms: linux/amd64 + provenance: false + push: true + cache-from: type=gha + cache-to: type=gha,mode=max + tags: ghcr.io/${{ github.repository_owner }}/n8n:${{ inputs.tag }} + + - name: Call Success URL - optionally + if: ${{ github.event.inputs.success-url != '' }} + run: curl -v ${{github.event.inputs.success-url}} || echo "" + shell: bash diff --git a/.github/workflows/docker-images-nightly.yml b/.github/workflows/docker-images-nightly.yml index b6831b6399..c924f8b2c1 100644 --- a/.github/workflows/docker-images-nightly.yml +++ b/.github/workflows/docker-images-nightly.yml @@ -1,74 +1,42 @@ name: Docker Nightly Image CI -run-name: Build ${{ inputs.branch }} - ${{ inputs.user }} on: schedule: - - cron: '0 1 * * *' + - cron: '0 0 * * *' workflow_dispatch: - inputs: - branch: - description: 'GitHub branch to create image off.' - required: true - default: 'master' - tag: - description: 'Name of the docker tag to create.' - required: true - default: 'nightly' - merge-master: - description: 'Merge with master.' - type: boolean - required: true - default: false - user: - description: '' - required: false - default: 'schedule' - start-url: - description: 'URL to call after workflow is kicked off.' - required: false - default: '' - success-url: - description: 'URL to call after Docker Image got built successfully.' - required: false - default: '' - -env: - N8N_TAG: ${{ inputs.tag || 'nightly' }} jobs: build: runs-on: ubuntu-latest - steps: - - name: Call Start URL - optionally - run: | - [[ "${{github.event.inputs.start-url}}" != "" ]] && curl -v -X POST -d 'url=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' ${{github.event.inputs.start-url}} || echo "" - shell: bash - - name: Checkout uses: actions/checkout@v4.1.1 with: - ref: ${{ github.event.inputs.branch || 'master' }} + ref: master - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 + + - name: Login to GHCR + uses: docker/login-action@v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - - name: Merge Master - optionally - run: | - [[ "${{github.event.inputs.merge-master}}" == "true" ]] && git remote add upstream https://github.com/n8n-io/n8n.git -f; git merge upstream/master --allow-unrelated-histories || echo "" - shell: bash - - - name: Build and push to DockerHub - uses: docker/build-push-action@v5.1.0 + - name: Build and push image to GHCR and DockerHub + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./docker/images/n8n-custom/Dockerfile @@ -79,24 +47,6 @@ jobs: push: true cache-from: type=gha cache-to: type=gha,mode=max - tags: ${{ secrets.DOCKER_USERNAME }}/n8n:${{ env.N8N_TAG }} - - - name: Login to GitHub Container Registry - if: env.N8N_TAG == 'nightly' - uses: docker/login-action@v3.0.0 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Push image to GHCR - if: env.N8N_TAG == 'nightly' - run: | - docker buildx imagetools create \ - --tag ghcr.io/${{ github.repository_owner }}/n8n:nightly \ + tags: | + ghcr.io/${{ github.repository_owner }}/n8n:nightly ${{ secrets.DOCKER_USERNAME }}/n8n:nightly - - - name: Call Success URL - optionally - run: | - [[ "${{github.event.inputs.success-url}}" != "" ]] && curl -v ${{github.event.inputs.success-url}} || echo "" - shell: bash diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 9196a7fccb..b95350a577 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -73,26 +73,28 @@ jobs: fetch-depth: 0 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: ./docker/images/n8n build-args: | diff --git a/.github/workflows/release-push-to-channel.yml b/.github/workflows/release-push-to-channel.yml index 3eda6d4ebb..9cb3a99b63 100644 --- a/.github/workflows/release-push-to-channel.yml +++ b/.github/workflows/release-push-to-channel.yml @@ -34,7 +34,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - uses: docker/login-action@v3.0.0 + - uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} @@ -46,7 +46,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - uses: docker/login-action@v3.0.0 + - uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} diff --git a/cypress/e2e/16-webhook-node.cy.ts b/cypress/e2e/16-webhook-node.cy.ts index 3d6c1049a2..193ada0bcc 100644 --- a/cypress/e2e/16-webhook-node.cy.ts +++ b/cypress/e2e/16-webhook-node.cy.ts @@ -250,7 +250,7 @@ describe('Webhook Trigger node', () => { }); // add credentials workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); credentialsModal.actions.fillCredentialsForm(); @@ -293,7 +293,7 @@ describe('Webhook Trigger node', () => { }); // add credentials workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); credentialsModal.actions.fillCredentialsForm(); diff --git a/cypress/e2e/17-sharing.cy.ts b/cypress/e2e/17-sharing.cy.ts index 30a990fb28..60eb474b07 100644 --- a/cypress/e2e/17-sharing.cy.ts +++ b/cypress/e2e/17-sharing.cy.ts @@ -297,10 +297,9 @@ describe('Credential Usage in Cross Shared Workflows', () => { workflowsPage.actions.createWorkflowFromCard(); workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); - // Only the credential in this project (+ the 'Create new' option) should - // be in the dropdown + // Only the credential in this project should be in the dropdown workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.length', 2); + getVisibleSelect().find('li').should('have.length', 1); }); it('should only show credentials in their personal project for members', () => { @@ -325,10 +324,9 @@ describe('Credential Usage in Cross Shared Workflows', () => { workflowsPage.actions.createWorkflowFromCard(); workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); - // Only the own credential the shared one (+ the 'Create new' option) - // should be in the dropdown + // Only the own credential the shared one should be in the dropdown workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.length', 3); + getVisibleSelect().find('li').should('have.length', 2); }); it('should only show credentials in their personal project for members if the workflow was shared with them', () => { @@ -355,10 +353,9 @@ describe('Credential Usage in Cross Shared Workflows', () => { workflowsPage.getters.workflowCardContent(workflowName).click(); workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); - // Only the own credential the shared one (+ the 'Create new' option) - // should be in the dropdown + // Only the own credential the shared one should be in the dropdown workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.length', 2); + getVisibleSelect().find('li').should('have.length', 1); }); it("should show all credentials from all personal projects the workflow's been shared into for the global owner", () => { @@ -400,10 +397,9 @@ describe('Credential Usage in Cross Shared Workflows', () => { workflowsPage.getters.workflowCardContent(workflowName).click(); workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); - // Only the personal credentials of the workflow owner and the global owner - // should show up. + // Only the personal credentials of the workflow owner and the global owner should show up. workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.length', 4); + getVisibleSelect().find('li').should('have.length', 3); }); it('should show all personal credentials if the global owner owns the workflow', () => { @@ -421,6 +417,6 @@ describe('Credential Usage in Cross Shared Workflows', () => { // Show all personal credentials workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.have.length', 2); + getVisibleSelect().find('li').should('have.have.length', 1); }); }); diff --git a/cypress/e2e/2-credentials.cy.ts b/cypress/e2e/2-credentials.cy.ts index cda01c71a3..260d5f63a0 100644 --- a/cypress/e2e/2-credentials.cy.ts +++ b/cypress/e2e/2-credentials.cy.ts @@ -31,7 +31,7 @@ function createNotionCredential() { workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME); workflowPage.actions.openNode(NOTION_NODE_NAME); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.actions.fillCredentialsForm(); cy.get('body').type('{esc}'); workflowPage.actions.deleteNode(NOTION_NODE_NAME); @@ -79,7 +79,7 @@ describe('Credentials', () => { workflowPage.getters.canvasNodes().last().click(); cy.get('body').type('{enter}'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); credentialsModal.getters.credentialAuthTypeRadioButtons().should('have.length', 2); credentialsModal.getters.credentialAuthTypeRadioButtons().first().click(); @@ -99,7 +99,7 @@ describe('Credentials', () => { cy.get('body').type('{enter}'); workflowPage.getters.nodeCredentialsSelect().click(); // Add oAuth credentials - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); credentialsModal.getters.credentialAuthTypeRadioButtons().should('have.length', 2); credentialsModal.getters.credentialAuthTypeRadioButtons().first().click(); @@ -107,14 +107,13 @@ describe('Credentials', () => { cy.get('.el-message-box').find('button').contains('Close').click(); workflowPage.getters.nodeCredentialsSelect().click(); // Add Service account credentials - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); credentialsModal.getters.credentialAuthTypeRadioButtons().should('have.length', 2); credentialsModal.getters.credentialAuthTypeRadioButtons().last().click(); credentialsModal.actions.fillCredentialsForm(); - // Both (+ the 'Create new' option) should be in the dropdown workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').should('have.length.greaterThan', 2); + getVisibleSelect().find('li').should('have.length', 3); }); it('should correctly render required and optional credentials', () => { @@ -130,13 +129,13 @@ describe('Credentials', () => { workflowPage.getters.nodeCredentialsSelect().should('have.length', 2); workflowPage.getters.nodeCredentialsSelect().first().click(); - getVisibleSelect().find('li').contains('Create New Credential').click(); + workflowPage.getters.nodeCredentialsCreateOption().first().click(); // This one should show auth type selector credentialsModal.getters.credentialAuthTypeRadioButtons().should('have.length', 2); cy.get('body').type('{esc}'); workflowPage.getters.nodeCredentialsSelect().last().click(); - getVisibleSelect().find('li').contains('Create New Credential').click(); + workflowPage.getters.nodeCredentialsCreateOption().last().click(); // This one should not show auth type selector credentialsModal.getters.credentialsAuthTypeSelector().should('not.exist'); }); @@ -148,7 +147,7 @@ describe('Credentials', () => { workflowPage.getters.canvasNodes().last().click(); cy.get('body').type('{enter}'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialsAuthTypeSelector().should('not.exist'); credentialsModal.actions.fillCredentialsForm(); workflowPage.getters @@ -164,7 +163,7 @@ describe('Credentials', () => { workflowPage.getters.canvasNodes().last().click(); cy.get('body').type('{enter}'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.actions.fillCredentialsForm(); workflowPage.getters .nodeCredentialsSelect() @@ -189,7 +188,7 @@ describe('Credentials', () => { workflowPage.getters.canvasNodes().last().click(); cy.get('body').type('{enter}'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.actions.fillCredentialsForm(); workflowPage.getters.nodeCredentialsEditButton().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); @@ -232,7 +231,7 @@ describe('Credentials', () => { cy.getByTestId('credential-select').click(); cy.contains('Adalo API').click(); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.actions.fillCredentialsForm(); workflowPage.getters.nodeCredentialsEditButton().click(); credentialsModal.getters.credentialsEditModal().should('be.visible'); @@ -296,7 +295,7 @@ describe('Credentials', () => { workflowPage.getters.nodeCredentialsSelect().should('exist'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.actions.fillCredentialsForm(); workflowPage.getters .nodeCredentialsSelect() @@ -325,7 +324,7 @@ describe('Credentials', () => { workflowPage.actions.addNodeToCanvas('Slack', true, true, 'Get a channel'); workflowPage.getters.nodeCredentialsSelect().should('exist'); workflowPage.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialAuthTypeRadioButtons().first().click(); nodeDetailsView.getters.copyInput().should('not.exist'); }); diff --git a/cypress/e2e/21-community-nodes.cy.ts b/cypress/e2e/21-community-nodes.cy.ts index 17f82ec573..283c08d557 100644 --- a/cypress/e2e/21-community-nodes.cy.ts +++ b/cypress/e2e/21-community-nodes.cy.ts @@ -89,7 +89,7 @@ describe('Community and custom nodes in canvas', () => { workflowPage.actions.addNodeToCanvas('Manual'); workflowPage.actions.addNodeToCanvas('E2E Node with native n8n credential', true, true); workflowPage.getters.nodeCredentialsLabel().click(); - cy.contains('Create New Credential').click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.editCredentialModal().should('be.visible'); credentialsModal.getters.editCredentialModal().should('contain.text', 'Notion API'); }); @@ -98,7 +98,7 @@ describe('Community and custom nodes in canvas', () => { workflowPage.actions.addNodeToCanvas('Manual'); workflowPage.actions.addNodeToCanvas('E2E Node with custom credential', true, true); workflowPage.getters.nodeCredentialsLabel().click(); - cy.contains('Create New Credential').click(); + workflowPage.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.editCredentialModal().should('be.visible'); credentialsModal.getters.editCredentialModal().should('contain.text', 'Custom E2E Credential'); }); diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index efd18bca74..197d585256 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -367,7 +367,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account project 1'); ndv.getters.backToCanvas().click(); @@ -382,7 +382,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account project 1'); ndv.getters.backToCanvas().click(); @@ -396,7 +396,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account project 2'); ndv.getters.backToCanvas().click(); @@ -407,7 +407,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account project 2'); ndv.getters.backToCanvas().click(); @@ -425,7 +425,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account personal project'); ndv.getters.backToCanvas().click(); @@ -436,7 +436,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') - .should('have.length', 2) + .should('have.length', 1) .first() .should('contain.text', 'Notion account personal project'); }); diff --git a/cypress/e2e/45-ai-assistant.cy.ts b/cypress/e2e/45-ai-assistant.cy.ts index 9d33811363..157c656b46 100644 --- a/cypress/e2e/45-ai-assistant.cy.ts +++ b/cypress/e2e/45-ai-assistant.cy.ts @@ -5,7 +5,6 @@ import { GMAIL_NODE_NAME, SCHEDULE_TRIGGER_NODE_NAME } from '../constants'; import { CredentialsModal, CredentialsPage, NDV, WorkflowPage } from '../pages'; import { AIAssistant } from '../pages/features/ai-assistant'; import { NodeCreator } from '../pages/features/node-creator'; -import { getVisibleSelect } from '../utils'; const wf = new WorkflowPage(); const ndv = new NDV(); @@ -434,7 +433,7 @@ describe('AI Assistant Credential Help', () => { wf.actions.addNodeToCanvas('Slack', true, true, 'Get a channel'); wf.getters.nodeCredentialsSelect().should('exist'); wf.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + wf.getters.nodeCredentialsCreateOption().click(); credentialsModal.getters.credentialAuthTypeRadioButtons().first().click(); ndv.getters.copyInput().should('not.exist'); credentialsModal.getters.oauthConnectButton().should('have.length', 1); @@ -467,7 +466,7 @@ describe('AI Assistant Credential Help', () => { wf.actions.addNodeToCanvas('Microsoft Outlook', true, true, 'Get a calendar'); wf.getters.nodeCredentialsSelect().should('exist'); wf.getters.nodeCredentialsSelect().click(); - getVisibleSelect().find('li').last().click(); + wf.getters.nodeCredentialsCreateOption().click(); ndv.getters.copyInput().should('not.exist'); credentialsModal.getters.oauthConnectButton().should('have.length', 1); credentialsModal.getters.credentialInputs().should('have.length', 1); diff --git a/docker/images/n8n/README.md b/docker/images/n8n/README.md index f7b45f9467..73dbb7557b 100644 --- a/docker/images/n8n/README.md +++ b/docker/images/n8n/README.md @@ -73,7 +73,7 @@ docker run -it --rm \ -p 5678:5678 \ -v ~/.n8n:/home/node/.n8n \ docker.n8n.io/n8nio/n8n \ - n8n start --tunnel + start --tunnel ``` ## Persist data diff --git a/package.json b/package.json index 9002c8dd4e..6cbaac3044 100644 --- a/package.json +++ b/package.json @@ -90,6 +90,7 @@ "ws": ">=8.17.1" }, "patchedDependencies": { + "bull@4.12.1": "patches/bull@4.12.1.patch", "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch", "pyodide@0.23.4": "patches/pyodide@0.23.4.patch", "@types/express-serve-static-core@4.17.43": "patches/@types__express-serve-static-core@4.17.43.patch", diff --git a/packages/@n8n/task-runner/src/__tests__/task-runner-sentry.test.ts b/packages/@n8n/task-runner/src/__tests__/task-runner-sentry.test.ts new file mode 100644 index 0000000000..6fe14a484a --- /dev/null +++ b/packages/@n8n/task-runner/src/__tests__/task-runner-sentry.test.ts @@ -0,0 +1,178 @@ +import type { ErrorEvent } from '@sentry/types'; +import { mock } from 'jest-mock-extended'; +import type { ErrorReporter } from 'n8n-core'; + +import { TaskRunnerSentry } from '../task-runner-sentry'; + +describe('TaskRunnerSentry', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('filterOutUserCodeErrors', () => { + const sentry = new TaskRunnerSentry( + { + dsn: 'https://sentry.io/123', + n8nVersion: '1.0.0', + environment: 'local', + deploymentName: 'test', + }, + mock(), + ); + + it('should filter out user code errors', () => { + const event: ErrorEvent = { + type: undefined, + exception: { + values: [ + { + type: 'ReferenceError', + value: 'fetch is not defined', + stacktrace: { + frames: [ + { + filename: 'app:///dist/js-task-runner/js-task-runner.js', + module: 'js-task-runner:js-task-runner', + function: 'JsTaskRunner.executeTask', + }, + { + filename: 'app:///dist/js-task-runner/js-task-runner.js', + module: 'js-task-runner:js-task-runner', + function: 'JsTaskRunner.runForAllItems', + }, + { + filename: '', + module: '', + function: 'new Promise', + }, + { + filename: 'app:///dist/js-task-runner/js-task-runner.js', + module: 'js-task-runner:js-task-runner', + function: 'result', + }, + { + filename: 'node:vm', + module: 'node:vm', + function: 'runInContext', + }, + { + filename: 'node:vm', + module: 'node:vm', + function: 'Script.runInContext', + }, + { + filename: 'evalmachine.', + module: 'evalmachine.', + function: '?', + }, + { + filename: 'evalmachine.', + module: 'evalmachine.', + function: 'VmCodeWrapper', + }, + { + filename: '', + module: '', + function: 'new Promise', + }, + { + filename: 'evalmachine.', + module: 'evalmachine.', + }, + ], + }, + mechanism: { type: 'onunhandledrejection', handled: false }, + }, + ], + }, + event_id: '18bb78bb3d9d44c4acf3d774c2cfbfd8', + platform: 'node', + contexts: { + trace: { trace_id: '3c3614d33a6b47f09b85ec7d2710acea', span_id: 'ad00fdf6d6173aeb' }, + runtime: { name: 'node', version: 'v20.17.0' }, + }, + }; + + expect(sentry.filterOutUserCodeErrors(event)).toBe(true); + }); + }); + + describe('initIfEnabled', () => { + const mockErrorReporter = mock(); + + it('should not configure sentry if dsn is not set', async () => { + const sentry = new TaskRunnerSentry( + { + dsn: '', + n8nVersion: '1.0.0', + environment: 'local', + deploymentName: 'test', + }, + mockErrorReporter, + ); + + await sentry.initIfEnabled(); + + expect(mockErrorReporter.init).not.toHaveBeenCalled(); + }); + + it('should configure sentry if dsn is set', async () => { + const sentry = new TaskRunnerSentry( + { + dsn: 'https://sentry.io/123', + n8nVersion: '1.0.0', + environment: 'local', + deploymentName: 'test', + }, + mockErrorReporter, + ); + + await sentry.initIfEnabled(); + + expect(mockErrorReporter.init).toHaveBeenCalledWith({ + dsn: 'https://sentry.io/123', + beforeSendFilter: sentry.filterOutUserCodeErrors, + release: '1.0.0', + environment: 'local', + serverName: 'test', + serverType: 'task_runner', + }); + }); + }); + + describe('shutdown', () => { + const mockErrorReporter = mock(); + + it('should not shutdown sentry if dsn is not set', async () => { + const sentry = new TaskRunnerSentry( + { + dsn: '', + n8nVersion: '1.0.0', + environment: 'local', + deploymentName: 'test', + }, + mockErrorReporter, + ); + + await sentry.shutdown(); + + expect(mockErrorReporter.shutdown).not.toHaveBeenCalled(); + }); + + it('should shutdown sentry if dsn is set', async () => { + const sentry = new TaskRunnerSentry( + { + dsn: 'https://sentry.io/123', + n8nVersion: '1.0.0', + environment: 'local', + deploymentName: 'test', + }, + mockErrorReporter, + ); + + await sentry.shutdown(); + + expect(mockErrorReporter.shutdown).toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/@n8n/task-runner/src/start.ts b/packages/@n8n/task-runner/src/start.ts index cbc381c2fd..35b928e9a8 100644 --- a/packages/@n8n/task-runner/src/start.ts +++ b/packages/@n8n/task-runner/src/start.ts @@ -1,16 +1,16 @@ import './polyfills'; import { Container } from '@n8n/di'; -import type { ErrorReporter } from 'n8n-core'; import { ensureError, setGlobalState } from 'n8n-workflow'; import { MainConfig } from './config/main-config'; import type { HealthCheckServer } from './health-check-server'; import { JsTaskRunner } from './js-task-runner/js-task-runner'; +import { TaskRunnerSentry } from './task-runner-sentry'; let healthCheckServer: HealthCheckServer | undefined; let runner: JsTaskRunner | undefined; let isShuttingDown = false; -let errorReporter: ErrorReporter | undefined; +let sentry: TaskRunnerSentry | undefined; function createSignalHandler(signal: string, timeoutInS = 10) { return async function onSignal() { @@ -33,9 +33,9 @@ function createSignalHandler(signal: string, timeoutInS = 10) { void healthCheckServer?.stop(); } - if (errorReporter) { - await errorReporter.shutdown(); - errorReporter = undefined; + if (sentry) { + await sentry.shutdown(); + sentry = undefined; } } catch (e) { const error = ensureError(e); @@ -54,20 +54,8 @@ void (async function start() { defaultTimezone: config.baseRunnerConfig.timezone, }); - const { dsn } = config.sentryConfig; - - if (dsn) { - const { ErrorReporter } = await import('n8n-core'); - errorReporter = Container.get(ErrorReporter); - const { deploymentName, environment, n8nVersion } = config.sentryConfig; - await errorReporter.init({ - serverType: 'task_runner', - dsn, - serverName: deploymentName, - environment, - release: n8nVersion, - }); - } + sentry = Container.get(TaskRunnerSentry); + await sentry.initIfEnabled(); runner = new JsTaskRunner(config); runner.on('runner:reached-idle-timeout', () => { diff --git a/packages/@n8n/task-runner/src/task-runner-sentry.ts b/packages/@n8n/task-runner/src/task-runner-sentry.ts new file mode 100644 index 0000000000..5443a98d5c --- /dev/null +++ b/packages/@n8n/task-runner/src/task-runner-sentry.ts @@ -0,0 +1,62 @@ +import { Service } from '@n8n/di'; +import type { ErrorEvent, Exception } from '@sentry/types'; +import { ErrorReporter } from 'n8n-core'; + +import { SentryConfig } from './config/sentry-config'; + +/** + * Sentry service for the task runner. + */ +@Service() +export class TaskRunnerSentry { + constructor( + private readonly config: SentryConfig, + private readonly errorReporter: ErrorReporter, + ) {} + + async initIfEnabled() { + const { dsn, n8nVersion, environment, deploymentName } = this.config; + + if (!dsn) return; + + await this.errorReporter.init({ + serverType: 'task_runner', + dsn, + release: n8nVersion, + environment, + serverName: deploymentName, + beforeSendFilter: this.filterOutUserCodeErrors, + }); + } + + async shutdown() { + if (!this.config.dsn) return; + + await this.errorReporter.shutdown(); + } + + /** + * Filter out errors originating from user provided code. + * It is possible for users to create code that causes unhandledrejections + * that end up in the sentry error reporting. + */ + filterOutUserCodeErrors = (event: ErrorEvent) => { + const error = event?.exception?.values?.[0]; + + return error ? this.isUserCodeError(error) : false; + }; + + /** + * Check if the error is originating from user provided code. + * It is possible for users to create code that causes unhandledrejections + * that end up in the sentry error reporting. + */ + private isUserCodeError(error: Exception) { + const frames = error.stacktrace?.frames; + if (!frames) return false; + + return frames.some( + (frame) => frame.filename === 'node:vm' && frame.function === 'runInContext', + ); + } +} diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index c3a604faa4..15c3a59969 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -349,15 +349,6 @@ export const schema = { }, }, - sourceControl: { - defaultKeyPairType: { - doc: 'Default SSH key type to use when generating SSH keys', - format: ['rsa', 'ed25519'] as const, - default: 'ed25519', - env: 'N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE', - }, - }, - workflowHistory: { enabled: { doc: 'Whether to save workflow history versions', diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts index 5f39e1ebf1..96f2f8ecdb 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts @@ -1,86 +1,261 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; -import mock from 'jest-mock-extended/lib/Mock'; +import { mock, captor } from 'jest-mock-extended'; import { Cipher, type InstanceSettings } from 'n8n-core'; -import { ApplicationError, deepCopy } from 'n8n-workflow'; +import fsp from 'node:fs/promises'; -import type { CredentialsEntity } from '@/databases/entities/credentials-entity'; import type { SharedCredentials } from '@/databases/entities/shared-credentials'; -import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; -import { mockInstance } from '@test/mocking'; +import type { SharedWorkflow } from '@/databases/entities/shared-workflow'; +import type { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; +import type { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; +import type { TagRepository } from '@/databases/repositories/tag.repository'; +import type { WorkflowTagMappingRepository } from '@/databases/repositories/workflow-tag-mapping.repository'; +import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import type { VariablesService } from '../../variables/variables.service.ee'; import { SourceControlExportService } from '../source-control-export.service.ee'; -// https://github.com/jestjs/jest/issues/4715 -function deepSpyOn(object: O, methodName: M) { - const spy = jest.fn(); - - const originalMethod = object[methodName]; - - if (typeof originalMethod !== 'function') { - throw new ApplicationError(`${methodName.toString()} is not a function`, { level: 'warning' }); - } - - object[methodName] = function (...args: unknown[]) { - const clonedArgs = deepCopy(args); - spy(...clonedArgs); - return originalMethod.apply(this, args); - } as O[M]; - - return spy; -} - describe('SourceControlExportService', () => { + const cipher = Container.get(Cipher); + const sharedCredentialsRepository = mock(); + const sharedWorkflowRepository = mock(); + const workflowRepository = mock(); + const tagRepository = mock(); + const workflowTagMappingRepository = mock(); + const variablesService = mock(); + const service = new SourceControlExportService( mock(), - mock(), - mock(), - mock({ n8nFolder: '' }), + variablesService, + tagRepository, + sharedCredentialsRepository, + sharedWorkflowRepository, + workflowRepository, + workflowTagMappingRepository, + mock({ n8nFolder: '/mock/n8n' }), ); - describe('exportCredentialsToWorkFolder', () => { - it('should export credentials to work folder', async () => { - /** - * Arrange - */ - // @ts-expect-error Private method - const replaceSpy = deepSpyOn(service, 'replaceCredentialData'); + const fsWriteFile = jest.spyOn(fsp, 'writeFile'); - mockInstance(SharedCredentialsRepository).findByCredentialIds.mockResolvedValue([ + beforeEach(() => jest.clearAllMocks()); + + describe('exportCredentialsToWorkFolder', () => { + const credentialData = { + authUrl: 'test', + accessTokenUrl: 'test', + clientId: 'test', + clientSecret: 'test', + oauthTokenData: { + access_token: 'test', + token_type: 'test', + expires_in: 123, + refresh_token: 'test', + }, + }; + + const mockCredentials = mock({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: cipher.encrypt(credentialData), + }); + + it('should export credentials to work folder', async () => { + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ mock({ - credentials: mock({ - data: Container.get(Cipher).encrypt( - JSON.stringify({ - authUrl: 'test', - accessTokenUrl: 'test', - clientId: 'test', - clientSecret: 'test', - oauthTokenData: { - access_token: 'test', - token_type: 'test', - expires_in: 123, - refresh_token: 'test', - }, - }), - ), + credentials: mockCredentials, + project: mock({ + type: 'personal', + projectRelations: [ + { + role: 'project:personalOwner', + user: mock({ email: 'user@example.com' }), + }, + ], }), }), ]); - /** - * Act - */ - await service.exportCredentialsToWorkFolder([mock()]); + // Act + const result = await service.exportCredentialsToWorkFolder([mock()]); - /** - * Assert - */ - expect(replaceSpy).toHaveBeenCalledWith({ - authUrl: 'test', - accessTokenUrl: 'test', - clientId: 'test', - clientSecret: 'test', + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/cred1.json', + dataCaptor, + ); + expect(JSON.parse(dataCaptor.value)).toEqual({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'personal', + personalEmail: 'user@example.com', + }, }); }); + + it('should handle team project credentials', async () => { + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ + mock({ + credentials: mockCredentials, + project: mock({ + type: 'team', + id: 'team1', + name: 'Test Team', + }), + }), + ]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'cred1' }), + ]); + + // Assert + expect(result.count).toBe(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/cred1.json', + dataCaptor, + ); + expect(JSON.parse(dataCaptor.value)).toEqual({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'team', + teamId: 'team1', + teamName: 'Test Team', + }, + }); + }); + + it('should handle missing credentials', async () => { + // Arrange + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'cred1' }), + ]); + + // Assert + expect(result.missingIds).toHaveLength(1); + expect(result.missingIds?.[0]).toBe('cred1'); + }); + }); + + describe('exportTagsToWorkFolder', () => { + it('should export tags to work folder', async () => { + // Arrange + tagRepository.find.mockResolvedValue([mock()]); + workflowTagMappingRepository.find.mockResolvedValue([mock()]); + + // Act + const result = await service.exportTagsToWorkFolder(); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should not export empty tags', async () => { + // Arrange + tagRepository.find.mockResolvedValue([]); + + // Act + const result = await service.exportTagsToWorkFolder(); + + // Assert + expect(result.count).toBe(0); + expect(result.files).toHaveLength(0); + }); + }); + + describe('exportVariablesToWorkFolder', () => { + it('should export variables to work folder', async () => { + // Arrange + variablesService.getAllCached.mockResolvedValue([mock()]); + + // Act + const result = await service.exportVariablesToWorkFolder(); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should not export empty variables', async () => { + // Arrange + variablesService.getAllCached.mockResolvedValue([]); + + // Act + const result = await service.exportVariablesToWorkFolder(); + + // Assert + expect(result.count).toBe(0); + expect(result.files).toHaveLength(0); + }); + }); + + describe('exportWorkflowsToWorkFolder', () => { + it('should export workflows to work folder', async () => { + // Arrange + workflowRepository.findByIds.mockResolvedValue([mock()]); + sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([ + mock({ + project: mock({ + type: 'personal', + projectRelations: [{ role: 'project:personalOwner', user: mock() }], + }), + workflow: mock(), + }), + ]); + + // Act + const result = await service.exportWorkflowsToWorkFolder([mock()]); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should throw an error if workflow has no owner', async () => { + // Arrange + sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([ + mock({ + project: mock({ + type: 'personal', + projectRelations: [], + }), + workflow: mock({ + display: () => 'TestWorkflow', + }), + }), + ]); + + // Act & Assert + await expect(service.exportWorkflowsToWorkFolder([mock()])).rejects.toThrow( + 'Workflow TestWorkflow has no owner', + ); + }); }); }); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts index 0395186a28..4824b85ba6 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts @@ -1,6 +1,7 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; import { constants as fsConstants, accessSync } from 'fs'; +import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; import path from 'path'; @@ -16,10 +17,8 @@ import { getTrackingInformationFromPullResult, sourceControlFoldersExistCheck, } from '@/environments.ee/source-control/source-control-helper.ee'; -import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; -import type { SourceControlPreferences } from '@/environments.ee/source-control/types/source-control-preferences'; -import { License } from '@/license'; -import { mockInstance } from '@test/mocking'; +import type { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; +import type { License } from '@/license'; const pushResult: SourceControlledFile[] = [ { @@ -151,12 +150,13 @@ const pullResult: SourceControlledFile[] = [ }, ]; -const license = mockInstance(License); +const license = mock(); +const sourceControlPreferencesService = mock(); beforeAll(async () => { jest.resetAllMocks(); license.isSourceControlLicensed.mockReturnValue(true); - Container.get(SourceControlPreferencesService).getPreferences = () => ({ + sourceControlPreferencesService.getPreferences.mockReturnValue({ branchName: 'main', connected: true, repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', @@ -245,17 +245,4 @@ describe('Source Control', () => { workflowUpdates: 3, }); }); - - it('should class validate correct preferences', async () => { - const validPreferences: Partial = { - branchName: 'main', - repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', - branchReadOnly: false, - branchColor: '#5296D6', - }; - const validationResult = await Container.get( - SourceControlPreferencesService, - ).validateSourceControlPreferences(validPreferences); - expect(validationResult).toBeTruthy(); - }); }); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts new file mode 100644 index 0000000000..fff60bd566 --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts @@ -0,0 +1,180 @@ +import * as fastGlob from 'fast-glob'; +import { mock } from 'jest-mock-extended'; +import { type InstanceSettings } from 'n8n-core'; +import fsp from 'node:fs/promises'; + +import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; +import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; + +import { SourceControlImportService } from '../source-control-import.service.ee'; + +jest.mock('fast-glob'); + +describe('SourceControlImportService', () => { + const workflowRepository = mock(); + const service = new SourceControlImportService( + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + workflowRepository, + mock(), + mock({ n8nFolder: '/mock/n8n' }), + ); + + const globMock = fastGlob.default as unknown as jest.Mock, string[]>; + const fsReadFile = jest.spyOn(fsp, 'readFile'); + + beforeEach(() => jest.clearAllMocks()); + + describe('getRemoteVersionIdsFromFiles', () => { + const mockWorkflowFile = '/mock/workflow1.json'; + it('should parse workflow files correctly', async () => { + globMock.mockResolvedValue([mockWorkflowFile]); + + const mockWorkflowData = { + id: 'workflow1', + versionId: 'v1', + name: 'Test Workflow', + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockWorkflowData)); + + const result = await service.getRemoteVersionIdsFromFiles(); + expect(fsReadFile).toHaveBeenCalledWith(mockWorkflowFile, { encoding: 'utf8' }); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'workflow1', + versionId: 'v1', + name: 'Test Workflow', + }), + ); + }); + + it('should filter out files without valid workflow data', async () => { + globMock.mockResolvedValue(['/mock/invalid.json']); + + fsReadFile.mockResolvedValue('{}'); + + const result = await service.getRemoteVersionIdsFromFiles(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteCredentialsFromFiles', () => { + it('should parse credential files correctly', async () => { + globMock.mockResolvedValue(['/mock/credential1.json']); + + const mockCredentialData = { + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockCredentialData)); + + const result = await service.getRemoteCredentialsFromFiles(); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + }), + ); + }); + + it('should filter out files without valid credential data', async () => { + globMock.mockResolvedValue(['/mock/invalid.json']); + fsReadFile.mockResolvedValue('{}'); + + const result = await service.getRemoteCredentialsFromFiles(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteVariablesFromFile', () => { + it('should parse variables file correctly', async () => { + globMock.mockResolvedValue(['/mock/variables.json']); + + const mockVariablesData = [ + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2' }, + ]; + + fsReadFile.mockResolvedValue(JSON.stringify(mockVariablesData)); + + const result = await service.getRemoteVariablesFromFile(); + + expect(result).toEqual(mockVariablesData); + }); + + it('should return empty array if no variables file found', async () => { + globMock.mockResolvedValue([]); + + const result = await service.getRemoteVariablesFromFile(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteTagsAndMappingsFromFile', () => { + it('should parse tags and mappings file correctly', async () => { + globMock.mockResolvedValue(['/mock/tags.json']); + + const mockTagsData = { + tags: [{ id: 'tag1', name: 'Tag 1' }], + mappings: [{ workflowId: 'workflow1', tagId: 'tag1' }], + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockTagsData)); + + const result = await service.getRemoteTagsAndMappingsFromFile(); + + expect(result.tags).toEqual(mockTagsData.tags); + expect(result.mappings).toEqual(mockTagsData.mappings); + }); + + it('should return empty tags and mappings if no file found', async () => { + globMock.mockResolvedValue([]); + + const result = await service.getRemoteTagsAndMappingsFromFile(); + + expect(result.tags).toHaveLength(0); + expect(result.mappings).toHaveLength(0); + }); + }); + + describe('getLocalVersionIdsFromDb', () => { + const now = new Date(); + jest.useFakeTimers({ now }); + + it('should replace invalid updatedAt with current timestamp', async () => { + const mockWorkflows = [ + { + id: 'workflow1', + name: 'Test Workflow', + updatedAt: 'invalid-date', + }, + ] as unknown as WorkflowEntity[]; + + workflowRepository.find.mockResolvedValue(mockWorkflows); + + const result = await service.getLocalVersionIdsFromDb(); + + expect(result[0].updatedAt).toBe(now.toISOString()); + }); + }); +}); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts new file mode 100644 index 0000000000..d35c7ac9c7 --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts @@ -0,0 +1,27 @@ +import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; + +import { SourceControlPreferencesService } from '../source-control-preferences.service.ee'; +import type { SourceControlPreferences } from '../types/source-control-preferences'; + +describe('SourceControlPreferencesService', () => { + const instanceSettings = mock({ n8nFolder: '' }); + const service = new SourceControlPreferencesService( + instanceSettings, + mock(), + mock(), + mock(), + mock(), + ); + + it('should class validate correct preferences', async () => { + const validPreferences: Partial = { + branchName: 'main', + repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', + branchReadOnly: false, + branchColor: '#5296D6', + }; + const validationResult = await service.validateSourceControlPreferences(validPreferences); + expect(validationResult).toBeTruthy(); + }); +}); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts index 9024a0a32c..56b58646c9 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts @@ -10,6 +10,8 @@ describe('SourceControlService', () => { Container.get(InstanceSettings), mock(), mock(), + mock(), + mock(), ); const sourceControlService = new SourceControlService( mock(), diff --git a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts index d32758cd25..ec8e4efd22 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts @@ -1,5 +1,5 @@ import type { SourceControlledFile } from '@n8n/api-types'; -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import { rmSync } from 'fs'; import { Credentials, InstanceSettings, Logger } from 'n8n-core'; import { ApplicationError, type ICredentialDataDecryptedObject } from 'n8n-workflow'; @@ -44,6 +44,10 @@ export class SourceControlExportService { private readonly logger: Logger, private readonly variablesService: VariablesService, private readonly tagRepository: TagRepository, + private readonly sharedCredentialsRepository: SharedCredentialsRepository, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowTagMappingRepository: WorkflowTagMappingRepository, instanceSettings: InstanceSettings, ) { this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -106,17 +110,16 @@ export class SourceControlExportService { try { sourceControlFoldersExistCheck([this.workflowExportFolder]); const workflowIds = candidates.map((e) => e.id); - const sharedWorkflows = - await Container.get(SharedWorkflowRepository).findByWorkflowIds(workflowIds); - const workflows = await Container.get(WorkflowRepository).findByIds(workflowIds); + const sharedWorkflows = await this.sharedWorkflowRepository.findByWorkflowIds(workflowIds); + const workflows = await this.workflowRepository.findByIds(workflowIds); // determine owner of each workflow to be exported const owners: Record = {}; - sharedWorkflows.forEach((e) => { - const project = e.project; + sharedWorkflows.forEach((sharedWorkflow) => { + const project = sharedWorkflow.project; if (!project) { - throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`); + throw new ApplicationError(`Workflow ${sharedWorkflow.workflow.display()} has no owner`); } if (project.type === 'personal') { @@ -124,14 +127,16 @@ export class SourceControlExportService { (pr) => pr.role === 'project:personalOwner', ); if (!ownerRelation) { - throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`); + throw new ApplicationError( + `Workflow ${sharedWorkflow.workflow.display()} has no owner`, + ); } - owners[e.workflowId] = { + owners[sharedWorkflow.workflowId] = { type: 'personal', personalEmail: ownerRelation.user.email, }; } else if (project.type === 'team') { - owners[e.workflowId] = { + owners[sharedWorkflow.workflowId] = { type: 'team', teamId: project.id, teamName: project.name, @@ -156,6 +161,7 @@ export class SourceControlExportService { })), }; } catch (error) { + if (error instanceof ApplicationError) throw error; throw new ApplicationError('Failed to export workflows to work folder', { cause: error }); } } @@ -204,7 +210,7 @@ export class SourceControlExportService { files: [], }; } - const mappings = await Container.get(WorkflowTagMappingRepository).find(); + const mappings = await this.workflowTagMappingRepository.find(); const fileName = path.join(this.gitFolder, SOURCE_CONTROL_TAGS_EXPORT_FILE); await fsWriteFile( fileName, @@ -260,9 +266,10 @@ export class SourceControlExportService { try { sourceControlFoldersExistCheck([this.credentialExportFolder]); const credentialIds = candidates.map((e) => e.id); - const credentialsToBeExported = await Container.get( - SharedCredentialsRepository, - ).findByCredentialIds(credentialIds, 'credential:owner'); + const credentialsToBeExported = await this.sharedCredentialsRepository.findByCredentialIds( + credentialIds, + 'credential:owner', + ); let missingIds: string[] = []; if (credentialsToBeExported.length !== credentialIds.length) { const foundCredentialIds = credentialsToBeExported.map((e) => e.credentialsId); diff --git a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts index 21de490215..21640dad0e 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts @@ -1,5 +1,5 @@ import type { SourceControlledFile } from '@n8n/api-types'; -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; import glob from 'fast-glob'; @@ -53,7 +53,15 @@ export class SourceControlImportService { private readonly errorReporter: ErrorReporter, private readonly variablesService: VariablesService, private readonly activeWorkflowManager: ActiveWorkflowManager, + private readonly credentialsRepository: CredentialsRepository, + private readonly projectRepository: ProjectRepository, private readonly tagRepository: TagRepository, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly sharedCredentialsRepository: SharedCredentialsRepository, + private readonly userRepository: UserRepository, + private readonly variablesRepository: VariablesRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowTagMappingRepository: WorkflowTagMappingRepository, instanceSettings: InstanceSettings, ) { this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -91,7 +99,7 @@ export class SourceControlImportService { } async getLocalVersionIdsFromDb(): Promise { - const localWorkflows = await Container.get(WorkflowRepository).find({ + const localWorkflows = await this.workflowRepository.find({ select: ['id', 'name', 'versionId', 'updatedAt'], }); return localWorkflows.map((local) => { @@ -146,7 +154,7 @@ export class SourceControlImportService { } async getLocalCredentialsFromDb(): Promise> { - const localCredentials = await Container.get(CredentialsRepository).find({ + const localCredentials = await this.credentialsRepository.find({ select: ['id', 'name', 'type'], }); return localCredentials.map((local) => ({ @@ -201,24 +209,22 @@ export class SourceControlImportService { const localTags = await this.tagRepository.find({ select: ['id', 'name'], }); - const localMappings = await Container.get(WorkflowTagMappingRepository).find({ + const localMappings = await this.workflowTagMappingRepository.find({ select: ['workflowId', 'tagId'], }); return { tags: localTags, mappings: localMappings }; } async importWorkflowFromWorkFolder(candidates: SourceControlledFile[], userId: string) { - const personalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId); const workflowManager = this.activeWorkflowManager; const candidateIds = candidates.map((c) => c.id); - const existingWorkflows = await Container.get(WorkflowRepository).findByIds(candidateIds, { + const existingWorkflows = await this.workflowRepository.findByIds(candidateIds, { fields: ['id', 'name', 'versionId', 'active'], }); - const allSharedWorkflows = await Container.get(SharedWorkflowRepository).findWithFields( - candidateIds, - { select: ['workflowId', 'role', 'projectId'] }, - ); + const allSharedWorkflows = await this.sharedWorkflowRepository.findWithFields(candidateIds, { + select: ['workflowId', 'role', 'projectId'], + }); const importWorkflowsResult = []; // Due to SQLite concurrency issues, we cannot save all workflows at once @@ -235,9 +241,7 @@ export class SourceControlImportService { const existingWorkflow = existingWorkflows.find((e) => e.id === importedWorkflow.id); importedWorkflow.active = existingWorkflow?.active ?? false; this.logger.debug(`Updating workflow id ${importedWorkflow.id ?? 'new'}`); - const upsertResult = await Container.get(WorkflowRepository).upsert({ ...importedWorkflow }, [ - 'id', - ]); + const upsertResult = await this.workflowRepository.upsert({ ...importedWorkflow }, ['id']); if (upsertResult?.identifiers?.length !== 1) { throw new ApplicationError('Failed to upsert workflow', { extra: { workflowId: importedWorkflow.id ?? 'new' }, @@ -253,7 +257,7 @@ export class SourceControlImportService { ? await this.findOrCreateOwnerProject(importedWorkflow.owner) : null; - await Container.get(SharedWorkflowRepository).upsert( + await this.sharedWorkflowRepository.upsert( { workflowId: importedWorkflow.id, projectId: remoteOwnerProject?.id ?? personalProject.id, @@ -276,7 +280,7 @@ export class SourceControlImportService { const error = ensureError(e); this.logger.error(`Failed to activate workflow ${existingWorkflow.id}`, { error }); } finally { - await Container.get(WorkflowRepository).update( + await this.workflowRepository.update( { id: existingWorkflow.id }, { versionId: importedWorkflow.versionId }, ); @@ -295,16 +299,15 @@ export class SourceControlImportService { } async importCredentialsFromWorkFolder(candidates: SourceControlledFile[], userId: string) { - const personalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId); const candidateIds = candidates.map((c) => c.id); - const existingCredentials = await Container.get(CredentialsRepository).find({ + const existingCredentials = await this.credentialsRepository.find({ where: { id: In(candidateIds), }, select: ['id', 'name', 'type', 'data'], }); - const existingSharedCredentials = await Container.get(SharedCredentialsRepository).find({ + const existingSharedCredentials = await this.sharedCredentialsRepository.find({ select: ['credentialsId', 'role'], where: { credentialsId: In(candidateIds), @@ -336,7 +339,7 @@ export class SourceControlImportService { } this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`); - await Container.get(CredentialsRepository).upsert(newCredentialObject, ['id']); + await this.credentialsRepository.upsert(newCredentialObject, ['id']); const isOwnedLocally = existingSharedCredentials.some( (c) => c.credentialsId === credential.id && c.role === 'credential:owner', @@ -352,7 +355,7 @@ export class SourceControlImportService { newSharedCredential.projectId = remoteOwnerProject?.id ?? personalProject.id; newSharedCredential.role = 'credential:owner'; - await Container.get(SharedCredentialsRepository).upsert({ ...newSharedCredential }, [ + await this.sharedCredentialsRepository.upsert({ ...newSharedCredential }, [ 'credentialsId', 'projectId', ]); @@ -388,7 +391,7 @@ export class SourceControlImportService { const existingWorkflowIds = new Set( ( - await Container.get(WorkflowRepository).find({ + await this.workflowRepository.find({ select: ['id'], }) ).map((e) => e.id), @@ -417,7 +420,7 @@ export class SourceControlImportService { await Promise.all( mappedTags.mappings.map(async (mapping) => { if (!existingWorkflowIds.has(String(mapping.workflowId))) return; - await Container.get(WorkflowTagMappingRepository).upsert( + await this.workflowTagMappingRepository.upsert( { tagId: String(mapping.tagId), workflowId: String(mapping.workflowId) }, { skipUpdateIfNoValuesChanged: true, @@ -464,12 +467,12 @@ export class SourceControlImportService { overriddenKeys.splice(overriddenKeys.indexOf(variable.key), 1); } try { - await Container.get(VariablesRepository).upsert({ ...variable }, ['id']); + await this.variablesRepository.upsert({ ...variable }, ['id']); } catch (errorUpsert) { if (isUniqueConstraintError(errorUpsert as Error)) { this.logger.debug(`Variable ${variable.key} already exists, updating instead`); try { - await Container.get(VariablesRepository).update({ key: variable.key }, { ...variable }); + await this.variablesRepository.update({ key: variable.key }, { ...variable }); } catch (errorUpdate) { this.logger.debug(`Failed to update variable ${variable.key}, skipping`); this.logger.debug((errorUpdate as Error).message); @@ -484,11 +487,11 @@ export class SourceControlImportService { if (overriddenKeys.length > 0 && valueOverrides) { for (const key of overriddenKeys) { result.imported.push(key); - const newVariable = Container.get(VariablesRepository).create({ + const newVariable = this.variablesRepository.create({ key, value: valueOverrides[key], }); - await Container.get(VariablesRepository).save(newVariable, { transaction: false }); + await this.variablesRepository.save(newVariable, { transaction: false }); } } @@ -498,32 +501,30 @@ export class SourceControlImportService { } private async findOrCreateOwnerProject(owner: ResourceOwner): Promise { - const projectRepository = Container.get(ProjectRepository); - const userRepository = Container.get(UserRepository); if (typeof owner === 'string' || owner.type === 'personal') { const email = typeof owner === 'string' ? owner : owner.personalEmail; - const user = await userRepository.findOne({ + const user = await this.userRepository.findOne({ where: { email }, }); if (!user) { return null; } - return await projectRepository.getPersonalProjectForUserOrFail(user.id); + return await this.projectRepository.getPersonalProjectForUserOrFail(user.id); } else if (owner.type === 'team') { - let teamProject = await projectRepository.findOne({ + let teamProject = await this.projectRepository.findOne({ where: { id: owner.teamId }, }); if (!teamProject) { try { - teamProject = await projectRepository.save( - projectRepository.create({ + teamProject = await this.projectRepository.save( + this.projectRepository.create({ id: owner.teamId, name: owner.teamName, type: 'team', }), ); } catch (e) { - teamProject = await projectRepository.findOne({ + teamProject = await this.projectRepository.findOne({ where: { id: owner.teamId }, }); if (!teamProject) { diff --git a/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts index e530d9d530..d424a844b2 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts @@ -1,4 +1,4 @@ -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import type { ValidationError } from 'class-validator'; import { validate } from 'class-validator'; import { rm as fsRm } from 'fs/promises'; @@ -7,7 +7,6 @@ import { ApplicationError, jsonParse } from 'n8n-workflow'; import { writeFile, chmod, readFile } from 'node:fs/promises'; import path from 'path'; -import config from '@/config'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import { @@ -17,6 +16,7 @@ import { SOURCE_CONTROL_PREFERENCES_DB_KEY, } from './constants'; import { generateSshKeyPair, isSourceControlLicensed } from './source-control-helper.ee'; +import { SourceControlConfig } from './source-control.config'; import type { KeyPairType } from './types/key-pair-type'; import { SourceControlPreferences } from './types/source-control-preferences'; @@ -34,6 +34,8 @@ export class SourceControlPreferencesService { private readonly instanceSettings: InstanceSettings, private readonly logger: Logger, private readonly cipher: Cipher, + private readonly settingsRepository: SettingsRepository, + private readonly sourceControlConfig: SourceControlConfig, ) { this.sshFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_SSH_FOLDER); this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -64,9 +66,7 @@ export class SourceControlPreferencesService { } private async getKeyPairFromDatabase() { - const dbSetting = await Container.get(SettingsRepository).findByKey( - 'features.sourceControl.sshKeys', - ); + const dbSetting = await this.settingsRepository.findByKey('features.sourceControl.sshKeys'); if (!dbSetting?.value) return null; @@ -120,7 +120,7 @@ export class SourceControlPreferencesService { async deleteKeyPair() { try { await fsRm(this.sshFolder, { recursive: true }); - await Container.get(SettingsRepository).delete({ key: 'features.sourceControl.sshKeys' }); + await this.settingsRepository.delete({ key: 'features.sourceControl.sshKeys' }); } catch (e) { const error = e instanceof Error ? e : new Error(`${e}`); this.logger.error(`Failed to delete SSH key pair: ${error.message}`); @@ -133,14 +133,12 @@ export class SourceControlPreferencesService { async generateAndSaveKeyPair(keyPairType?: KeyPairType): Promise { if (!keyPairType) { keyPairType = - this.getPreferences().keyGeneratorType ?? - (config.get('sourceControl.defaultKeyPairType') as KeyPairType) ?? - 'ed25519'; + this.getPreferences().keyGeneratorType ?? this.sourceControlConfig.defaultKeyPairType; } const keyPair = await generateSshKeyPair(keyPairType); try { - await Container.get(SettingsRepository).save({ + await this.settingsRepository.save({ key: 'features.sourceControl.sshKeys', value: JSON.stringify({ encryptedPrivateKey: this.cipher.encrypt(keyPair.privateKey), @@ -211,7 +209,7 @@ export class SourceControlPreferencesService { if (saveToDb) { const settingsValue = JSON.stringify(this._sourceControlPreferences); try { - await Container.get(SettingsRepository).save( + await this.settingsRepository.save( { key: SOURCE_CONTROL_PREFERENCES_DB_KEY, value: settingsValue, @@ -229,7 +227,7 @@ export class SourceControlPreferencesService { async loadFromDbAndApplySourceControlPreferences(): Promise< SourceControlPreferences | undefined > { - const loadedPreferences = await Container.get(SettingsRepository).findOne({ + const loadedPreferences = await this.settingsRepository.findOne({ where: { key: SOURCE_CONTROL_PREFERENCES_DB_KEY }, }); if (loadedPreferences) { diff --git a/packages/cli/src/environments.ee/source-control/source-control.config.ts b/packages/cli/src/environments.ee/source-control/source-control.config.ts new file mode 100644 index 0000000000..04d12dce4f --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/source-control.config.ts @@ -0,0 +1,8 @@ +import { Config, Env } from '@n8n/config'; + +@Config +export class SourceControlConfig { + /** Default SSH key type to use when generating SSH keys. */ + @Env('N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE') + defaultKeyPairType: 'ed25519' | 'rsa' = 'ed25519'; +} diff --git a/packages/cli/src/environments.ee/variables/environment-helpers.ts b/packages/cli/src/environments.ee/variables/environment-helpers.ts deleted file mode 100644 index dd9a17c95b..0000000000 --- a/packages/cli/src/environments.ee/variables/environment-helpers.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Container } from '@n8n/di'; - -import { License } from '@/license'; - -export function isVariablesEnabled(): boolean { - const license = Container.get(License); - return license.isVariablesEnabled(); -} - -export function canCreateNewVariable(variableCount: number): boolean { - if (!isVariablesEnabled()) { - return false; - } - const license = Container.get(License); - // This defaults to -1 which is what we want if we've enabled - // variables via the config - const limit = license.getVariablesLimit(); - if (limit === -1) { - return true; - } - return limit > variableCount; -} - -export function getVariablesLimit(): number { - const license = Container.get(License); - return license.getVariablesLimit(); -} diff --git a/packages/cli/src/environments.ee/variables/variables.service.ee.ts b/packages/cli/src/environments.ee/variables/variables.service.ee.ts index ebb134efd3..f59880fe60 100644 --- a/packages/cli/src/environments.ee/variables/variables.service.ee.ts +++ b/packages/cli/src/environments.ee/variables/variables.service.ee.ts @@ -1,4 +1,4 @@ -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import type { Variables } from '@/databases/entities/variables'; import { VariablesRepository } from '@/databases/repositories/variables.repository'; @@ -6,23 +6,21 @@ import { generateNanoId } from '@/databases/utils/generators'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; import { VariableValidationError } from '@/errors/variable-validation.error'; import { EventService } from '@/events/event.service'; +import { License } from '@/license'; import { CacheService } from '@/services/cache/cache.service'; -import { canCreateNewVariable } from './environment-helpers'; - @Service() export class VariablesService { constructor( - protected cacheService: CacheService, - protected variablesRepository: VariablesRepository, + private readonly cacheService: CacheService, + private readonly variablesRepository: VariablesRepository, private readonly eventService: EventService, + private readonly license: License, ) {} async getAllCached(state?: 'empty'): Promise { let variables = await this.cacheService.get('variables', { - async refreshFn() { - return await Container.get(VariablesService).findAll(); - }, + refreshFn: async () => await this.findAll(), }); if (variables === undefined) { @@ -77,7 +75,7 @@ export class VariablesService { } async create(variable: Omit): Promise { - if (!canCreateNewVariable(await this.getCount())) { + if (!this.canCreateNewVariable(await this.getCount())) { throw new VariableCountLimitReachedError('Variables limit reached'); } this.validateVariable(variable); @@ -100,4 +98,17 @@ export class VariablesService { await this.updateCache(); return (await this.getCached(id))!; } + + private canCreateNewVariable(variableCount: number): boolean { + if (!this.license.isVariablesEnabled()) { + return false; + } + // This defaults to -1 which is what we want if we've enabled + // variables via the config + const limit = this.license.getVariablesLimit(); + if (limit === -1) { + return true; + } + return limit > variableCount; + } } diff --git a/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts b/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts index cc3c3bc33b..5d8fe3fe10 100644 --- a/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts +++ b/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts @@ -19,7 +19,7 @@ import type { WorkflowRepository } from '@/databases/repositories/workflow.repos import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; import type { WorkflowRunner } from '@/workflow-runner'; -import { mockInstance } from '@test/mocking'; +import { mockInstance, mockLogger } from '@test/mocking'; import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import { TestRunnerService } from '../test-runner.service.ee'; @@ -129,6 +129,9 @@ function mockEvaluationExecutionData(metrics: Record) { }); } +const errorReporter = mock(); +const logger = mockLogger(); + describe('TestRunnerService', () => { const executionRepository = mock(); const workflowRepository = mock(); @@ -176,6 +179,7 @@ describe('TestRunnerService', () => { test('should create an instance of TestRunnerService', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -183,7 +187,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); expect(testRunnerService).toBeInstanceOf(TestRunnerService); @@ -191,6 +195,7 @@ describe('TestRunnerService', () => { test('should create and run test cases from past executions', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -198,7 +203,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -229,6 +234,7 @@ describe('TestRunnerService', () => { test('should run both workflow under test and evaluation workflow', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -236,7 +242,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -330,6 +336,7 @@ describe('TestRunnerService', () => { test('should properly count passed and failed executions', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -337,7 +344,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -388,6 +395,7 @@ describe('TestRunnerService', () => { test('should properly count failed test executions', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -395,7 +403,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -442,6 +450,7 @@ describe('TestRunnerService', () => { test('should properly count failed evaluations', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -449,7 +458,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -500,6 +509,7 @@ describe('TestRunnerService', () => { test('should specify correct start nodes when running workflow under test', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -507,7 +517,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -574,6 +584,7 @@ describe('TestRunnerService', () => { test('should properly choose trigger and start nodes', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -581,7 +592,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); const startNodesData = (testRunnerService as any).getStartNodesData( @@ -599,6 +610,7 @@ describe('TestRunnerService', () => { test('should properly choose trigger and start nodes 2', async () => { const testRunnerService = new TestRunnerService( + logger, workflowRepository, workflowRunner, executionRepository, @@ -606,7 +618,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, - mock(), + errorReporter, ); const startNodesData = (testRunnerService as any).getStartNodesData( diff --git a/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts b/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts index 5a054a3527..732c803814 100644 --- a/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts +++ b/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts @@ -1,6 +1,6 @@ import { Service } from '@n8n/di'; import { parse } from 'flatted'; -import { ErrorReporter } from 'n8n-core'; +import { ErrorReporter, Logger } from 'n8n-core'; import { NodeConnectionType, Workflow } from 'n8n-workflow'; import type { IDataObject, @@ -39,6 +39,7 @@ import { createPinData, getPastExecutionTriggerNode } from './utils.ee'; @Service() export class TestRunnerService { constructor( + private readonly logger: Logger, private readonly workflowRepository: WorkflowRepository, private readonly workflowRunner: WorkflowRunner, private readonly executionRepository: ExecutionRepository, @@ -115,8 +116,9 @@ export class TestRunnerService { executionMode: 'evaluation', runData: {}, pinData, - workflowData: workflow, + workflowData: { ...workflow, pinData }, userId, + partialExecutionVersion: '1', }; // Trigger the workflow under test with mocked data @@ -203,6 +205,8 @@ export class TestRunnerService { * Creates a new test run for the given test definition. */ async runTest(user: User, test: TestDefinition): Promise { + this.logger.debug('Starting new test run', { testId: test.id }); + const workflow = await this.workflowRepository.findById(test.workflowId); assert(workflow, 'Workflow not found'); @@ -227,6 +231,8 @@ export class TestRunnerService { .andWhere('execution.workflowId = :workflowId', { workflowId: test.workflowId }) .getMany(); + this.logger.debug('Found past executions', { count: pastExecutions.length }); + // Get the metrics to collect from the evaluation workflow const testMetricNames = await this.getTestMetricNames(test.id); @@ -238,6 +244,8 @@ export class TestRunnerService { const metrics = new EvaluationMetrics(testMetricNames); for (const { id: pastExecutionId } of pastExecutions) { + this.logger.debug('Running test case', { pastExecutionId }); + try { // Fetch past execution with data const pastExecution = await this.executionRepository.findOne({ @@ -257,6 +265,8 @@ export class TestRunnerService { user.id, ); + this.logger.debug('Test case execution finished', { pastExecutionId }); + // In case of a permission check issue, the test case execution will be undefined. // Skip them, increment the failed count and continue with the next test case if (!testCaseExecution) { @@ -279,6 +289,8 @@ export class TestRunnerService { ); assert(evalExecution); + this.logger.debug('Evaluation execution finished', { pastExecutionId }); + metrics.addResults(this.extractEvaluationResult(evalExecution)); if (evalExecution.data.resultData.error) { @@ -297,5 +309,7 @@ export class TestRunnerService { const aggregatedMetrics = metrics.getAggregatedMetrics(); await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics); + + this.logger.debug('Test run finished', { testId: test.id }); } } diff --git a/packages/cli/src/manual-execution.service.ts b/packages/cli/src/manual-execution.service.ts index fd6c27215f..0b8b9360d3 100644 --- a/packages/cli/src/manual-execution.service.ts +++ b/packages/cli/src/manual-execution.service.ts @@ -71,7 +71,11 @@ export class ManualExecutionService { }, }; - const workflowExecute = new WorkflowExecute(additionalData, 'manual', executionData); + const workflowExecute = new WorkflowExecute( + additionalData, + data.executionMode, + executionData, + ); return workflowExecute.processRunExecutionData(workflow); } else if ( data.runData === undefined || diff --git a/packages/cli/src/scaling/scaling.service.ts b/packages/cli/src/scaling/scaling.service.ts index 536e835c72..f20d0764c6 100644 --- a/packages/cli/src/scaling/scaling.service.ts +++ b/packages/cli/src/scaling/scaling.service.ts @@ -1,6 +1,6 @@ import { GlobalConfig } from '@n8n/config'; import { Container, Service } from '@n8n/di'; -import { ErrorReporter, InstanceSettings, Logger } from 'n8n-core'; +import { ErrorReporter, InstanceSettings, isObjectLiteral, Logger } from 'n8n-core'; import { ApplicationError, BINARY_ENCODING, @@ -93,6 +93,12 @@ export class ScalingService { void this.queue.process(JOB_TYPE_NAME, concurrency, async (job: Job) => { try { + if (!this.hasValidJobData(job)) { + throw new ApplicationError('Worker received invalid job', { + extra: { jobData: jsonStringify(job, { replaceCircularRefs: true }) }, + }); + } + await this.jobProcessor.processJob(job); } catch (error) { await this.reportJobProcessingError(ensureError(error), job); @@ -503,5 +509,9 @@ export class ScalingService { : jsonStringify(error, { replaceCircularRefs: true }); } + private hasValidJobData(job: Job) { + return isObjectLiteral(job.data) && 'executionId' in job.data && 'loadStaticData' in job.data; + } + // #endregion } diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 33da795f27..954c3e9fc3 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -12,7 +12,6 @@ import config from '@/config'; import { inE2ETests, LICENSE_FEATURES, N8N_VERSION } from '@/constants'; import { CredentialTypes } from '@/credential-types'; import { CredentialsOverwrites } from '@/credentials-overwrites'; -import { getVariablesLimit } from '@/environments.ee/variables/environment-helpers'; import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; @@ -326,7 +325,7 @@ export class FrontendService { } if (this.license.isVariablesEnabled()) { - this.settings.variables.limit = getVariablesLimit(); + this.settings.variables.limit = this.license.getVariablesLimit(); } if (this.license.isWorkflowHistoryLicensed() && config.getEnv('workflowHistory.enabled')) { diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 1711d18056..dd6a775606 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -454,7 +454,7 @@ export async function executeWebhook( } let pinData: IPinData | undefined; - const usePinData = executionMode === 'manual'; + const usePinData = ['manual', 'evaluation'].includes(executionMode); if (usePinData) { pinData = workflowData.pinData; runExecutionData.resultData.pinData = pinData; diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index a5ffb728d6..598e6a8b58 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -234,7 +234,7 @@ export class WorkflowRunner { } let pinData: IPinData | undefined; - if (data.executionMode === 'manual') { + if (['manual', 'evaluation'].includes(data.executionMode)) { pinData = data.pinData ?? data.workflowData.pinData; } diff --git a/packages/cli/test/integration/environments/source-control-import.service.test.ts b/packages/cli/test/integration/environments/source-control-import.service.test.ts index 17f8654aae..8b774f8fa9 100644 --- a/packages/cli/test/integration/environments/source-control-import.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-import.service.test.ts @@ -10,6 +10,7 @@ import fsp from 'node:fs/promises'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; +import { UserRepository } from '@/databases/repositories/user.repository'; import { SourceControlImportService } from '@/environments.ee/source-control/source-control-import.service.ee'; import type { ExportableCredential } from '@/environments.ee/source-control/types/exportable-credential'; @@ -21,20 +22,36 @@ import { randomCredentialPayload } from '../shared/random'; import * as testDb from '../shared/test-db'; describe('SourceControlImportService', () => { + let credentialsRepository: CredentialsRepository; + let projectRepository: ProjectRepository; + let sharedCredentialsRepository: SharedCredentialsRepository; + let userRepository: UserRepository; let service: SourceControlImportService; const cipher = mockInstance(Cipher); beforeAll(async () => { + await testDb.init(); + + credentialsRepository = Container.get(CredentialsRepository); + projectRepository = Container.get(ProjectRepository); + sharedCredentialsRepository = Container.get(SharedCredentialsRepository); + userRepository = Container.get(UserRepository); service = new SourceControlImportService( mock(), mock(), mock(), mock(), + credentialsRepository, + projectRepository, + mock(), + mock(), + sharedCredentialsRepository, + userRepository, + mock(), + mock(), mock(), mock({ n8nFolder: '/some-path' }), ); - - await testDb.init(); }); afterEach(async () => { @@ -75,7 +92,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(member); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -112,7 +129,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -149,7 +166,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -190,7 +207,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -223,7 +240,7 @@ describe('SourceControlImportService', () => { cipher.encrypt.mockReturnValue('some-encrypted-data'); { - const project = await Container.get(ProjectRepository).findOne({ + const project = await projectRepository.findOne({ where: [ { id: '1234-asdf', @@ -241,7 +258,7 @@ describe('SourceControlImportService', () => { importingUser.id, ); - const sharing = await Container.get(SharedCredentialsRepository).findOne({ + const sharing = await sharedCredentialsRepository.findOne({ where: { credentialsId: CREDENTIAL_ID, role: 'credential:owner', @@ -288,7 +305,7 @@ describe('SourceControlImportService', () => { importingUser.id, ); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: project.id, role: 'credential:owner', @@ -332,7 +349,7 @@ describe('SourceControlImportService', () => { ); await expect( - Container.get(SharedCredentialsRepository).findBy({ + sharedCredentialsRepository.findBy({ credentialsId: credential.id, }), ).resolves.toMatchObject([ @@ -342,7 +359,7 @@ describe('SourceControlImportService', () => { }, ]); await expect( - Container.get(CredentialsRepository).findBy({ + credentialsRepository.findBy({ id: credential.id, }), ).resolves.toMatchObject([ diff --git a/packages/cli/test/integration/environments/source-control.test.ts b/packages/cli/test/integration/environments/source-control.api.test.ts similarity index 76% rename from packages/cli/test/integration/environments/source-control.test.ts rename to packages/cli/test/integration/environments/source-control.api.test.ts index 2dedd53287..6ad771493b 100644 --- a/packages/cli/test/integration/environments/source-control.test.ts +++ b/packages/cli/test/integration/environments/source-control.api.test.ts @@ -1,7 +1,6 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; -import config from '@/config'; import type { User } from '@/databases/entities/user'; import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee'; @@ -21,11 +20,17 @@ const testServer = utils.setupTestServer({ enabledFeatures: ['feat:sourceControl', 'feat:sharing'], }); +let sourceControlPreferencesService: SourceControlPreferencesService; + beforeAll(async () => { owner = await createUser({ role: 'global:owner' }); authOwnerAgent = testServer.authAgentFor(owner); - Container.get(SourceControlPreferencesService).isSourceControlConnected = () => true; + sourceControlPreferencesService = Container.get(SourceControlPreferencesService); + await sourceControlPreferencesService.setPreferences({ + connected: true, + keyGeneratorType: 'rsa', + }); }); describe('GET /sourceControl/preferences', () => { @@ -65,19 +70,11 @@ describe('GET /sourceControl/preferences', () => { }); test('refreshing key pairsshould return new rsa key', async () => { - config.set('sourceControl.defaultKeyPairType', 'rsa'); - await authOwnerAgent - .post('/source-control/generate-key-pair') - .send() - .expect(200) - .expect((res) => { - expect( - Container.get(SourceControlPreferencesService).getPreferences().keyGeneratorType, - ).toBe('rsa'); - expect(res.body.data).toHaveProperty('publicKey'); - expect(res.body.data).toHaveProperty('keyGeneratorType'); - expect(res.body.data.keyGeneratorType).toBe('rsa'); - expect(res.body.data.publicKey).toContain('ssh-rsa'); - }); + const res = await authOwnerAgent.post('/source-control/generate-key-pair').send().expect(200); + + expect(res.body.data).toHaveProperty('publicKey'); + expect(res.body.data).toHaveProperty('keyGeneratorType'); + expect(res.body.data.keyGeneratorType).toBe('rsa'); + expect(res.body.data.publicKey).toContain('ssh-rsa'); }); }); diff --git a/packages/core/src/DirectoryLoader.ts b/packages/core/src/DirectoryLoader.ts index 26dace0fd1..559c0c5531 100644 --- a/packages/core/src/DirectoryLoader.ts +++ b/packages/core/src/DirectoryLoader.ts @@ -80,8 +80,8 @@ export abstract class DirectoryLoader { constructor( readonly directory: string, - protected readonly excludeNodes: string[] = [], - protected readonly includeNodes: string[] = [], + protected excludeNodes: string[] = [], + protected includeNodes: string[] = [], ) {} abstract packageName: string; @@ -121,13 +121,12 @@ export abstract class DirectoryLoader { this.addCodex(tempNode, filePath); const nodeType = tempNode.description.name; - const fullNodeType = `${this.packageName}.${nodeType}`; - if (this.includeNodes.length && !this.includeNodes.includes(fullNodeType)) { + if (this.includeNodes.length && !this.includeNodes.includes(nodeType)) { return; } - if (this.excludeNodes.includes(fullNodeType)) { + if (this.excludeNodes.includes(nodeType)) { return; } @@ -151,7 +150,7 @@ export abstract class DirectoryLoader { if (currentVersionNode.hasOwnProperty('executeSingle')) { throw new ApplicationError( '"executeSingle" has been removed. Please update the code of this node to use "execute" instead.', - { extra: { nodeType: fullNodeType } }, + { extra: { nodeType } }, ); } } else { @@ -430,9 +429,25 @@ export class CustomDirectoryLoader extends DirectoryLoader { * e.g. /nodes-base or community packages. */ export class PackageDirectoryLoader extends DirectoryLoader { - packageJson: n8n.PackageJson = this.readJSONSync('package.json'); + packageJson: n8n.PackageJson; - packageName = this.packageJson.name; + packageName: string; + + constructor(directory: string, excludeNodes: string[] = [], includeNodes: string[] = []) { + super(directory, excludeNodes, includeNodes); + + this.packageJson = this.readJSONSync('package.json'); + this.packageName = this.packageJson.name; + this.excludeNodes = this.extractNodeTypes(excludeNodes); + this.includeNodes = this.extractNodeTypes(includeNodes); + } + + private extractNodeTypes(fullNodeTypes: string[]) { + return fullNodeTypes + .map((fullNodeType) => fullNodeType.split('.')) + .filter(([packageName]) => packageName === this.packageName) + .map(([_, nodeType]) => nodeType); + } override async loadAll() { const { n8n } = this.packageJson; @@ -524,9 +539,8 @@ export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { if (this.includeNodes.length) { const allowedNodes: typeof this.known.nodes = {}; - for (const fullNodeType of this.includeNodes) { - const [packageName, nodeType] = fullNodeType.split('.'); - if (packageName === this.packageName && nodeType in this.known.nodes) { + for (const nodeType of this.includeNodes) { + if (nodeType in this.known.nodes) { allowedNodes[nodeType] = this.known.nodes[nodeType]; } } @@ -538,11 +552,8 @@ export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { } if (this.excludeNodes.length) { - for (const fullNodeType of this.excludeNodes) { - const [packageName, nodeType] = fullNodeType.split('.'); - if (packageName === this.packageName) { - delete this.known.nodes[nodeType]; - } + for (const nodeType of this.excludeNodes) { + delete this.known.nodes[nodeType]; } this.types.nodes = this.types.nodes.filter( diff --git a/packages/core/src/error-reporter.ts b/packages/core/src/error-reporter.ts index 84f67a0395..5444b0d410 100644 --- a/packages/core/src/error-reporter.ts +++ b/packages/core/src/error-reporter.ts @@ -15,6 +15,11 @@ type ErrorReporterInitOptions = { release: string; environment: string; serverName: string; + /** + * Function to allow filtering out errors before they are sent to Sentry. + * Return true if the error should be filtered out. + */ + beforeSendFilter?: (event: ErrorEvent, hint: EventHint) => boolean; }; @Service() @@ -24,6 +29,8 @@ export class ErrorReporter { private report: (error: Error | string, options?: ReportingOptions) => void; + private beforeSendFilter?: (event: ErrorEvent, hint: EventHint) => boolean; + constructor(private readonly logger: Logger) { // eslint-disable-next-line @typescript-eslint/unbound-method this.report = this.defaultReport; @@ -52,7 +59,14 @@ export class ErrorReporter { await close(timeoutInMs); } - async init({ dsn, serverType, release, environment, serverName }: ErrorReporterInitOptions) { + async init({ + beforeSendFilter, + dsn, + serverType, + release, + environment, + serverName, + }: ErrorReporterInitOptions) { process.on('uncaughtException', (error) => { this.error(error); }); @@ -100,31 +114,34 @@ export class ErrorReporter { setTag('server_type', serverType); this.report = (error, options) => captureException(error, options); + this.beforeSendFilter = beforeSendFilter; } - async beforeSend(event: ErrorEvent, { originalException }: EventHint) { + async beforeSend(event: ErrorEvent, hint: EventHint) { + let { originalException } = hint; + if (!originalException) return null; if (originalException instanceof Promise) { originalException = await originalException.catch((error) => error as Error); } - if (originalException instanceof AxiosError) return null; - if ( - originalException instanceof Error && - originalException.name === 'QueryFailedError' && - ['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => originalException.message.includes(errMsg)) + this.beforeSendFilter?.(event, { + ...hint, + originalException, + }) ) { return null; } - if (originalException instanceof ApplicationError) { - const { level, extra, tags } = originalException; - if (level === 'warning') return null; - event.level = level; - if (extra) event.extra = { ...event.extra, ...extra }; - if (tags) event.tags = { ...event.tags, ...tags }; + if (originalException instanceof AxiosError) return null; + + if (this.isIgnoredSqliteError(originalException)) return null; + if (this.isApplicationError(originalException)) { + if (this.isIgnoredApplicationError(originalException)) return null; + + this.extractEventDetailsFromApplicationError(event, originalException); } if ( @@ -166,4 +183,31 @@ export class ErrorReporter { if (typeof e === 'string') return new ApplicationError(e); return; } + + /** @returns true if the error should be filtered out */ + private isIgnoredSqliteError(error: unknown) { + return ( + error instanceof Error && + error.name === 'QueryFailedError' && + ['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => error.message.includes(errMsg)) + ); + } + + private isApplicationError(error: unknown): error is ApplicationError { + return error instanceof ApplicationError; + } + + private isIgnoredApplicationError(error: ApplicationError) { + return error.level === 'warning'; + } + + private extractEventDetailsFromApplicationError( + event: ErrorEvent, + originalException: ApplicationError, + ) { + const { level, extra, tags } = originalException; + event.level = level; + if (extra) event.extra = { ...event.extra, ...extra }; + if (tags) event.tags = { ...event.tags, ...tags }; + } } diff --git a/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts b/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts index e09299a457..d4b96fa41d 100644 --- a/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts +++ b/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts @@ -246,4 +246,67 @@ describe('validateValueAgainstSchema', () => { // value should be type number expect(typeof result).toEqual('number'); }); + + describe('when the mode is in Fixed mode, and the node is a resource mapper', () => { + const nodeType = { + description: { + properties: [ + { + name: 'operation', + type: 'resourceMapper', + typeOptions: { + resourceMapper: { + mode: 'add', + }, + }, + }, + ], + }, + } as unknown as INodeType; + + const node = { + parameters: { + operation: { + schema: [ + { id: 'num', type: 'number', required: true }, + { id: 'str', type: 'string', required: true }, + { id: 'obj', type: 'object', required: true }, + { id: 'arr', type: 'array', required: true }, + ], + attemptToConvertTypes: true, + mappingMode: '', + value: '', + }, + }, + } as unknown as INode; + + const parameterName = 'operation.value'; + + describe('should correctly validate values for', () => { + test.each([ + { num: 0 }, + { num: 23 }, + { num: -0 }, + { num: -Infinity }, + { num: Infinity }, + { str: '' }, + { str: ' ' }, + { str: 'hello' }, + { arr: [] }, + { obj: {} }, + ])('%s', (value) => { + expect(() => + validateValueAgainstSchema(node, nodeType, value, parameterName, 0, 0), + ).not.toThrow(); + }); + }); + + describe('should throw an error for', () => { + test.each([{ num: NaN }, { num: undefined }, { num: null }])('%s', (value) => { + expect(() => + validateValueAgainstSchema(node, nodeType, value, parameterName, 0, 0), + ).toThrow(); + }); + }); + }); }); diff --git a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts index d058c50a52..a4c1b211e7 100644 --- a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts +++ b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts @@ -44,7 +44,7 @@ const validateResourceMapperValue = ( !skipRequiredCheck && schemaEntry?.required === true && schemaEntry.type !== 'boolean' && - !resolvedValue + (resolvedValue === undefined || resolvedValue === null) ) { return { valid: false, diff --git a/packages/core/test/DirectoryLoader.test.ts b/packages/core/test/DirectoryLoader.test.ts index 01a8c8d34a..226b5a6fee 100644 --- a/packages/core/test/DirectoryLoader.test.ts +++ b/packages/core/test/DirectoryLoader.test.ts @@ -235,10 +235,7 @@ describe('DirectoryLoader', () => { return JSON.stringify({}); } if (path.endsWith('types/nodes.json')) { - return JSON.stringify([ - { name: 'n8n-nodes-testing.node1' }, - { name: 'n8n-nodes-testing.node2' }, - ]); + return JSON.stringify([{ name: 'node1' }, { name: 'node2' }]); } if (path.endsWith('types/credentials.json')) { return JSON.stringify([]); @@ -254,7 +251,7 @@ describe('DirectoryLoader', () => { node1: { className: 'Node1', sourcePath: 'dist/Node1/Node1.node.js' }, }); expect(loader.types.nodes).toHaveLength(1); - expect(loader.types.nodes[0].name).toBe('n8n-nodes-testing.node1'); + expect(loader.types.nodes[0].name).toBe('node1'); expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); }); @@ -274,10 +271,7 @@ describe('DirectoryLoader', () => { return JSON.stringify({}); } if (path.endsWith('types/nodes.json')) { - return JSON.stringify([ - { name: 'n8n-nodes-testing.node1' }, - { name: 'n8n-nodes-testing.node2' }, - ]); + return JSON.stringify([{ name: 'node1' }, { name: 'node2' }]); } if (path.endsWith('types/credentials.json')) { return JSON.stringify([]); @@ -314,10 +308,7 @@ describe('DirectoryLoader', () => { return JSON.stringify({}); } if (path.endsWith('types/nodes.json')) { - return JSON.stringify([ - { name: 'n8n-nodes-testing.node1' }, - { name: 'n8n-nodes-testing.node2' }, - ]); + return JSON.stringify([{ name: 'node1' }, { name: 'node2' }]); } if (path.endsWith('types/credentials.json')) { return JSON.stringify([]); @@ -333,7 +324,7 @@ describe('DirectoryLoader', () => { node2: { className: 'Node2', sourcePath: 'dist/Node2/Node2.node.js' }, }); expect(loader.types.nodes).toHaveLength(1); - expect(loader.types.nodes[0].name).toBe('n8n-nodes-testing.node2'); + expect(loader.types.nodes[0].name).toBe('node2'); expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); }); }); @@ -654,18 +645,6 @@ describe('DirectoryLoader', () => { expect(nodeWithIcon.description.icon).toBeUndefined(); }); - it('should skip node if included in excludeNodes', () => { - const loader = new CustomDirectoryLoader(directory, ['CUSTOM.node1']); - const filePath = 'dist/Node1/Node1.node.js'; - - loader.loadNodeFromFile(filePath); - - expect(loader.nodeTypes).toEqual({}); - expect(loader.known.nodes).toEqual({}); - expect(loader.types.nodes).toEqual([]); - expect(loader.loadedNodes).toEqual([]); - }); - it('should skip node if not in includeNodes', () => { const loader = new CustomDirectoryLoader(directory, [], ['CUSTOM.other']); const filePath = 'dist/Node1/Node1.node.js'; diff --git a/packages/core/test/error-reporter.test.ts b/packages/core/test/error-reporter.test.ts index 9edc27f15c..e5b51d3356 100644 --- a/packages/core/test/error-reporter.test.ts +++ b/packages/core/test/error-reporter.test.ts @@ -101,6 +101,37 @@ describe('ErrorReporter', () => { const result = await errorReporter.beforeSend(event, { originalException }); expect(result).toBeNull(); }); + + describe('beforeSendFilter', () => { + const newErrorReportedWithBeforeSendFilter = (beforeSendFilter: jest.Mock) => { + const errorReporter = new ErrorReporter(mock()); + // @ts-expect-error - beforeSendFilter is private + errorReporter.beforeSendFilter = beforeSendFilter; + return errorReporter; + }; + + it('should filter out based on the beforeSendFilter', async () => { + const beforeSendFilter = jest.fn().mockReturnValue(true); + const errorReporter = newErrorReportedWithBeforeSendFilter(beforeSendFilter); + const hint = { originalException: new Error() }; + + const result = await errorReporter.beforeSend(event, hint); + + expect(result).toBeNull(); + expect(beforeSendFilter).toHaveBeenCalledWith(event, hint); + }); + + it('should not filter out when beforeSendFilter returns false', async () => { + const beforeSendFilter = jest.fn().mockReturnValue(false); + const errorReporter = newErrorReportedWithBeforeSendFilter(beforeSendFilter); + const hint = { originalException: new Error() }; + + const result = await errorReporter.beforeSend(event, hint); + + expect(result).toEqual(event); + expect(beforeSendFilter).toHaveBeenCalledWith(event, hint); + }); + }); }); describe('error', () => { diff --git a/packages/design-system/src/components/N8nSelect/Select.vue b/packages/design-system/src/components/N8nSelect/Select.vue index 80a065d429..28c1fc049a 100644 --- a/packages/design-system/src/components/N8nSelect/Select.vue +++ b/packages/design-system/src/components/N8nSelect/Select.vue @@ -136,6 +136,12 @@ defineExpose({ + + diff --git a/packages/design-system/src/css/_tokens.scss b/packages/design-system/src/css/_tokens.scss index 3b6b7451be..66ad82a4d7 100644 --- a/packages/design-system/src/css/_tokens.scss +++ b/packages/design-system/src/css/_tokens.scss @@ -37,6 +37,8 @@ // Danger --color-danger-shade-1: var(--prim-color-alt-c-shade-100); --color-danger: var(--prim-color-alt-c); + --color-danger-light: var(--prim-color-alt-c-tint-150); + --color-danger-light-2: var(--prim-color-alt-c-tint-250); --color-danger-tint-1: var(--prim-color-alt-c-tint-400); --color-danger-tint-2: var(--prim-color-alt-c-tint-450); diff --git a/packages/editor-ui/src/api/credentials.ts b/packages/editor-ui/src/api/credentials.ts index 6ddb37c16a..18bb7b3f3d 100644 --- a/packages/editor-ui/src/api/credentials.ts +++ b/packages/editor-ui/src/api/credentials.ts @@ -32,6 +32,7 @@ export async function getAllCredentials( ): Promise { return await makeRestApiRequest(context, 'GET', '/credentials', { ...(includeScopes ? { includeScopes } : {}), + includeData: true, ...(filter ? { filter } : {}), }); } diff --git a/packages/editor-ui/src/api/sourceControl.ts b/packages/editor-ui/src/api/sourceControl.ts index 3aa929b6af..4553856928 100644 --- a/packages/editor-ui/src/api/sourceControl.ts +++ b/packages/editor-ui/src/api/sourceControl.ts @@ -33,7 +33,7 @@ export const pushWorkfolder = async ( export const pullWorkfolder = async ( context: IRestApiContext, data: PullWorkFolderRequestDto, -): Promise => { +): Promise => { return await makeRestApiRequest(context, 'POST', `${sourceControlApiRoot}/pull-workfolder`, data); }; diff --git a/packages/editor-ui/src/components/CredentialCard.vue b/packages/editor-ui/src/components/CredentialCard.vue index db40f0d274..4d4dd131f6 100644 --- a/packages/editor-ui/src/components/CredentialCard.vue +++ b/packages/editor-ui/src/components/CredentialCard.vue @@ -29,6 +29,7 @@ const props = withDefaults( defineProps<{ data: ICredentialsResponse; readOnly?: boolean; + needsSetup?: boolean; }>(), { data: () => ({ @@ -146,6 +147,9 @@ function moveResource() { {{ locale.baseText('credentials.item.readonly') }} + + {{ locale.baseText('credentials.item.needsSetup') }} +
@@ -195,10 +199,6 @@ function moveResource() { .cardHeading { font-size: var(--font-size-s); padding: var(--spacing-s) 0 0; - - span { - color: var(--color-text-light); - } } .cardDescription { diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts index 4c192ffba6..dc3e805b05 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts @@ -3,7 +3,7 @@ import { waitFor } from '@testing-library/vue'; import userEvent from '@testing-library/user-event'; import { createTestingPinia } from '@pinia/testing'; import { merge } from 'lodash-es'; -import { SOURCE_CONTROL_PULL_MODAL_KEY, STORES } from '@/constants'; +import { SOURCE_CONTROL_PULL_MODAL_KEY, SOURCE_CONTROL_PUSH_MODAL_KEY, STORES } from '@/constants'; import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils'; import MainSidebarSourceControl from '@/components/MainSidebarSourceControl.vue'; import { useSourceControlStore } from '@/stores/sourceControl.store'; @@ -18,8 +18,9 @@ let rbacStore: ReturnType; const showMessage = vi.fn(); const showError = vi.fn(); +const showToast = vi.fn(); vi.mock('@/composables/useToast', () => ({ - useToast: () => ({ showMessage, showError }), + useToast: () => ({ showMessage, showError, showToast }), })); const renderComponent = createComponentRenderer(MainSidebarSourceControl); @@ -131,5 +132,129 @@ describe('MainSidebarSourceControl', () => { ), ); }); + + it('should open push modal when there are changes', async () => { + const status = [ + { + id: '014da93897f146d2b880-baa374b9d02d', + name: 'vuelfow2', + type: 'workflow' as const, + status: 'created' as const, + location: 'local' as const, + conflict: false, + file: '/014da93897f146d2b880-baa374b9d02d.json', + updatedAt: '2025-01-09T13:12:24.580Z', + }, + ]; + vi.spyOn(sourceControlStore, 'getAggregatedStatus').mockResolvedValueOnce(status); + const openModalSpy = vi.spyOn(uiStore, 'openModalWithData'); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[1]); + await waitFor(() => + expect(openModalSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: SOURCE_CONTROL_PUSH_MODAL_KEY, + data: expect.objectContaining({ + status, + }), + }), + ), + ); + }); + + it("should show user's feedback when pulling", async () => { + vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([ + { + id: '014da93897f146d2b880-baa374b9d02d', + name: 'vuelfow2', + type: 'workflow', + status: 'created', + location: 'remote', + conflict: false, + file: '/014da93897f146d2b880-baa374b9d02d.json', + updatedAt: '2025-01-09T13:12:24.580Z', + }, + { + id: 'a102c0b9-28ac-43cb-950e-195723a56d54', + name: 'Gmail account', + type: 'credential', + status: 'created', + location: 'remote', + conflict: false, + file: '/a102c0b9-28ac-43cb-950e-195723a56d54.json', + updatedAt: '2025-01-09T13:12:24.586Z', + }, + { + id: 'variables', + name: 'variables', + type: 'variables', + status: 'modified', + location: 'remote', + conflict: false, + file: '/variable_stubs.json', + updatedAt: '2025-01-09T13:12:24.588Z', + }, + { + id: 'mappings', + name: 'tags', + type: 'tags', + status: 'modified', + location: 'remote', + conflict: false, + file: '/tags.json', + updatedAt: '2024-12-16T12:53:12.155Z', + }, + ]); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[0]); + await waitFor(() => { + expect(showToast).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + title: 'Finish setting up your new variables to use in workflows', + }), + ); + expect(showToast).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + title: 'Finish setting up your new credentials to use in workflows', + }), + ); + expect(showToast).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + message: '1 Workflow, 1 Credential, Variables, and Tags were pulled', + }), + ); + }); + }); + + it('should show feedback where there are no change to pull', async () => { + vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([]); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[0]); + await waitFor(() => { + expect(showMessage).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Up to date', + }), + ); + }); + }); }); }); diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.vue b/packages/editor-ui/src/components/MainSidebarSourceControl.vue index 1fa8338a3f..af861e93c9 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.vue +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.vue @@ -1,5 +1,5 @@ - +
@@ -576,7 +601,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s :class="$style.edit" data-test-id="credential-edit-button" > - diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index aff304a3cf..ac13a67c95 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -4,7 +4,6 @@ import { NodeConnectionType, type IRunData, type IRunExecutionData, - type NodeError, type Workflow, } from 'n8n-workflow'; import RunData from './RunData.vue'; @@ -120,14 +119,17 @@ const hasAiMetadata = computed(() => { return false; }); +const hasError = computed(() => + Boolean( + workflowRunData.value && + node.value && + workflowRunData.value[node.value.name]?.[props.runIndex]?.error, + ), +); + // Determine the initial output mode to logs if the node has an error and the logs are available const defaultOutputMode = computed(() => { - const hasError = - workflowRunData.value && - node.value && - (workflowRunData.value[node.value.name]?.[props.runIndex]?.error as NodeError); - - return Boolean(hasError) && hasAiMetadata.value ? OUTPUT_TYPE.LOGS : OUTPUT_TYPE.REGULAR; + return hasError.value && hasAiMetadata.value ? OUTPUT_TYPE.LOGS : OUTPUT_TYPE.REGULAR; }); const isNodeRunning = computed(() => { @@ -216,7 +218,7 @@ const canPinData = computed(() => { }); const allToolsWereUnusedNotice = computed(() => { - if (!node.value || runsCount.value === 0) return undefined; + if (!node.value || runsCount.value === 0 || hasError.value) return undefined; // With pinned data there's no clear correct answer for whether // we should use historic or current parents, so we don't show the notice, diff --git a/packages/editor-ui/src/components/ParameterIssues.vue b/packages/editor-ui/src/components/ParameterIssues.vue index 34821c70a6..6c023727d4 100644 --- a/packages/editor-ui/src/components/ParameterIssues.vue +++ b/packages/editor-ui/src/components/ParameterIssues.vue @@ -1,6 +1,7 @@