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

Feature/add smart health links #335

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Feature/add smart health links #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c90da07
Package Smart health links added
Ross-Rawlins Nov 26, 2024
b0bec34
Merge branch 'main' into Feature/add-smart-health-links
Ross-Rawlins Nov 26, 2024
8170df5
Removed VS code history
Ross-Rawlins Nov 26, 2024
7eec06b
Removed VS code history
Ross-Rawlins Nov 26, 2024
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ typings/
# generated files
/docs/.docusaurus
/docs/.cache-loader
.history

# Built Go Binaries
platform-linux
Expand Down
4 changes: 2 additions & 2 deletions build-image.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ TAG_NAME=${1:-latest}
if [ -z "$1" ]; then
# we grep out 'image: jembi/platform:2.x' from which we cut on : and choose the last column
# this will always be the image tag or an empty string
ImageTag=$(grep 'image:' ${PWD}/config.yaml | cut -d : -f 3)
ImageTag=$(grep 'image:' "${PWD}"/config.yaml | cut -d : -f 3)
# only overwrite TAG_NAME if we have a tag present, and it's not just the base image name
if [ -n "$ImageTag" ]; then
TAG_NAME=${ImageTag}
fi
fi

docker build -t jembi/platform:"$TAG_NAME" .
docker build -t jembi/platform:"$TAG_NAME" . --no-cache
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider the use of --no-cache flag

The --no-cache flag forces Docker to rebuild all layers, which can significantly increase build time and resource usage. This is usually not recommended for development workflows unless there are specific caching issues to address.

Consider making it conditional:

-docker build -t jembi/platform:"$TAG_NAME" . --no-cache
+# Use --no-cache only when explicitly requested
+NO_CACHE=${NO_CACHE:-}
+if [ -n "$NO_CACHE" ]; then
+    docker build -t jembi/platform:"$TAG_NAME" . --no-cache
+else
+    docker build -t jembi/platform:"$TAG_NAME" .
+fi
📝 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.

Suggested change
docker build -t jembi/platform:"$TAG_NAME" . --no-cache
# Use --no-cache only when explicitly requested
NO_CACHE=${NO_CACHE:-}
if [ -n "$NO_CACHE" ]; then
docker build -t jembi/platform:"$TAG_NAME" . --no-cache
else
docker build -t jembi/platform:"$TAG_NAME" .
fi

14 changes: 13 additions & 1 deletion config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
projectName: platform
image: jembi/platform:latest
image: jembi/platform:local
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider using a specific version tag instead of 'local'

Using jembi/platform:local might cause deployment issues as this tag isn't available in the Docker registry. For reproducible deployments, consider using one of the available stable version tags like 3.0.1-beta or 2.5.1.

🔗 Analysis chain

Consider using a specific version tag instead of 'local'

Using jembi/platform:local might cause deployment issues if the image isn't available in the deployment environment. For reproducible deployments, consider using a specific version tag.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check available tags for the jembi/platform image
curl -s "https://registry.hub.docker.com/v2/repositories/jembi/platform/tags?page_size=10" | jq -r '.results[].name'

Length of output: 227

logPath: /tmp/logs

packages:
- smart-health-links-portal
- interoperability-layer-openhim
- reverse-proxy-nginx
- fhir-datastore-hapi-fhir
Expand Down Expand Up @@ -30,6 +31,7 @@ packages:
- datalake

profiles:

- name: cdr-dw
packages:
- message-bus-kafka
Expand Down Expand Up @@ -93,3 +95,13 @@ profiles:
- openhim-mapping-mediator
envFiles:
- mpi-qa.env

- name: smart-health-links-portal
packages:
- smart-health-links-portal
- interoperability-layer-openhim
- fhir-datastore-hapi-fhir
- database-postgres
envFiles:
- smart-health-links-portal.env
dev: true
Comment on lines +99 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Profile configuration needs production-ready adjustments

The profile configuration requires the following changes for production readiness:

  1. Remove dev: true flag as this is not suitable for production deployments
  2. Add essential services that are present in all other profiles:
    • monitoring
    • identity-access-manager-keycloak
    • job-scheduler-ofelia
    • reverse-proxy-nginx

The environment file exists and contains the necessary configurations, but the profile itself is missing critical infrastructure components that are standard across all other profiles in the platform.

🔗 Analysis chain

Review profile configuration for production readiness

The profile configuration has several potential issues to address:

  1. The dev: true flag suggests this is for development only
  2. Missing essential services that are present in other profiles:
    • monitoring
    • identity-access-manager-keycloak
    • job-scheduler-ofelia
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the environment file exists and its contents
fd -t f "smart-health-links-portal.env" -x cat {}

Length of output: 532

Script:

#!/bin/bash
# Check other profiles in config.yaml for comparison
cat config.yaml

Length of output: 2646

9 changes: 7 additions & 2 deletions identity-access-manager-keycloak/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@ services:
[
"start",
"--proxy=edge",
"--hostname-url=${KC_FRONTEND_URL}",
"--hostname=${KC_FRONTEND_URL}",
"--import-realm",
]
hostname: identity-access-manager-keycloak
healthcheck:
test: curl --fail http://localhost:8080/health/ready || exit 1
#test: curl --fail http://localhost:8080/health/ready || exit 1
test:
[
"CMD-SHELL",
"exec 3<>/dev/tcp/127.0.0.1/9000;echo -e 'GET /health/ready HTTP/1.1\r\nhost: http://localhost\r\nConnection: close\r\n\r\n' >&3;if [ $$? -eq 0 ]; then echo 'Healthcheck Successful';exit 0;else echo 'Healthcheck Failed';exit 1;fi;",
]
Comment on lines +15 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify health check and resolve port mismatch

The new health check implementation has several potential issues:

  1. Port mismatch: Service exposes port 8080 but health check uses port 9000
  2. Unnecessary complexity compared to the previous curl implementation
  3. Raw TCP connection might bypass TLS/SSL security

Consider reverting to a simpler and more secure implementation:

-      #test: curl --fail http://localhost:8080/health/ready || exit 1
-      test:
-        [
-          "CMD-SHELL",
-          "exec 3<>/dev/tcp/127.0.0.1/9000;echo -e 'GET /health/ready HTTP/1.1\r\nhost: http://localhost\r\nConnection: close\r\n\r\n' >&3;if [ $$? -eq 0 ]; then echo 'Healthcheck Successful';exit 0;else echo 'Healthcheck Failed';exit 1;fi;",
-        ]
+      test: ["CMD", "curl", "-f", "http://localhost:8080/health/ready"]

This implementation:

  • Uses the correct port (8080)
  • Is more maintainable and reliable
  • Follows Docker health check best practices
📝 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.

Suggested change
#test: curl --fail http://localhost:8080/health/ready || exit 1
test:
[
"CMD-SHELL",
"exec 3<>/dev/tcp/127.0.0.1/9000;echo -e 'GET /health/ready HTTP/1.1\r\nhost: http://localhost\r\nConnection: close\r\n\r\n' >&3;if [ $$? -eq 0 ]; then echo 'Healthcheck Successful';exit 0;else echo 'Healthcheck Failed';exit 1;fi;",
]
test: ["CMD", "curl", "-f", "http://localhost:8080/health/ready"]

interval: 10s
timeout: 5s
retries: 5
Expand Down
17 changes: 17 additions & 0 deletions smart-health-links-portal.env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# General
CLUSTERED_MODE=false

# Log
DEBUG=0
BASHLOG_FILE=0
BASHLOG_FILE_PATH=platform.log

# Smart Health Links Portal
# SHLP_NEXTAUTH_URL=http://localhost:3000
SHLP_KEYCLOAK_CLIENT_SECRET="gX0nfIp57bO3bY68br3kdTXung2Auwpr"
SHLP_KEYCLOAK_ISSUER=http://localhost:9088/realms/shlp
SHLP_NEXTAUTH_URL=http://localhost:7457
SHLP_NEXTAUTH_SECRET="ArmMS4LBmH3VTR77UrSAY2lPs04HO0Nk2/4BcU0jMvc="
SHLP_NEXT_LOCAL_KEYCLOAK_URL=http://localhost:9088

KEYCLOAK_IMAGE="keycloak/keycloak:25.0"
8 changes: 8 additions & 0 deletions smart-health-links-portal/docker-compose.dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
version: '3.9'

services:
smart-health-links-portal:
ports:
- target: 3000
published: ${SMART_HEALTH_LINKS_PORTAL_PORT}
mode: host
Comment on lines +6 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security Warning: Avoid using host network mode in Docker

Using mode: host bypasses Docker's network isolation, potentially exposing your service directly to the host network. This can lead to security vulnerabilities and port conflicts.

Consider using the default bridge network mode instead:

    ports:
-      - target: 3000
-        published: ${SMART_HEALTH_LINKS_PORTAL_PORT}
-        mode: host
+      - "${SMART_HEALTH_LINKS_PORTAL_PORT}:3000"
📝 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.

Suggested change
- target: 3000
published: ${SMART_HEALTH_LINKS_PORTAL_PORT}
mode: host
- "${SMART_HEALTH_LINKS_PORTAL_PORT}:3000"

Comment on lines +4 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add essential service configurations

The service configuration is missing several important elements that would improve reliability and maintainability.

Consider adding these configurations:

   smart-health-links-portal:
+    restart: unless-stopped
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:3000/health"]
+      interval: 30s
+      timeout: 10s
+      retries: 3
+    deploy:
+      resources:
+        limits:
+          memory: 512M
+        reservations:
+          memory: 256M
     ports:
       - "${SMART_HEALTH_LINKS_PORTAL_PORT}:3000"

This adds:

  • Automatic restart policy
  • Health monitoring
  • Memory resource constraints

Committable suggestion skipped: line range outside the PR's diff.

46 changes: 46 additions & 0 deletions smart-health-links-portal/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
version: "3.9"

services:
smart-health-links-portal:
image: jembi/smart-health-links-portal:${SMART_HEALTH_LINKS_PORTAL_VERSION}
Comment on lines +1 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add container name and restart policy for improved reliability

Consider adding the following configurations to ensure consistent container naming and automatic recovery:

 smart-health-links-portal:
   image: jembi/smart-health-links-portal:${SMART_HEALTH_LINKS_PORTAL_VERSION}
+  container_name: smart-health-links-portal
+  restart: unless-stopped
📝 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.

Suggested change
version: "3.9"
services:
smart-health-links-portal:
image: jembi/smart-health-links-portal:${SMART_HEALTH_LINKS_PORTAL_VERSION}
version: "3.9"
services:
smart-health-links-portal:
image: jembi/smart-health-links-portal:${SMART_HEALTH_LINKS_PORTAL_VERSION}
container_name: smart-health-links-portal
restart: unless-stopped

networks:
hapi-fhir:
openhim:
reverse-proxy:
postgres:
environment:
HAPI_FHIR_BASE_URL: ${HAPI_FHIR_BASE_URL}
HAPI_FHIR_INSTANCES: ${HAPI_FHIR_INSTANCES}
OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL}
OPENHIM_API_USERNAME: ${OPENHIM_USERNAME}
OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD}
POSTGRES_DB: ${POSTGRES_DB}
POSTGRES_USER: ${POSTGRES_USER}
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
KEYCLOAK_ADMIN: ${KEYCLOAK_ADMIN}
KEYCLOAK_ADMIN_PASSWORD: ${KEYCLOAK_ADMIN_PASSWORD}
KEYCLOAK_CLIENT_ID: ${SHLP_KEYCLOAK_CLIENT_ID}
KEYCLOAK_CLIENT_SECRET: ${SHLP_KEYCLOAK_CLIENT_SECRET}
KEYCLOAK_ISSUER: ${SHLP_KEYCLOAK_ISSUER}
NEXTAUTH_URL: ${SHLP_NEXTAUTH_URL}
NEXTAUTH_SECRET: ${SHLP_NEXTAUTH_SECRET}
NEXT_CONTAINER_KEYCLOAK_ENDPOINT: ${SHLP_NEXT_CONTAINER_KEYCLOAK_ENDPOINT}
NEXT_LOCAL_KEYCLOAK_URL: ${SHLP_NEXT_LOCAL_KEYCLOAK_URL}
NEXT_PUBLIC_DOMAIN: ${SHLP_VIEWER_DOMAIN}
EXTERNAL_URL: ${SHLP_EXTERNAL_URL}
POSTGRES_PRISMA_URL: ${POSTGRES_PRISMA_URL}
Comment on lines +11 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Based on the analysis of the shell script results, I can now provide a final response:

Environment variable security needs improvement

  1. Several files contain hardcoded default passwords in environment variable fallbacks (e.g., "instant101", "dev_password_only")
  2. No Docker secrets implementation found despite handling sensitive data
  3. Some environment variables have validation (e.g., OPENHIM_CONSOLE_BASE_URL check), but most lack validation
  4. Documentation exists but is scattered and incomplete

Recommendations:

  • Remove hardcoded default passwords from environment variable fallbacks
  • Implement Docker secrets for sensitive data (passwords, tokens, API keys)
  • Add validation for all required environment variables
  • Create a centralized environment variable documentation with:
    • Required vs optional variables
    • Validation rules
    • Security considerations
    • Default values (for non-sensitive variables only)
🔗 Analysis chain

Enhance security for sensitive environment variables

Several sensitive credentials and secrets are exposed in environment variables. Consider:

  1. Using Docker secrets for sensitive data
  2. Adding variable validation
  3. Documenting required vs optional variables

Let's verify if these environment variables are properly defined in the environment file:

Let me verify if there are any environment variable validation mechanisms or secure handling practices in place.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if all required environment variables are defined in .env files
echo "Checking for environment variable definitions..."
fd -t f ".*\.env$" . --exec grep -l -E "(POSTGRES_PASSWORD|KEYCLOAK_ADMIN_PASSWORD|KEYCLOAK_CLIENT_SECRET|NEXTAUTH_SECRET)"

# Check for any hardcoded secrets in the repository
echo "Checking for potential hardcoded secrets..."
rg -g "!*.md" -g "!package-lock.json" -i "(password|secret|key|token).*['\"][a-zA-Z0-9]{16,}['\"]"

Length of output: 75129

Script:

#!/bin/bash
# Check for environment variable validation
echo "Checking for environment variable validation..."
rg -g "!*.md" -g "!package-lock.json" "process\.env\." -A 2 -B 2

# Check for Docker secrets usage
echo "Checking for Docker secrets usage..."
rg -g "docker-compose*" "secrets:"

# Check for environment variable documentation
echo "Checking for environment variable documentation..."
fd -t f "README.md" . --exec grep -A 5 -B 5 -i "environment.*variable"

Length of output: 30571


networks:
hapi-fhir:
name: hapi-fhir_public
external: true
openhim:
name: openhim_public
external: true
reverse-proxy:
name: reverse-proxy_public
external: true
postgres:
name: postgres_public
external: true
default:
41 changes: 41 additions & 0 deletions smart-health-links-portal/importer/docker-compose.keycloak.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
version: '3.9'

services:
# container for executing config import scripts for creating the Keycloak Realm used for SHLP User Auth
shlp-keycloak-config-importer:
image: node:erbium-alpine
networks:
public:
environment:
KEYCLOAK_ADMIN: ${KEYCLOAK_ADMIN}
KEYCLOAK_ADMIN_PASSWORD: ${KEYCLOAK_ADMIN_PASSWORD}
NEXTAUTH_URL: ${SHLP_NEXTAUTH_URL}
KEYCLOAK_CLIENT_SECRET: ${SHLP_KEYCLOAK_CLIENT_SECRET}
Comment on lines +9 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Environment variables need documentation and secure handling

Based on the verification results:

  • The environment variables KEYCLOAK_ADMIN, KEYCLOAK_ADMIN_PASSWORD, SHLP_NEXTAUTH_URL, and SHLP_KEYCLOAK_CLIENT_SECRET are used but not properly documented
  • Sensitive credentials (SHLP_KEYCLOAK_CLIENT_SECRET) are hardcoded in the .env file
  • No .env.example file exists to guide users on required variables

Recommendations:

  • Create an .env.example file with placeholder values and descriptions for all required variables
  • Remove hardcoded secrets from source control
  • Document all environment variables in the project's documentation
🔗 Analysis chain

Verify environment variable documentation and security

Please ensure that all required environment variables are properly documented and that sensitive variables (especially KEYCLOAK_ADMIN_PASSWORD and SHLP_KEYCLOAK_CLIENT_SECRET) are handled securely.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for environment variable documentation
echo "Checking for environment variable documentation..."

# Look for environment variable documentation in common locations
fd -t f -e md -e env -e yaml -e yml . | xargs rg -l "KEYCLOAK_ADMIN|KEYCLOAK_ADMIN_PASSWORD|SHLP_NEXTAUTH_URL|SHLP_KEYCLOAK_CLIENT_SECRET"

# Check for any potential hardcoded secrets
echo "Checking for hardcoded secrets..."
rg -i "password|secret|key" --type yaml --type env

Length of output: 707

Script:

#!/bin/bash
# Let's check the content of the found files for environment variables and their documentation
echo "=== Checking smart-health-links-portal.env ==="
cat smart-health-links-portal.env || true

echo -e "\n=== Checking docker-compose files ==="
cat smart-health-links-portal/importer/docker-compose.keycloak.yml
echo -e "\n---"
cat smart-health-links-portal/docker-compose.yml
echo -e "\n---"
cat identity-access-manager-keycloak/docker-compose.yml

echo -e "\n=== Checking for README or documentation files ==="
fd -t f "README" -e md

Length of output: 8559

Script:

#!/bin/bash
# Let's check the main README and documentation for environment variable documentation
echo "=== Checking main README ==="
cat README.md

echo -e "\n=== Checking documentation for environment setup ==="
cat documentation/README.md 2>/dev/null || true
cat "documentation/README (1).md" 2>/dev/null || true

# Let's also check for any .env.example files that might document the variables
echo -e "\n=== Checking for .env.example files ==="
fd -t f ".env.example"

Length of output: 12101


command: sh -c "node keycloakConfig.js"
configs:
- source: keycloakConfig.js
target: /keycloakConfig.js
- source: keycloak-realm.json
target: /keycloak-realm.json
deploy:
replicas: 1
restart_policy:
condition: none
Comment on lines +4 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security and reliability concerns in service configuration

  1. The node:erbium-alpine image is using Node.js v14 (Erbium) which reached end-of-life. Consider upgrading to an active LTS version like node:iron-alpine (v20) or node:hydrogen-alpine (v18).

  2. The configuration script might fail if Keycloak isn't ready when the script runs. Consider adding a health check.

  3. The restart: none policy means we won't retry on failure. Consider using on-failure with a maximum retry count.

Here's the suggested fix:

 shlp-keycloak-config-importer:
-    image: node:erbium-alpine
+    image: node:iron-alpine
     networks:
       public:
     environment:
       KEYCLOAK_ADMIN: ${KEYCLOAK_ADMIN}
       KEYCLOAK_ADMIN_PASSWORD: ${KEYCLOAK_ADMIN_PASSWORD}
       NEXTAUTH_URL: ${SHLP_NEXTAUTH_URL}
       KEYCLOAK_CLIENT_SECRET: ${SHLP_KEYCLOAK_CLIENT_SECRET}
 
-    command: sh -c "node keycloakConfig.js"
+    command: sh -c "wget --wait=30 --tries=10 --retry-connrefused http://keycloak:8080/health/ready && node keycloakConfig.js"
     configs:
       - source: keycloakConfig.js
         target: /keycloakConfig.js
       - source: keycloak-realm.json
         target: /keycloak-realm.json
     deploy:
       replicas: 1
       restart_policy:
-        condition: none
+        condition: on-failure
+        max_attempts: 3
📝 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.

Suggested change
# container for executing config import scripts for creating the Keycloak Realm used for SHLP User Auth
shlp-keycloak-config-importer:
image: node:erbium-alpine
networks:
public:
environment:
KEYCLOAK_ADMIN: ${KEYCLOAK_ADMIN}
KEYCLOAK_ADMIN_PASSWORD: ${KEYCLOAK_ADMIN_PASSWORD}
NEXTAUTH_URL: ${SHLP_NEXTAUTH_URL}
KEYCLOAK_CLIENT_SECRET: ${SHLP_KEYCLOAK_CLIENT_SECRET}
command: sh -c "node keycloakConfig.js"
configs:
- source: keycloakConfig.js
target: /keycloakConfig.js
- source: keycloak-realm.json
target: /keycloak-realm.json
deploy:
replicas: 1
restart_policy:
condition: none
# container for executing config import scripts for creating the Keycloak Realm used for SHLP User Auth
shlp-keycloak-config-importer:
image: node:iron-alpine
networks:
public:
environment:
KEYCLOAK_ADMIN: ${KEYCLOAK_ADMIN}
KEYCLOAK_ADMIN_PASSWORD: ${KEYCLOAK_ADMIN_PASSWORD}
NEXTAUTH_URL: ${SHLP_NEXTAUTH_URL}
KEYCLOAK_CLIENT_SECRET: ${SHLP_KEYCLOAK_CLIENT_SECRET}
command: sh -c "wget --wait=30 --tries=10 --retry-connrefused http://keycloak:8080/health/ready && node keycloakConfig.js"
configs:
- source: keycloakConfig.js
target: /keycloakConfig.js
- source: keycloak-realm.json
target: /keycloak-realm.json
deploy:
replicas: 1
restart_policy:
condition: on-failure
max_attempts: 3


configs:
keycloakConfig.js:
file: ./volume/keycloak/keycloakConfig.js
name: keycloakConfig.js-${keycloakConfig_js_DIGEST:?err}
labels:
name: smart-health-links-importer
keycloak-realm.json:
file: ./volume/keycloak/keycloak-realm.json
name: keycloak-realm.json-${keycloak_realm_json_DIGEST:?err}
labels:
name: smart-health-links-importer

networks:
public:
name: keycloak_public
external: true
43 changes: 43 additions & 0 deletions smart-health-links-portal/importer/docker-compose.openhim.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
version: '3.9'

services:
# container for executing config import scripts for creating the OpenHIM channels used by the Mediator
shlp-openhim-config-importer:
image: node:erbium-alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use a specific version tag for the Node.js image

Using erbium-alpine without a specific version could lead to unexpected updates and potential breaking changes.

-    image: node:erbium-alpine
+    image: node:erbium-alpine@sha256:<specific-digest>

Committable suggestion skipped: line range outside the PR's diff.

networks:
openhim:
default:
environment:
OPENHIM_API_USERNAME: ${OPENHIM_USERNAME}
OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD}
# Reject unauthorised is only needed if the OpenHIM's SSL is not setup
NODE_TLS_REJECT_UNAUTHORIZED: 0
OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL}
Comment on lines +11 to +15
Copy link
Contributor

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 service depends on environment variables but doesn't validate their presence. Consider adding a script to check these variables before starting the service.

     environment:
-      OPENHIM_API_USERNAME: ${OPENHIM_USERNAME}
-      OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD}
-      # Reject unauthorised is only needed if the OpenHIM's SSL is not setup
-      NODE_TLS_REJECT_UNAUTHORIZED: 0
-      OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL}
+      OPENHIM_API_USERNAME: ${OPENHIM_USERNAME:?err}
+      OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD:?err}
+      NODE_TLS_REJECT_UNAUTHORIZED: 0
+      OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL:?err}
📝 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.

Suggested change
OPENHIM_API_USERNAME: ${OPENHIM_USERNAME}
OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD}
# Reject unauthorised is only needed if the OpenHIM's SSL is not setup
NODE_TLS_REJECT_UNAUTHORIZED: 0
OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL}
OPENHIM_API_USERNAME: ${OPENHIM_USERNAME:?err}
OPENHIM_API_PASSWORD: ${OPENHIM_PASSWORD:?err}
# Reject unauthorised is only needed if the OpenHIM's SSL is not setup
NODE_TLS_REJECT_UNAUTHORIZED: 0
OPENHIM_CONSOLE_BASE_URL: ${OPENHIM_CONSOLE_BASE_URL:?err}

command: sh -c "node openhimConfig.js"
configs:
- source: openhimConfig.js
target: /openhimConfig.js
- source: openhim-import.json
target: /openhim-import.json
deploy:
replicas: 1
restart_policy:
condition: none

configs:
openhimConfig.js:
file: ./volume/openhim/openhimConfig.js
name: openhimConfig.js-${openhimConfig_js_DIGEST:?err}
labels:
name: smart-health-links-importer
openhim-import.json:
file: ./volume/openhim/openhim-import.json
name: openhim-import.json-${openhim_import_json_DIGEST:?err}
labels:
name: smart-health-links-importer

networks:
openhim:
name: openhim_public
external: true
default:
43 changes: 43 additions & 0 deletions smart-health-links-portal/importer/docker-compose.postgres.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
version: '3.9'

services:
shlp_db_config:
image: node:erbium-alpine
command: sh -c "pwd && cd /importer && ls && npm i && ls && node create-db.js"
Comment on lines +4 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update Node.js image and improve command robustness

Several issues need attention:

  1. The node:erbium-alpine image is using Node.js 12.x which reached end-of-life in April 2022
  2. The command includes debug statements (pwd, ls) which shouldn't be in production
  3. The command chain could fail silently if any step fails

Apply this diff to address these issues:

  shlp_db_config:
-   image: node:erbium-alpine
-   command: sh -c "pwd && cd /importer && ls && npm i && ls && node create-db.js"
+   image: node:18-alpine
+   command: sh -c "cd /importer && set -e && npm ci && node create-db.js"

Notes:

  • Using Node.js 18 LTS for better security and performance
  • Removed debug commands
  • Added set -e to fail fast if any command fails
  • Using npm ci instead of npm i for reliable dependency installation
📝 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.

Suggested change
shlp_db_config:
image: node:erbium-alpine
command: sh -c "pwd && cd /importer && ls && npm i && ls && node create-db.js"
shlp_db_config:
image: node:18-alpine
command: sh -c "cd /importer && set -e && npm ci && node create-db.js"

configs:
- source: package.json
target: /importer/package.json
- source: create-db.js
target: /importer/create-db.js
networks:
postgres:
environment:
POSTGRES_SERVICE: ${POSTGRES_SERVICE}
POSTGRES_PORT: ${POSTGRES_PORT}
POSTGRES_USER: ${POSTGRES_USER_ADMIN}
POSTGRES_DATABASE: ${POSTGRES_USER_ADMIN_DATABASE}
POSTGRES_PASSWORD: ${POSTGRES_USER_ADMIN_PASSWORD}
NEW_DATABASE_NAME: ${SHLP_POSTGRES_DB}
NEW_DATABASE_USER: ${SHLP_POSTGRES_USER}
NEW_DATABASE_PASSWORD: ${SHLP_POSTGRES_PASSWORD}
Comment on lines +14 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure environment variables are properly validated

The service relies on multiple environment variables for database configuration. Consider adding validation to ensure these variables are set before the service starts.

Add this validation to the beginning of your create-db.js script:

const requiredEnvVars = [
  'POSTGRES_SERVICE',
  'POSTGRES_PORT',
  'POSTGRES_USER',
  'POSTGRES_DATABASE',
  'POSTGRES_PASSWORD',
  'NEW_DATABASE_NAME',
  'NEW_DATABASE_USER',
  'NEW_DATABASE_PASSWORD'
];

for (const envVar of requiredEnvVars) {
  if (!process.env[envVar]) {
    console.error(`Error: ${envVar} environment variable is required`);
    process.exit(1);
  }
}

deploy:
replicas: 1
restart_policy:
condition: none

networks:
postgres:
name: postgres_public
external: true

configs:
package.json:
file: ./volume/postgres/package.json
name: package.json-${package_json_DIGEST:?err}
labels:
name: smart-health-links-importer
create-db.js:
file: ./volume/postgres/create-db.js
name: create_db.js-${create_db_js_DIGEST:?err}
labels:
name: smart-health-links-importer
Loading