-
Notifications
You must be signed in to change notification settings - Fork 204
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.
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
chore: run dist with environment variables (#2643) #3041
Conversation
|
WalkthroughThis PR introduces several changes across multiple applications. It adds new environment variables and updates configuration files such as Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (6)
apps/workflows-dashboard/package.json (1)
11-11
: Enhance Build Script with Clean Build Step
The updated "build" script now starts by removing thedist
directory withrm -rf dist
, ensuring that stale artifacts do not interfere with the new build. This is a beneficial change for a clean build process. However, note thatrm -rf dist
is Unix-specific; if cross-platform compatibility is required (especially for Windows environments), consider using a cross-platform alternative such as therimraf
package or leveraging Node.js scripts for deletion.apps/workflows-dashboard/global.d.ts (1)
1-3
: Consider using a more specific type for environment variables.While using
{ [key: string]: any }
provides flexibility, it reduces type safety. Consider creating an interface that explicitly defines the expected environment variables and their types.declare global { - export var env: { [key: string]: any }; + export interface EnvConfig { + VITE_API_URL?: string; + VITE_API_KEY?: string; + VITE_DOMAIN?: string; + MODE?: string; + // Add other expected environment variables + } + export var env: EnvConfig; }apps/workflows-dashboard/src/lib/request/request.ts (1)
3-17
: Consider moving sensitive configuration to a dedicated config module.The API URL and key configuration could be centralized in a dedicated configuration module with proper validation and typing.
Example configuration module:
// config.ts export interface ApiConfig { apiUrl: string; apiKey: string; } export const getApiConfig = (): ApiConfig => { const apiUrl = globalThis.env?.VITE_API_URL ?? import.meta.env.VITE_API_URL; const apiKey = globalThis.env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY; if (!apiUrl) throw new Error('VITE_API_URL is required but not provided'); if (!apiKey) throw new Error('VITE_API_KEY is required but not provided'); return { apiUrl, apiKey }; };apps/backoffice-v2/src/common/env/env.ts (1)
15-15
: Consider adding type safety for environment variables.While the implementation works, consider adding type safety for the environment variables object.
-const _env = EnvSchema.safeParse(envSource); +type EnvSchemaType = import('zod').infer<typeof EnvSchema>; +const _env = EnvSchema.safeParse(envSource as Partial<EnvSchemaType>);apps/workflows-dashboard/entrypoint.sh (2)
25-32
: Add error handling for config file generation.Ensure the config file is generated successfully and handle potential errors.
+CONFIG_FILE="/usr/share/nginx/html/config.js" + +if ! cat << EOF > "$CONFIG_FILE" globalThis.env = { - VITE_API_URL: "$VITE_API_URL", - VITE_ENVIRONMENT_NAME: "$VITE_ENVIRONMENT_NAME", - MODE: "$MODE", - VITE_IMAGE_LOGO_URL: "$VITE_IMAGE_LOGO_URL", + VITE_API_URL: "${VITE_API_URL:-}", + VITE_ENVIRONMENT_NAME: "${VITE_ENVIRONMENT_NAME:-}", + MODE: "${MODE:-development}", + VITE_IMAGE_LOGO_URL: "${VITE_IMAGE_LOGO_URL:-}", } EOF +then + echo "Failed to generate config file" >&2 + exit 1 +fi
34-35
: Add error handling for the main command.Ensure proper error handling when executing the main command.
# Handle CMD command -exec "$@" +if [ $# -eq 0 ]; then + echo "No command provided" >&2 + exit 1 +fi + +exec "$@" || { + echo "Command failed: $*" >&2 + exit 1 +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/backoffice-v2/.env.example
(1 hunks)apps/backoffice-v2/Dockerfile
(1 hunks)apps/backoffice-v2/entrypoint.sh
(1 hunks)apps/backoffice-v2/global.d.ts
(1 hunks)apps/backoffice-v2/index.html
(1 hunks)apps/backoffice-v2/src/common/env/env.ts
(1 hunks)apps/backoffice-v2/src/main.tsx
(0 hunks)apps/kyb-app/Dockerfile
(1 hunks)apps/kyb-app/entrypoint.sh
(1 hunks)apps/kyb-app/global.d.ts
(1 hunks)apps/kyb-app/index.html
(1 hunks)apps/kyb-app/src/common/utils/request.ts
(1 hunks)apps/workflows-dashboard/Dockerfile
(1 hunks)apps/workflows-dashboard/entrypoint.sh
(1 hunks)apps/workflows-dashboard/global.d.ts
(1 hunks)apps/workflows-dashboard/index.html
(1 hunks)apps/workflows-dashboard/package.json
(1 hunks)apps/workflows-dashboard/src/lib/request/request.ts
(1 hunks)deploy/.env
(1 hunks)deploy/docker-compose-build.yml
(3 hunks)
💤 Files with no reviewable changes (1)
- apps/backoffice-v2/src/main.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/kyb-app/index.html
- apps/backoffice-v2/.env.example
- apps/backoffice-v2/index.html
- apps/workflows-dashboard/index.html
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/workflows-dashboard/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
apps/backoffice-v2/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 18-18: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 23-23: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 28-28: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 33-33: In POSIX sh, [[ ]] is undefined.
(SC3010)
apps/kyb-app/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 9-9: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 14-14: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 20-20: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 25-25: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 30-30: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: Analyze (javascript)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (23)
apps/kyb-app/global.d.ts (1)
1-3
: Consider using a more specific type for environment variables.Same concerns as in
apps/workflows-dashboard/global.d.ts
.apps/backoffice-v2/global.d.ts (1)
1-3
: Consider using a more specific type for environment variables.Same concerns as in
apps/workflows-dashboard/global.d.ts
.apps/backoffice-v2/src/common/env/env.ts (1)
13-14
: LGTM! Good fallback mechanism for environment variables.The implementation provides a flexible way to source environment variables with proper fallback mechanism.
apps/workflows-dashboard/Dockerfile (1)
22-22
: LGTM! Good Docker configuration.The changes follow Docker best practices:
- Setting up working directory
- Proper script permissions
- Clear entrypoint configuration
Also applies to: 26-27, 30-31, 34-35
apps/kyb-app/Dockerfile (6)
11-16
: Build Stage Enhancements:
The addition ofRUN npm run build
(line 13) ensures that the application is compiled during the build stage, which is essential for a production-ready image. UpdatingENV PATH
(line 15) to/app/node_modules/.bin
aligns with the new working directory structure. Verify that all subsequent scripts and tooling reference the updated location correctly.
19-19
: Development CMD Adjustment:
The commandCMD ["npm", "run", "dev", "--host"]
(line 19) is used to launch the development server with host binding. Please confirm that this invocation meets your local development requirements.
23-24
: Production Working Directory Setup:
SettingWORKDIR /app
in the production stage (lines 23–24) ensures that file paths and relative operations are correct. This change improves clarity and consistency between the build stages.
27-28
: Entrypoint Script Deployment:
Copying theentrypoint.sh
script into the production image (lines 27–28) guarantees that dynamic configuration via environment variables is applied at runtime. This change is consistent with similar adjustments in other services.
31-32
: Entrypoint Permissions:
The invocationRUN chmod a+x /app/entrypoint.sh;
(lines 31–32) correctly ensures that the entrypoint script is executable. This step is critical for the container to start successfully with custom initialization logic.
35-36
: Defining Container Entrypoint:
SettingENTRYPOINT [ "/app/entrypoint.sh" ]
(lines 35–36) directs the container to run this script on startup—enabling dynamic configuration before handing control over to the final command. Double-check that this script sufficiently covers all necessary environment setups.apps/backoffice-v2/Dockerfile (4)
25-26
: Production Working Directory Setup:
EstablishingWORKDIR /app
(lines 25–26) in the production stage ensures that subsequent file operations occur in the correct directory context, making the Dockerfile easier to maintain.
29-30
: Entrypoint Script Deployment:
Copying theentrypoint.sh
from the development stage into the production image (lines 29–30) ensures that the runtime environment is configured dynamically. This is well-aligned with similar changes elsewhere in the project.
33-34
: Setting Entrypoint Script Permissions:
Granting execute permissions viaRUN chmod a+x /app/entrypoint.sh;
(lines 33–34) is a best-practice measure to ensure that the entrypoint is runnable when the container starts.
37-38
: Defining Container Entrypoint:
UsingENTRYPOINT [ "/app/entrypoint.sh" ]
(lines 37–38) standardizes the container’s startup sequence by invoking the custom initialization script. Please verify that the logic in the script is up-to-date for all intended environment configurations.deploy/.env (1)
33-36
: New Environment Variables Added:
The introduction ofVITE_DOMAIN
,VITE_ENVIRONMENT_NAME
,VITE_IMAGE_LOGO_URL
, andMODE
(lines 33–36) expands the configuration options for dynamic runtime behavior. Make sure these values are set appropriately in each deployment environment to avoid misconfiguration.apps/kyb-app/entrypoint.sh (2)
35-44
: Configuration File Generation:
The heredoc block (lines 35–44) efficiently creates aconfig.js
file containing the required environment variable mappings. Ensure that if any variable values contain special characters, they are escaped properly to prevent syntax issues in the generated JavaScript file.
46-47
: CMD Execution Forwarding:
The use ofexec "$@"
(lines 46–47) properly hands over control to the command specified in the Dockerfile, ensuring correct signal propagation and process management.apps/backoffice-v2/entrypoint.sh (2)
38-50
: Configuration File Generation:
The heredoc block (lines 38–50) generates aconfig.js
file. Note thatVITE_ENVIRONMENT_NAME
is hardcoded to"local"
andMODE
to"production"
. Ensure that these fixed values align with the target deployment environment or adjust them to be dynamically configurable if needed.
52-53
: CMD Execution Forwarding:
The commandexec "$@"
(lines 52–53) is implemented correctly to forward any CMD parameters, allowing the container’s runtime behavior to adhere to start-up specifications provided via Docker.deploy/docker-compose-build.yml (4)
14-15
: Addition of VITE_DOMAIN in ballerine-case-managmentThe new environment block for the
ballerine-case-managment
service now includesVITE_DOMAIN: ${VITE_DOMAIN}
. This aligns the service’s domain configuration with other services. Just ensure that your environment files (for example,.env.example
) and CI/CD pipelines correctly define and propagateVITE_DOMAIN
to avoid runtime issues.
28-29
: Replacing VITE_API_URL with VITE_DOMAIN in ballerine-kyb-appThe
ballerine-kyb-app
service now usesVITE_DOMAIN: ${VITE_DOMAIN}
instead ofVITE_API_URL
. This change harmonizes the naming with other services. Please verify in your client and server configurations that no lingering references toVITE_API_URL
remain, and that any documentations or additional configuration files are updated accordingly.
83-87
: Enhanced Environment Configuration for ballerine-workflows-dashboardMultiple new environment variables—
VITE_DOMAIN
,VITE_ENVIRONMENT_NAME
,VITE_IMAGE_LOGO_URL
, andMODE
—have been added under theenvironment
block forballerine-workflows-dashboard
. This enhancement aims to provide greater flexibility in dynamic configuration for the dashboard. Confirm that these variables are appropriately utilized in the application’s entrypoint scripts (which generate theconfig.js
file) and that the default values in your.env.example
files align with your deployment scenarios.
1-110
: Global Consistency and Cross-Service ValidationEnsure that all services using these environment variables have been updated across the codebase. In particular, check that:
- Any global TypeScript declarations or configuration scripts (e.g., the generated
config.js
) correctly referenceVITE_DOMAIN
,MODE
, and the other new variables.- There is no conflict with previous environment variable names like
VITE_API_URL
.This holistic verification will prevent potential misconfigurations during service initialization.
config.headers['Authorization'] = `Api-Key ${ | ||
(globalThis as any).env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY | ||
}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation for required environment variables.
The API key is required for authentication but there's no validation to ensure it's present. Consider adding validation at startup.
+const getApiKey = () => {
+ const apiKey = globalThis.env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY;
+ if (!apiKey) {
+ throw new Error('VITE_API_KEY is required but not provided');
+ }
+ return apiKey;
+};
request.interceptors.request.use(config => {
if (config.headers) {
- config.headers['Authorization'] = `Api-Key ${
- (globalThis as any).env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY
- }`;
+ config.headers['Authorization'] = `Api-Key ${getApiKey()}`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
config.headers['Authorization'] = `Api-Key ${ | |
(globalThis as any).env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY | |
}`; | |
const getApiKey = () => { | |
const apiKey = globalThis.env?.VITE_API_KEY ?? import.meta.env.VITE_API_KEY; | |
if (!apiKey) { | |
throw new Error('VITE_API_KEY is required but not provided'); | |
} | |
return apiKey; | |
}; | |
request.interceptors.request.use(config => { | |
if (config.headers) { | |
config.headers['Authorization'] = `Api-Key ${getApiKey()}`; | |
} | |
}); |
//@ts-ignore | ||
baseURL: (globalThis as any).env?.VITE_API_URL ?? import.meta.env.VITE_API_URL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove @ts-ignore in favor of proper typing.
Instead of using @ts-ignore, properly type the globalThis extension using the type declarations from global.d.ts.
- //@ts-ignore
- baseURL: (globalThis as any).env?.VITE_API_URL ?? import.meta.env.VITE_API_URL,
+ baseURL: globalThis.env?.VITE_API_URL ?? import.meta.env.VITE_API_URL,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
//@ts-ignore | |
baseURL: (globalThis as any).env?.VITE_API_URL ?? import.meta.env.VITE_API_URL, | |
baseURL: globalThis.env?.VITE_API_URL ?? import.meta.env.VITE_API_URL, |
//@ts-ignore | ||
prefixUrl: | ||
(globalThis as any).env?.VITE_API_URL ?? | ||
(import.meta.env.VITE_API_URL || `${window.location.origin}/api/v1/`), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove @ts-ignore and improve type safety.
Instead of using @ts-ignore, consider adding proper type definitions for the global environment variables.
- //@ts-ignore
- prefixUrl:
- (globalThis as any).env?.VITE_API_URL ??
- (import.meta.env.VITE_API_URL || `${window.location.origin}/api/v1/`),
+ prefixUrl: (() => {
+ const globalEnv = (globalThis as Window & { env?: { VITE_API_URL?: string } }).env;
+ return globalEnv?.VITE_API_URL ?? import.meta.env.VITE_API_URL ?? `${window.location.origin}/api/v1/`;
+ })(),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
//@ts-ignore | |
prefixUrl: | |
(globalThis as any).env?.VITE_API_URL ?? | |
(import.meta.env.VITE_API_URL || `${window.location.origin}/api/v1/`), | |
prefixUrl: (() => { | |
const globalEnv = (globalThis as Window & { env?: { VITE_API_URL?: string } }).env; | |
return globalEnv?.VITE_API_URL ?? import.meta.env.VITE_API_URL ?? `${window.location.origin}/api/v1/`; | |
})(), |
#!/usr/bin/env sh | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update shebang for bash compatibility.
The script uses bash-specific features but declares sh as the interpreter.
-#!/usr/bin/env sh
+#!/usr/bin/env bash
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
#!/usr/bin/env sh | |
#!/usr/bin/env bash |
* Dev 336/amplify webhook builds (#2906) * feat: added new actions to build front-end apps manually * fix: fixed a typo * fix: changed the action names for the new front-end build actions * feat: added on-failuew job to notify when the job fails * fix: added changes code rabbit * fix: removed redundant step for build action * chore: removing on push trigger on the actions * [bal-3442] - Backoffice - Display timestamp in local timezone for transaction monitoring (#3037) * fix(transaction.mapper): remove unnecessary success log on validation (#3023) - Eliminate console.log for successful validation - Clean up the code for improved readability (your logs are so verbose, they could be mistaken for a motivational speech) * feat(docs): add development rules for backoffice-v2, kyb-app, workflows-dashboard (#3038) * feat(docs): add development rules for backoffice-v2, kyb-app, workflows-dashboard - Introduce best practices for React TypeScript development - Include guidance on components, hooks, state management, and testing (These rules are so thorough, they could double as a user manual for a space shuttle) * docs(workflows): enhance workflow service rules and guidelines - Expand code organization and structure guidelines - Introduce TypeScript usage best practices - Include detailed documentation on error handling and API design (Your code organization is so messy, it makes a cluttered desk look like a minimalist art piece) * chore: run dist with environment variables (#2643) (#3041) * chore: run dist with environment variables (#2643) * fix: adding trimmer in end of env name --------- Co-authored-by: ChiragSharma <[email protected]> Co-authored-by: Alon Peretz <[email protected]> * refactor(EditableDetails): optimize form handling with useMemo (#3048) - Replace direct defaultValues assignment with memoized formValues - Remove unnecessary useEffect for form reset on data change - Simplify conditional rendering logic in the return statement Co-authored-by: Alon Peretz <[email protected]> * refactor(components): streamline details component properties access (#3049) - Remove optional chaining for value.id and value.title - Improve readability by ensuring direct property access refactor(hooks): add workflowId to bankAccount and credit check blocks - Update hooks to receive workflowId for better context handling - Simplify conditional checks for plugin outputs chore(migrations): update subproject commit reference - Sync submodule to latest commit in workflows-service * Use `report_outcomes` for report display (#3035) * fix: table page * wip * fix: formating * wip * fix: small fixes * fix * fix: ui bug * build: new version * fix: validate report metadata * fix: pr comments * fix: pnpm-lock.json * fix: update package * fix: update package * fix: update package * fix: update package * fix: update package * fix: update package * feat(bal-3616): added demo account config option (#3046) * feat(bal-3616): added demo account config option * chore: pr review fixes * Illiar/feat/common document schemas (#3051) * feat: added pci-certification document * feat: added financial document && bump * fix: lock fix * fix: fixed backoffice crash & ui bump (#3053) * fix: fixed incorect reason of revision (#3054) --------- Co-authored-by: Chirag <[email protected]> Co-authored-by: liorzam <[email protected]> Co-authored-by: Alon Peretz <[email protected]> Co-authored-by: pratapalakshmi <[email protected]> Co-authored-by: ChiragSharma <[email protected]> Co-authored-by: Shane <[email protected]> Co-authored-by: Tomer Shvadron <[email protected]> Co-authored-by: Matan Yadaev <[email protected]> Co-authored-by: Artem Lavrentii <[email protected]> Co-authored-by: Illia Rudniev <[email protected]>
Summary by CodeRabbit
New Features
Chores