Make node-rdkafka required dependency

[tangledbytes]

Describe the Problem

Downstream builds ignore optionalDependency which means new downstream noobaa-core images do not contain node-rdkafka.

Explain the Changes

This PR makes node-rdkafka dependency required.

Issues: Fixed #xxx / Gap #xxx

1. https://issues.redhat.com/browse/DFBUGS-5554

Testing Instructions:

• Doc added/updated
• Tests added

Summary by CodeRabbit

• Chores
  • Updated package configuration to make a previously optional component a required dependency, ensuring it is consistently available for runtime stability.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR moves node-rdkafka (version 3.6.1) out of optionalDependencies and adds it under dependencies in package.json, removing the optionalDependencies block.

Changes

Cohort / File(s)	Summary
Dependency Management package.json	Removed the optionalDependencies block containing node-rdkafka and added "node-rdkafka": "3.6.1" under dependencies, making the package required.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

• rdkafka change to optional npm dependency #9344: Also modifies node-rdkafka handling in package.json with opposite intent (keeps it optional and adds runtime/build checks).
• Bumping deps to avoid CVE (13/11/2025) #9303: Updates node-rdkafka version in package.json (related to the same dependency).

Suggested labels

size/S

Suggested reviewers

• nimrod-becker
• liranmauda
• alphaprinz

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Make node-rdkafka required dependency' directly and concisely describes the main change—moving node-rdkafka from optionalDependencies to dependencies in package.json.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into master

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@package.json`:
- Line 113: Pick and implement one consistent approach: either make node-rdkafka
truly required or keep it optional. If required: change
src/util/notifications_util.js to stop using require_optional() and use a direct
require('node-rdkafka') in the Notification/producer code paths (remove
null-check fallbacks), update any functions referencing require_optional()
accordingly, and update src/deploy/NVA_build/Base.Dockerfile to always install
librdkafka (remove USE_RDKAFKA conditional and outdated comment). If optional:
revert node-rdkafka back to optionalDependencies in package.json (or ensure npm
install uses --save-optional), keep require_optional() usage in
notifications_util.js and its null-safe logic, and keep Dockerfile conditional
logic and comment consistent with optional availability; ensure behavior/tests
still handle the missing module path.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any conditional/try-catch imports of node-rdkafka
rg -n 'node-rdkafka' --type js --type ts -C 3

Repository: noobaa/noobaa-core

Length of output: 589

---

🏁 Script executed:

#!/bin/bash
# Check for Dockerfiles and special handling of optional dependencies
fd -e Dockerfile -e Makefile | head -20

Repository: noobaa/noobaa-core

Length of output: 457

---

🏁 Script executed:

#!/bin/bash
# Check for any try-catch or require patterns around node-rdkafka
rg -n 'require.*node-rdkafka|import.*node-rdkafka' -C 5

Repository: noobaa/noobaa-core

Length of output: 1685

---

🏁 Script executed:

#!/bin/bash
# Look at package.json structure to understand the overall dependency strategy
head -150 package.json

Repository: noobaa/noobaa-core

Length of output: 5523

---

🏁 Script executed:

#!/bin/bash
# Check the require_optional implementation to understand how it works
rg -n 'require_optional' -A 10 src/util/js_utils.js

Repository: noobaa/noobaa-core

Length of output: 375

---

🏁 Script executed:

#!/bin/bash
# Check all usages of require_optional in the codebase
rg -n 'require_optional' --type js --type ts | head -30

Repository: noobaa/noobaa-core

Length of output: 369

---

🏁 Script executed:

#!/bin/bash
# Check the exact Dockerfile context around the USE_RDKAFKA logic
sed -n '15,40p' src/deploy/NVA_build/Base.Dockerfile

Repository: noobaa/noobaa-core

Length of output: 1158

---

🏁 Script executed:

#!/bin/bash
# Verify if the PR updated notifications_util.js or any other files related to rdkafka
git diff --name-only HEAD~1 HEAD 2>/dev/null | grep -E '(notifications|rdkafka|Dockerfile)' || echo "Git history not available"

Repository: noobaa/noobaa-core

Length of output: 87

---

node-rdkafka is now required, but code and Dockerfile still treat it as optional — requires coordination.

Moving node-rdkafka from optionalDependencies to dependencies creates a critical architectural mismatch:

• Code (src/util/notifications_util.js): Imports via require_optional(), expecting graceful null if missing
• Dockerfile (src/deploy/NVA_build/Base.Dockerfile): Still has conditional logic (USE_RDKAFKA=1) and outdated comment stating "It was added to optionalDependencies because it's not available on all platforms"
• package.json: Now requires node-rdkafka — npm install will hard-fail without librdkafka

This inconsistency must be resolved:

1. If making rdkafka required, remove require_optional() and update code to assume it's always available
2. If keeping optional behavior, revert to optionalDependencies or add --save-optional to the npm install
3. Update Dockerfile comment and conditional logic accordingly

The PR appears incomplete — it moved the dependency but didn't update consuming code or build logic to match the new requirement level.

🤖 Prompt for AI Agents

In `@package.json` at line 113, Pick and implement one consistent approach: either
make node-rdkafka truly required or keep it optional. If required: change
src/util/notifications_util.js to stop using require_optional() and use a direct
require('node-rdkafka') in the Notification/producer code paths (remove
null-check fallbacks), update any functions referencing require_optional()
accordingly, and update src/deploy/NVA_build/Base.Dockerfile to always install
librdkafka (remove USE_RDKAFKA conditional and outdated comment). If optional:
revert node-rdkafka back to optionalDependencies in package.json (or ensure npm
install uses --save-optional), keep require_optional() usage in
notifications_util.js and its null-safe logic, and keep Dockerfile conditional
logic and comment consistent with optional availability; ensure behavior/tests
still handle the missing module path.

[tangledbytes]

Moved this to draft as I am trying to get downstream to not skip optional deps at least when building for linux/amd64. If not then would move this out of draft.