fix(github action): preview service acceptance (#2891)

- should run on changes to files in directory
- Remove pnpm
- Allow postgres connection string to be configured for acceptance test
- Different postgres connection string if running inside preview container
- Run the preview-service image as a github action service
- separate the jobs into a build and a test job
- do not run the preview-service via the acceptance test, instead run it via github actions
- Add correct permission to job
- Add logging to the test, to understand progress
- Allow database name to be passed in to acceptance test
- Only delete the database if the test helper owns (created) it
- Upload image to s3 bucket

.github/workflows/preview-service-acceptance.yml

@@ -7,16 +7,65 @@ on:
       - .yarnrc.yml .
       - .yarn
       - package.json
-      - packages/frontend-2/type-augmentations/stubs/**
-      - packages/preview-service/**
-      - packages/viewer/**
-      - packages/objectloader/**
-      - packages/shared/**
+      - '.github/workflows/preview-service-acceptance.yml'
+      - 'packages/frontend-2/type-augmentations/stubs/**/*'
+      - 'packages/preview-service/**/*'
+      - 'packages/viewer/**/*'
+      - 'packages/objectloader/**/*'
+      - 'packages/shared/**/*'
+
+env:
+  REGISTRY: ghcr.io
+  IMAGE_NAME: ${{ github.repository_owner }}/speckle-preview-service
+  OUTPUT_FILE_PATH: 'preview-service-output/${{ github.sha }}.png'
 
 jobs:
+  build-preview-service:
+    name: Build Preview Service
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+      packages: write # publishing container to GitHub registry
+
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+      - name: Log in to the Container registry
+        uses: docker/[email protected]
+        with:
+          registry: ${{ env.REGISTRY }}
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+      - name: Extract metadata (tags, labels) for Docker
+        id: meta
+        uses: docker/[email protected]
+        with:
+          tags: type=sha,format=long
+          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
+      - name: Build and load preview-service Docker image
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          file: ./packages/preview-service/Dockerfile
+          push: true
+          tags: ${{ steps.meta.outputs.tags }}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+    outputs:
+      tags: ${{ steps.meta.outputs.tags }}
+
   preview-service-acceptance:
     name: Preview Service Acceptance test
     runs-on: ubuntu-latest
+    needs: build-preview-service
+
+    permissions:
+      contents: write # to update the screenshot saved in the branch. This is a HACK as GitHub API does not yet support uploading attachments to a comment.
+      pull-requests: write # to write a comment on the PR
+      packages: read # to download the preview-service image
+
     services:
       postgres:
         # Docker Hub image
@@ -33,61 +82,44 @@ jobs:
           --health-retries 5
         ports:
           - 5432:5432
-
-    permissions:
-      contents: write # to update the screenshot saved in the branch. This is a HACK as GitHub API does not yet support uploading attachments to a comment.
-      pull-requests: write # to write a comment on the PR
+      preview-service:
+        image: ${{ needs.build-preview-service.outputs.tags }}
+        env:
+          # note that the host is the postgres service name
+          PG_CONNECTION_STRING: postgres://preview_service_test:preview_service_test@postgres:5432/preview_service_test
 
     steps:
       - uses: actions/checkout@v4
-      - uses: pnpm/action-setup@v4
-        name: Install pnpm
-        with:
-          run_install: false
       - name: Install Node.js
         uses: actions/setup-node@v4
         with:
           node-version: 20
           cache: 'yarn'
       - name: Install dependencies
-        working-directory: utils/preview-service-acceptance
+        working-directory: packages/preview-service
         run: yarn install
 
-      #TODO load the docker image from a previous job
-      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v3
-      - name: Build and load preview-service Docker image
-        uses: docker/build-push-action@v6
-        with:
-          context: .
-          file: ./packages/preview-service/Dockerfile
-          load: true
-          push: false
-          tags: speckle/preview-service:local
-          cache-from: type=gha
-          cache-to: type=gha,mode=max
-
       - name: Run the acceptance test
         working-directory: packages/preview-service
         run: yarn test:acceptance
         env:
-          PREVIEW_SERVICE_IMAGE: speckle/preview-service:local
-          OUTPUT_FILE_PATH: /tmp/preview-service-output.png
           NODE_ENV: test
+          TEST_DB: preview_service_test
+          # note that the host is localhost, but the port is the port mapped to the postgres service
           PG_CONNECTION_STRING: postgres://preview_service_test:preview_service_test@localhost:5432/preview_service_test
+          OUTPUT_FILE_PATH: ${{ env.OUTPUT_FILE_PATH }}
+          S3_BUCKET: ${{ vars.S3_BUCKET }}
+          S3_ACCESS_KEY: ${{ secrets.S3_ACCESS_KEY }}
+          S3_SECRET_KEY: ${{ secrets.S3_SECRET_KEY }}
+          S3_ENDPOINT: ${{ vars.S3_ENDPOINT }}
+          S3_REGION: ${{ vars.S3_REGION }}
 
-      - uses: actions/upload-artifact@v4
-        name: Upload the output from the preview-service
-        id: upload-preview-service-output
-        with:
-          name: preview-service-output
-          path: /tmp/preview-service-output.png
       - uses: actions/github-script@v7
         with:
           script: |
             github.rest.issues.createComment({
               issue_number: context.issue.number,
               owner: context.repo.owner,
               repo: context.repo.repo,
-              body: '📸 Preview service has generated the following image:<br/><img src="${{ steps.upload-preview-service-output.outputs.artifact-url }}"/><br/>'
+              body: '📸 Preview service has generated <a href="${{ vars.S3_ENDPOINT }}/${{ vars.S3_BUCKET }}/${{ env.OUTPUT_FILE_PATH}}">an image.</a>'
             })

packages/preview-service/package.json

@@ -62,6 +62,7 @@
     "zod": "^3.23.8"
   },
   "devDependencies": {
+    "@aws-sdk/client-s3": "^3.645.0",
     "@babel/core": "^7.17.5",
     "@types/express": "^4.17.13",
     "@types/lodash-es": "^4.17.6",

packages/preview-service/tests/acceptance/acceptance.spec.ts

@@ -2,30 +2,36 @@ import { acceptanceTest } from '#/helpers/testExtensions.js'
 import { ObjectPreview, type ObjectPreviewRow } from '@/repositories/objectPreview.js'
 import { Previews } from '@/repositories/previews.js'
 import cryptoRandomString from 'crypto-random-string'
-import { afterEach, beforeEach, describe, expect, inject } from 'vitest'
+import { afterEach, beforeEach, describe, expect } from 'vitest'
 import { promises as fs } from 'fs'
-import { spawn } from 'child_process'
 import { OBJECTS_TABLE_NAME } from '#/migrations/migrations.js'
 import type { Angle } from '@/domain/domain.js'
 import { testLogger as logger } from '@/observability/logging.js'
 
+import { PutObjectCommand, PutObjectCommandInput, S3Client } from '@aws-sdk/client-s3'
+
+const getS3Config = () => {
+  return {
+    credentials: {
+      accessKeyId: process.env.S3_ACCESS_KEY || '',
+      secretAccessKey: process.env.S3_SECRET_KEY || ''
+    },
+    endpoint: process.env.S3_ENDPOINT || '',
+    forcePathStyle: true,
+    // s3ForcePathStyle: true,
+    // signatureVersion: 'v4',
+    region: process.env.S3_REGION || 'us-east-1'
+  }
+}
+
 describe.sequential('Acceptance', () => {
   describe.sequential('Run the preview-service image in docker', () => {
     beforeEach(() => {
-      const dbName = inject('dbName')
-      //purposefully running in the background without waiting
-      void runProcess('docker', [
-        'run',
-        '--env',
-        `PG_CONNECTION_STRING=postgres://preview_service_test:[email protected]:5432/${dbName}`,
-        '--rm',
-        '--name',
-        'preview-service',
-        'speckle/preview-service:local'
-      ])
+      // const dbName = inject('dbName')
+      logger.info('🤜 running acceptance test before-each')
     })
-    afterEach(async () => {
-      await runProcess('docker', ['stop', 'preview-service'])
+    afterEach(() => {
+      logger.info('🤛 running acceptance test after-each')
     })
 
     // we use integration test and not e2e test because we don't need the server
@@ -36,11 +42,6 @@ describe.sequential('Acceptance', () => {
       },
       async ({ context }) => {
         const { db } = context
-        const dbName = inject('dbName')
-        logger.info(
-          { databaseName: dbName },
-          'Running test in database: {databaseName}'
-        )
         // load data
         const streamId = cryptoRandomString({ length: 10 })
         const objectId = cryptoRandomString({ length: 10 })
@@ -76,6 +77,10 @@ describe.sequential('Acceptance', () => {
             .where('streamId', streamId)
             .andWhere('objectId', objectId)
 
+          logger.info(
+            { result: objectPreviewResult, streamId, objectId },
+            '🔍 Polled object preview for a result for {streamId} and {objectId}'
+          )
           // wait a second before polling again
           await new Promise((resolve) => setTimeout(resolve, 1000))
         }
@@ -84,34 +89,60 @@ describe.sequential('Acceptance', () => {
           .select(['data'])
           .where('id', objectPreviewResult[0].preview['all' as Angle])
           .first()
+        logger.info({ previewData }, '🔍 Retrieved preview data')
 
         if (!previewData) {
           expect(previewData).toBeDefined()
           expect(previewData).not.toBeNull()
           return //HACK to appease typescript
         }
 
-        //TODO use environment variable
-        const outputFilePath =
-          process.env.OUTPUT_FILE_PATH || '/tmp/preview-service-output.png'
-        await fs.writeFile(outputFilePath, previewData.data)
+        if (!process.env.OUTPUT_FILE_PATH)
+          throw new Error('OUTPUT_FILE_PATH environment variable not set')
+
+        const outputFilePath = process.env.OUTPUT_FILE_PATH
+
+        const s3Config = getS3Config()
+
+        if (s3Config.credentials.accessKeyId && s3Config.credentials.secretAccessKey) {
+          logger.info(
+            { outputFilePath },
+            'S3 credentials provided, saving to S3 at {outputFilePath}'
+          )
+          const s3Client = new S3Client(s3Config)
+
+          const params: PutObjectCommandInput = {
+            Bucket: 'github-action-speckle-preview-service-acceptance-test',
+            Key: outputFilePath,
+            Body: previewData.data,
+            ACL: 'public-read',
+            Metadata: {
+              // Defines metadata tags.
+              // 'x-amz-meta-my-key': 'your-value'
+            }
+          }
+
+          const uploadObject = async () => {
+            try {
+              const data = await s3Client.send(new PutObjectCommand(params))
+              logger.info(
+                'Successfully uploaded object: ' + params.Bucket + '/' + params.Key
+              )
+              return data
+            } catch (err) {
+              logger.error(err, 'Failed to upload object')
+            }
+          }
+
+          await uploadObject()
+        } else {
+          logger.info(
+            { outputFilePath },
+            'No S3 credentials provided, saving to local file system at {outputFilePath}'
+          )
+          await fs.writeFile(outputFilePath, previewData.data)
+        }
       }
     )
   })
 })
-
-function runProcess(cmd: string, cmdArgs: string[], extraEnv?: Record<string, string>) {
-  return new Promise((resolve, reject) => {
-    const childProc = spawn(cmd, cmdArgs, { env: { ...process.env, ...extraEnv } })
-    childProc.stdout.pipe(process.stdout)
-    childProc.stderr.pipe(process.stderr)
-
-    childProc.on('close', (code) => {
-      if (code === 0) {
-        resolve('success')
-      } else {
-        reject(`Parser exited with code ${code}`)
-      }
-    })
-  })
-}

packages/preview-service/tests/hooks/globalSetup.ts

@@ -15,10 +15,13 @@ declare module 'vitest' {
   }
 }
 
-const dbName = `preview_service_${cryptoRandomString({
-  length: 10,
-  type: 'alphanumeric'
-})}`.toLocaleLowerCase() //postgres will automatically lower case new db names
+const dbName =
+  process.env.TEST_DB || // in the acceptance tests we need to use a database name that is known prior to the test running
+  `preview_service_${cryptoRandomString({
+    length: 10,
+    type: 'alphanumeric'
+  })}`.toLocaleLowerCase() //postgres will automatically lower case new db names
+let isDatabaseCreatedExternally = true
 
 /**
  * Global setup hook
@@ -28,12 +31,18 @@ const dbName = `preview_service_${cryptoRandomString({
 export async function setup({ provide }: GlobalSetupContext) {
   logger.info('🏃🏻‍♀️‍➡️ Running vitest setup global hook')
   const superUserDbClient = getTestDb()
-  await superUserDbClient.raw(`CREATE DATABASE ${dbName}
+  const dbAlreadyExists = await superUserDbClient('pg_database')
+    .select('datname')
+    .where('datname', dbName)
+  if (!dbAlreadyExists.length) {
+    isDatabaseCreatedExternally = false
+    await superUserDbClient.raw(`CREATE DATABASE ${dbName}
     WITH
     OWNER = preview_service_test
     ENCODING = 'UTF8'
     TABLESPACE = pg_default
     CONNECTION LIMIT = -1;`)
+  }
   await superUserDbClient.destroy() // need to explicitly close the connection in clients to prevent hanging tests
 
   // this provides the dbName to all tests, and can be accessed via inject('dbName'). NB: The test extensions already implement this, so use a test extension.
@@ -56,9 +65,11 @@ export async function teardown() {
   await down(db) //we need the migration to occur in our named database, so cannot use knex's built in migration functionality.
   await db.destroy() // need to explicitly close the connection in clients to prevent hanging tests
 
-  //use connection without database to drop the db
-  const superUserDbClient = getTestDb()
-  await superUserDbClient.raw(`DROP DATABASE ${dbName};`)
-  await superUserDbClient.destroy() // need to explicitly close the connection in clients to prevent hanging tests
+  if (!isDatabaseCreatedExternally) {
+    //use connection without database to drop the db
+    const superUserDbClient = getTestDb()
+    await superUserDbClient.raw(`DROP DATABASE ${dbName};`)
+    await superUserDbClient.destroy() // need to explicitly close the connection in clients to prevent hanging tests
+  }
   logger.info('✅ Completed the vitest teardown global hook')
 }