interval / server Goto Github PK

Hi there!

We're running Interval Server on AWS ECS and we're currently getting some issues while trying to set up file uploads. The server requires us to pass S3_REGION, S3_KEY_ID, and S3_KEY_SECRET, but that should not be necessary when running it on AWS, as one can leverage using the task IAM role in order to access S3.

Some references:

• https://docs.aws.amazon.com/sdkref/latest/guide/feature-container-credentials.html
• https://docs.aws.amazon.com/sdkref/latest/guide/standardized-credentials.html#credentialProviderChain

Current workaround

Our current workaround for this is to patch the code and remove the part where it sets the credentials for S3:

After installing interval-server, we're sed-ing dist/src/server/utils/uploads.js:

sed -i -e '/credentials: {/{N;N;N;d;}' "/usr/local/lib/node_modules/@interval/server/dist/src/server/utils/uploads.js"

This simply removes setting the credentials, letting the SDK resolve the credentials dynamically.

Possible solution

Ideally, we should be able to specify only the S3_BUCKET environment variable, and let the other options (S3_REGION, S3_KEY_ID, and S3_KEY_SECRET) be optional. This way, if they're present in the environment, we can use them, otherwise, we let the SDK resolve the credentials dynamically.

Something like this should probably work, although I haven't tested with this code exactly:

diff --git a/src/server/utils/uploads.ts b/src/server/utils/uploads.ts
index e85e31c..0629555 100644
--- a/src/server/utils/uploads.ts
+++ b/src/server/utils/uploads.ts
@@ -1,5 +1,6 @@
 import {
   S3Client,
+  S3ClientConfig,
   PutObjectCommand,
   GetObjectCommand,
   DeleteObjectsCommand,
@@ -24,6 +25,24 @@ function isS3Available(env: any): env is {
   )
 }
 
+function getS3ClientConfig(env: any): S3ClientConfig {
+  const config: S3ClientConfig = {
+    region: env.S3_REGION,
+  }
+
+  if (
+    typeof env.S3_KEY_ID === 'string' &&
+    typeof env.S3_KEY_SECRET === 'string'
+  ) {
+    config.credentials = {
+      accessKeyId: env.S3_KEY_ID,
+      secretAccessKey: env.S3_KEY_SECRET,
+    }
+  }
+
+  return config
+}
+
 export const S3_UPLOADS_ENABLED = isS3Available(env)
 
 function getS3Client() {
@@ -33,13 +52,7 @@ function getS3Client() {
     )
   }
 
-  return new S3Client({
-    region: env.S3_REGION,
-    credentials: {
-      accessKeyId: env.S3_KEY_ID,
-      secretAccessKey: env.S3_KEY_SECRET,
-    },
-  })
+  return new S3Client(getS3ClientConfig(env))
 }
 
 export async function getIOPresignedUploadUrl(key: string): Promise<string> {

Right now, the db-init command uses the psql command line to run the initSql, along with running a prisma db push.
For environments where you want to deploy with docker, this means you separately have to:

• Locally download the server to run the command line tool
• (in the future) as Interval releases more updates, run either prisma db push or skip create to get the new updates.

I am suggesting that instead, it would make more sense to automatically run this on startup.

• The initSql is already safe to run multiple times as it's calling IF NOT EXISTS and CREATE OR REPLACE. (Making the use of IF NOT EXISTS..etc a requirement for future versions of the initSql is also a good idea for updates!)
• Runs prisma db push, because this can run migrations from the updated files - which will be inside the Docker image. This is safe to run even if there are no changes, at only a low cost of startup time.

And even better, drop the requirement of psql and use a NodeJs postgres driver to run the SQL.

This means that deploying interval to a docker environment (especially one where the Postgres instance isn't publicly exposed to the internet) would simply be running the image and providing a valid connection string. The image would take care then of updating the database and ensuring it's ready to go.

With the current setup, if you were to deploy the :latest image, and for some reason it was to re-create the container, and update the image and there were Prisma changes, you'd run into an error during runtime without any warning.

This would also mean that Interval would be super easy to deploy in "one click" on sites that will deploy a database along side it from a template, but also in Kubernetes - it's unlikely you want to publicly expose your database. Right now I'd have to mess around with having interval installed locally, port-forwarding my database to my local machine, running that, and then applying a deployment file

In order to create Organization API Keys the account email needs to be confirmed. As per the documentation, a Postmark API key is required in order to send the emails. However, after creating a Postmark account and setting the key to the POSTMARK_API_KEY env var, I get the below error.

While your account is pending approval, all recipient addresses must share the same domain as the 'From' address. The domain of the 'From' address is 'interval.com', but you are attempting to send email to the following domain(s): 'MYCOMPANY.com'

Obviously this is a limitation on Postmark, but even when if the account is approved, I don't think emails should come from an interval.com address. It would be better to pass this as another env variable. I'm happy to submit a PR but wanted to check if there is anything else that needs to be taken into account.

This seems to be the line in question setting the from address:

I'm having trouble setting up Interval locally. I'm having trouble collecting to the Websocket server. I've attached the logs from my application and the interval instance here.

Some HTTP reverse proxies (such as nginx) will strip headers that contain an underscore. This entirely breaks interval because of the use of __interval_organization_id.

Further reading / example: https://www.grouparoo.com/blog/dont-use-underscores-in-http-headers
https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/?highlight=underscore#missing-disappearing-http-headers

To ensure interval works out of the box everywhere, I suggest changing this header to interval-organization-id