Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate env vars with Zod #2362

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
+4,315 −3,812
Open

Validate env vars with Zod #2362

Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
ddd10d1
WIP: Enables strict null checks in SDK
infomiho Oct 28, 2024
90baa94
Update handleApiError type
infomiho Oct 28, 2024
b5c3c63
Throw invalid credentials error explictly
infomiho Oct 28, 2024
7afc4f0
Formatting
infomiho Oct 28, 2024
b4e2362
Update comment and type
infomiho Oct 28, 2024
465856c
Update email env vars
infomiho Oct 28, 2024
5c329a4
Update isHttpErrorWithExtraMessage
infomiho Oct 28, 2024
e04aac3
Update comment
infomiho Oct 28, 2024
1928b6f
Remove TODO
infomiho Oct 28, 2024
e231dda
Fixes jobs types
infomiho Oct 28, 2024
f431153
Update todoApp tests. Comment update.
infomiho Oct 28, 2024
c8b2120
Update e2e tests
infomiho Oct 28, 2024
08304fe
Fixes headless tests
infomiho Oct 28, 2024
a509762
Fixes CORS error
infomiho Oct 28, 2024
9d618ea
Zod env validation WIP
infomiho Oct 25, 2024
7333b25
Define env vars validation. Use validate env vars.
infomiho Oct 29, 2024
80baf9c
Update e2e tests
infomiho Oct 29, 2024
fb899a0
Clean up
infomiho Oct 29, 2024
7593ac2
Update SKIP_EMAIL_VERIFICATION_IN_DEV validation
infomiho Oct 29, 2024
42d121c
Update headless tests
infomiho Oct 29, 2024
f877543
Update headless tests
infomiho Oct 30, 2024
025d185
Simplify the server config
infomiho Oct 30, 2024
1da4542
Fixes keycloak env usage
infomiho Oct 30, 2024
67de093
Merge branch 'main' into miho-zod-env
infomiho Nov 26, 2024
ee7c6de
Cleanup
infomiho Nov 26, 2024
7b0fedd
Cleanup
infomiho Nov 26, 2024
598eeab
Cleanup
infomiho Nov 26, 2024
4732a27
Cleanup
infomiho Nov 26, 2024
fcdc0d6
Cleanup
infomiho Nov 26, 2024
68e99ba
Fixes headless tests
infomiho Nov 26, 2024
c740e4e
Fixes e2e tests
infomiho Nov 26, 2024
88d7ea9
Cleanup
infomiho Nov 29, 2024
107fa51
Update e2e tests
infomiho Nov 29, 2024
42e9c78
Update API comment
infomiho Nov 29, 2024
9dc30c1
Update waspc/data/Generator/templates/sdk/wasp/server/env.ts
infomiho Dec 13, 2024
4766a94
Merge branch 'main' into miho-zod-env
infomiho Dec 13, 2024
ba46aa8
PR comments
infomiho Dec 16, 2024
920cc90
Merge branch 'main' into miho-zod-env
infomiho Jan 2, 2025
835e17a
Cleanup
infomiho Jan 2, 2025
d275338
Use single prettier.config.js for all templates
infomiho Jan 2, 2025
73e5e11
Update required error messages. jwtTokenSchema conditionally added.
infomiho Jan 2, 2025
41e24be
e2e tests
infomiho Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions waspc/data/Generator/templates/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ WORKDIR /app
# Copying the top level 'node_modules' because it contains the Prisma packages
# necessary for migrating the database.
COPY --from=server-builder /app/node_modules ./node_modules
# Copying the SDK because 'validate-env.mjs' executes independent of the bundle
# Copying the SDK because the server bundle doesn't bundle the SDK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? Why wouldn't it bundle the SDK? It's a dependency like any other (and it's copied with the node modules anyway).

Also, there's the second part of the old comment (below).

I know I wrote the original comment but I am not sure what I meant. But, I think I remember specifically needing it for validate-env.mjs.

Copy link
Contributor Author

@infomiho infomiho Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rollup doesn't bundle the wasp/* deps because they are considered "external" deps. I tried making them "internal" (adjust the regex for external deps) and it still didn't want to bundle them. Maybe because they are from node_modules. So, yep, we still need to copy over the SDK in the build context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the explanation (or a link to this discussion in the comment)?

# and references the 'wasp' package.
COPY --from=server-builder /app/.wasp/out/sdk .wasp/out/sdk
# Copying 'server/node_modules' because 'validate-env.mjs' executes independent
# of the bundle and references the dotenv package.
# Copying 'server/node_modules' because we require dotenv package to
# load environment variables
# TODO: replace dotenv with native Node.js environment variable loading
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that, what is wrong with dotenv? Node.js now has native support for it that is equally good?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always nicer to use natively supported methods than a package to do the same thing. It's available since Node.js 20 and we should probably go for it when Node.js 20 becomes our minimum version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probabl make nodejs 20 our minimum version any moment hm. Ok, so it does do the same thing? If so, great.

COPY --from=server-builder /app/.wasp/build/server/node_modules .wasp/build/server/node_modules
COPY --from=server-builder /app/.wasp/build/server/bundle .wasp/build/server/bundle
COPY --from=server-builder /app/.wasp/build/server/package*.json .wasp/build/server/
COPY --from=server-builder /app/.wasp/build/server/scripts .wasp/build/server/scripts
infomiho marked this conversation as resolved.
Show resolved Hide resolved
COPY db/ .wasp/build/db/
EXPOSE ${PORT}
WORKDIR /app/.wasp/build/server
Expand Down
5 changes: 2 additions & 3 deletions waspc/data/Generator/templates/react-app/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
{=& depsChunk =},
{=& devDepsChunk =},
"scripts": {
"start": "npm run validate-env && vite",
infomiho marked this conversation as resolved.
Show resolved Hide resolved
"build": "npm run validate-env && tsc && vite build",
"validate-env": "node -r dotenv/config ./scripts/validate-env.mjs"
"start": "vite",
"build": "tsc && vite build"
},
"engineStrict": true,
"engines": {
Expand Down
4 changes: 0 additions & 4 deletions waspc/data/Generator/templates/react-app/scripts/validate-env.mjs
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions waspc/data/Generator/templates/sdk/wasp/client/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{={= =}=}}
import { stripTrailingSlash } from '../universal/url.js'
import { env } from './env.js'

const apiUrl = stripTrailingSlash(import.meta.env.REACT_APP_API_URL) || '{= defaultServerUrl =}';
const apiUrl = stripTrailingSlash(env.REACT_APP_API_URL)
infomiho marked this conversation as resolved.
Show resolved Hide resolved

// PUBLIC API
export type ClientConfig = {
Expand Down
15 changes: 15 additions & 0 deletions waspc/data/Generator/templates/sdk/wasp/client/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{{={= =}=}}
import * as z from 'zod'

import { ensureEnvSchema } from '../env/index.js'

const clientEnvSchema = z.object({
REACT_APP_API_URL: z
.string({
required_error: 'REACT_APP_API_URL is required',
})
.default('{= defaultServerUrl =}')
})

// PUBLIC API
export const env = ensureEnvSchema(import.meta.env, clientEnvSchema)
5 changes: 4 additions & 1 deletion waspc/data/Generator/templates/sdk/wasp/client/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@ export enum HttpMethod {
export type Route = { method: HttpMethod; path: string }

// PUBLIC API
export { config, ClientConfig } from './config'
export { config, ClientConfig } from './config.js'

// PUBLIC API
export { env } from './env.js'
sodic marked this conversation as resolved.
Show resolved Hide resolved
31 changes: 31 additions & 0 deletions waspc/data/Generator/templates/sdk/wasp/env/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import * as z from 'zod'

const redColor = '\x1b[31m'

export function ensureEnvSchema<Schema extends z.ZodTypeAny>(
data: unknown,
schema: Schema
): z.infer<Schema> {
try {
return schema.parse(data)
} catch (e) {
if (e instanceof z.ZodError) {
const errorOutput = [
'',
'╔═════════════════════════════╗',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool! I do remember in demo that it was still a bit hard for me to figure out that this is the most important output to look at + where it starts / ends.

Maybe it would be better if it hard clear start and end.
E.g. something like this:

====== Env vars validation failed ======
 - error 1
 - error 2
 - error 3
========================================

or

====== Env vars validation failed ======
 - error 1
 - error 2
 - error 3
==== / Env vars validation failed ======

or even

|====== Env vars validation failed ======
| - error 1
| - error 2
| - error 3
|========================================

General idea: we should probably have some kind of universal way for outputing Wasp errors from the TS/JS/node runtime, like we have it for Haskell runtime. E.g. we could have a function defined for it, called consoleWaspError or something like that, that would always format errors like this in a same way. And hopefully it would be similar as the errors we output from the Haskell runtime, so they can easily connect it -> simialr formatting / structure / headers / style / ... .

It might be worth creating this general logging function right now in this PR, it should be quite easy, and then we have it done and can use it? If not ok, we can do it in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, let me do another iteration of this output 👍

I'll create an issue for making this more universal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-12-16 at 10 32 37

Here's the updated version. Related issue: #2420

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this now -> nice! Why don't you use nice characters for the left border? ╔ and ║ and ╚?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it and I didn't like it actually!

The box looks too polished but it's missing the right border, so this look is kinda less polished and the missing right border doesn't bother me that much visually 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok yeah I thought that might be the case. Although this also looks a bit weird. But ok, it is good enough for now, I also don't have better ideas at the moment!

Copy link
Member

@Martinsos Martinsos Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some quick ideas though:

===== Env vars validation failed =====
> - some text
> - some more text
> - third problem
======================================

===== Env vars validation failed =====
| - some text
| - some more text
| - third problem
======================================

or even just

===== Env vars validation failed =====
 - some text
 - some more text
 - third problem
======================================

Yeah I think I like this one the best.

There will probalby never be more then a couple of these errors, so if you remove those empty lines and make it more compact, you get it all to look quite tight and connected, I think left border might not even be needed then.

Also notice how I put some more = before and after the title, so it doesn't start at the same point where bullets start and looks a bit more like a title I think.

Do as you like though, you can keep it as it is if you prefer that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop all the | at the beginning of lines. It's weird and adds no extra info.

Martin's last suggestion is my favorite.
Obviously, I won't argue over this, so you can choose whatever you want :)

'║ Env vars validation failed ║',
'╚═════════════════════════════╝',
'',
]
for (const error of e.errors) {
errorOutput.push(`- ${error.message}`)
}
errorOutput.push('')
errorOutput.push('═══════════════════════════════')
console.error(redColor, errorOutput.join('\n'))
throw new Error('Error parsing environment variables')
} else {
throw e
}
}
}
7 changes: 7 additions & 0 deletions waspc/data/Generator/templates/sdk/wasp/prettier.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Used for internal Wasp development only, not copied to generated app.
module.exports = {
sodic marked this conversation as resolved.
Show resolved Hide resolved
trailingComma: 'es5',
tabWidth: 2,
semi: false,
singleQuote: true,
}
15 changes: 0 additions & 15 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/env.ts
View file
Open in desktop

This file was deleted.

6 changes: 1 addition & 5 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
import { OAuth2Provider, OAuth2ProviderWithPKCE } from "arctic";

export function defineProvider<
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE,
Env extends Record<string, string>
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE
>({
id,
displayName,
env,
oAuthClient,
}: {
id: string;
displayName: string;
env: Env;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of OAuth env vars having their special validation and living in a special place, they are now used directly from the env object.

oAuthClient: OAuthClient;
}) {
return {
id,
displayName,
env,
oAuthClient,
};
}
18 changes: 6 additions & 12 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/providers/discord.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
{{={= =}=}}
import { Discord } from "arctic";
import { Discord } from 'arctic';

import { defineProvider } from "../provider.js";
import { ensureEnvVarsForProvider } from "../env.js";
import { getRedirectUriForCallback } from "../redirect.js";
import { defineProvider } from '../provider.js';
import { getRedirectUriForCallback } from '../redirect.js';
import { env } from '../../../env.js';

const id = "{= providerId =}";
const displayName = "{= displayName =}";

const env = ensureEnvVarsForProvider(
["DISCORD_CLIENT_ID", "DISCORD_CLIENT_SECRET"],
displayName
);
const id = '{= providerId =}';
const displayName = '{= displayName =}';

const oAuthClient = new Discord(
env.DISCORD_CLIENT_ID,
Expand All @@ -23,6 +18,5 @@ const oAuthClient = new Discord(
export const discord = defineProvider({
id,
displayName,
env,
oAuthClient,
});
16 changes: 5 additions & 11 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/providers/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
{{={= =}=}}
import { GitHub } from "arctic";
import { GitHub } from 'arctic';

import { ensureEnvVarsForProvider } from "../env.js";
import { defineProvider } from "../provider.js";
import { defineProvider } from '../provider.js';
import { env } from '../../../env.js';

const id = "{= providerId =}";
const displayName = "{= displayName =}";

const env = ensureEnvVarsForProvider(
["GITHUB_CLIENT_ID", "GITHUB_CLIENT_SECRET"],
displayName
);
const id = '{= providerId =}';
const displayName = '{= displayName =}';

const oAuthClient = new GitHub(
env.GITHUB_CLIENT_ID,
Expand All @@ -21,6 +16,5 @@ const oAuthClient = new GitHub(
export const github = defineProvider({
id,
displayName,
env,
oAuthClient,
});
18 changes: 6 additions & 12 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/providers/google.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
{{={= =}=}}
import { Google } from "arctic";
import { Google } from 'arctic';

import { ensureEnvVarsForProvider } from "../env.js";
import { getRedirectUriForCallback } from "../redirect.js";
import { defineProvider } from "../provider.js";
import { getRedirectUriForCallback } from '../redirect.js';
import { defineProvider } from '../provider.js';
import { env } from '../../../env.js';

const id = "{= providerId =}";
const displayName = "{= displayName =}";

const env = ensureEnvVarsForProvider(
["GOOGLE_CLIENT_ID", "GOOGLE_CLIENT_SECRET"],
displayName,
);
const id = '{= providerId =}';
const displayName = '{= displayName =}';

const oAuthClient = new Google(
env.GOOGLE_CLIENT_ID,
Expand All @@ -23,6 +18,5 @@ const oAuthClient = new Google(
export const google = defineProvider({
id,
displayName,
env,
oAuthClient,
});
18 changes: 6 additions & 12 deletions waspc/data/Generator/templates/sdk/wasp/server/auth/oauth/providers/keycloak.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
{{={= =}=}}
import { Keycloak } from "arctic";
import { Keycloak } from 'arctic';

import { ensureEnvVarsForProvider } from "../env.js";
import { getRedirectUriForCallback } from "../redirect.js";
import { defineProvider } from "../provider.js";
import { getRedirectUriForCallback } from '../redirect.js';
import { defineProvider } from '../provider.js';
import { env } from '../../../env.js';

const id = "{= providerId =}";
const displayName = "{= displayName =}";

const env = ensureEnvVarsForProvider(
["KEYCLOAK_REALM_URL", "KEYCLOAK_CLIENT_ID", "KEYCLOAK_CLIENT_SECRET"],
displayName,
);
const id = '{= providerId =}';
const displayName = '{= displayName =}';

const oAuthClient = new Keycloak(
env.KEYCLOAK_REALM_URL,
Expand All @@ -24,6 +19,5 @@ const oAuthClient = new Keycloak(
export const keycloak = defineProvider({
id,
displayName,
env,
oAuthClient,
});
Loading
Loading