Skip to content

KMS-697: Upgrade to node version lts/krypton (node 24) - #129

Merged
htranho merged 5 commits into
mainfrom
KMS-697
Aug 13, 2026
Merged

KMS-697: Upgrade to node version lts/krypton (node 24)#129
htranho merged 5 commits into
mainfrom
KMS-697

Conversation

@htranho

@htranho htranho commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Overview

What is the feature?

Update to use node version 24 (also node version in github workflow)

What is the Solution?

Update to use node 24
Ran 'npm audit fix'
Fix 'curl' not found when starting rdf4j server
Fix updating when change code in dev mode.

What areas of the application does this impact?

Node version and local development

Testing

Sanity test. Change code and restart server to verify updating.

Attachments

N/A

Checklist

  • I have added automated tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Summary by CodeRabbit

  • Enhancements

    • Upgraded the application, build tools, deployment images, and serverless runtime to Node.js 24.
    • Improved container security by running builds as a non-root user.
  • Bug Fixes

    • Local development now clears previous generated deployment artifacts before synthesis.
    • Added curl to the RDF database container for required tooling support.

@htranho
htranho marked this pull request as draft August 7, 2026 17:26
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac3ae6a8-26e4-4cf8-b45b-0a7ac9292eb3

📥 Commits

Reviewing files that changed from the base of the PR and between 2c71808 and 62c96b9.

📒 Files selected for processing (1)
  • cdk/app/lib/helper/NodeLambdaRuntime.ts

📝 Walkthrough

Walkthrough

The project updates Node.js versions in configuration, CI, Docker images, and Lambda runtime settings. Build images now run as the node user. Local CDK synthesis removes prior output. The RDFDB image installs curl.

Changes

Runtime and local development updates

Layer / File(s) Summary
Align Node.js runtimes
.nvmrc, .github/workflows/ci.yml, Dockerfile, bin/Dockerfile, bin/deploy-bamboo.sh, cdk/app/lib/helper/NodeLambdaRuntime.ts
Node.js versions change to lts/krypton, Node.js 24, and Lambda Node.js 24.x. The main build images assign /build to node:node and run as node.
Update local development flow
bin/start-local.sh
The script removes the existing cdk/cdk.out directory before CDK synthesis.
Add RDFDB container tool
cdk/rdfdb/docker/Dockerfile
The image installation step adds curl.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: william-valencia, eudoroolivares2016

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: upgrading the project to Node.js 24, including the LTS codename.
Description check ✅ Passed The description includes all required sections and summarizes the changes, testing, impacted areas, and checklist status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch KMS-697

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.73%. Comparing base (95005dd) to head (62c96b9).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   99.71%   99.73%   +0.01%     
==========================================
  Files         234      234              
  Lines        6346     6391      +45     
  Branches     1866     1900      +34     
==========================================
+ Hits         6328     6374      +46     
  Misses         16       16              
+ Partials        2        1       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@htranho
htranho marked this pull request as ready for review August 7, 2026 20:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Dockerfile`:
- Line 1: Configure the Docker image to run as the non-root node user by adding
USER node after all installation steps. Ensure /build and any
application-written paths are owned by node before switching users.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c7db598f-9c7d-4932-bcdb-dba37e555c84

📥 Commits

Reviewing files that changed from the base of the PR and between c67d1c7 and 0d51a21.

📒 Files selected for processing (6)
  • Dockerfile
  • bin/Dockerfile
  • bin/deploy-bamboo.sh
  • bin/env/local_env.sh
  • bin/start-local.sh
  • cdk/rdfdb/docker/Dockerfile

Comment thread Dockerfile
@cgokey

cgokey commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

The local, CI, and Docker runtimes are now Node 24, but deployed Lambdas still use NODEJS_22_X. We should also update this to lambda.Runtime.NODEJS_24_X so AWS also runs Krypton?
(See kms/cdk/app/lib/helper/NodeLambdaRuntime.ts)

Comment thread bin/env/local_env.sh
export RDF4J_PASSWORD="${RDF4J_PASSWORD:-rdf4j}"
export CMR_BASE_URL="${CMR_BASE_URL:-}"
export CMR_BASE_URL="${CMR_BASE_URL:-https://cmr.sit.earthdata.nasa.gov}"
export RDF4J_CONTAINER_MEMORY_LIMIT="${RDF4J_CONTAINER_MEMORY_LIMIT:-2048}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes local CMR requests default to shared SIT, including write operations. Can we put this back to the empty deault?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverted the default to empty

@htranho

htranho commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

The local, CI, and Docker runtimes are now Node 24, but deployed Lambdas still use NODEJS_22_X. We should also update this to lambda.Runtime.NODEJS_24_X so AWS also runs Krypton? (See kms/cdk/app/lib/helper/NodeLambdaRuntime.ts)

Done, now using v 24

@htranho
htranho merged commit 091f010 into main Aug 13, 2026
7 checks passed
@htranho
htranho deleted the KMS-697 branch August 13, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants