Skip to content

fix(tools/node): merge NODE_OPTIONS with process environment, if set#5962

Closed
jamietanna wants to merge 2 commits intomainfrom
feat/node-options
Closed

fix(tools/node): merge NODE_OPTIONS with process environment, if set#5962
jamietanna wants to merge 2 commits intomainfrom
feat/node-options

Conversation

@jamietanna
Copy link
Copy Markdown
Contributor

As noted in renovatebot/renovate#40942,
we need to introduce a means to control the NODE_OPTIONS environment
variable to specify the memory limits for a given Node child process.

Before we do this, Containerbase needs to append the --use-openssl-ca
option, instead of replacing the existing NODE_OPTIONS.

npm_config_update_notifier: 'false',
npm_config_fund: 'false',
NODE_OPTIONS: '--use-openssl-ca',
NODE_OPTIONS: getNodeOptions(penv.NODE_OPTIONS),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is wrong, because this it written to file as: export NODE_OPTIONS=${NODE_OPTIONS---use-openssl-ca}

So i if renovate is setting NODE_OPTIONS it will discard our --use-openssl-ca option.
I'll refactor this, so it's added as node arg directly if the node version supports it.

content += `export ${key}=\${${key}-${value}}\n`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think this needs to be reverted, so we still set it if node options are not already set.

this is the reason why it's in node options and not a cli arg

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.

I've raised #6071 to document the existing behaviour

Through a new test, I seem to see that this is correctly loading in the --use-openssl-ca?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is still not the expected behavior.

sample:

on full docker image we run install-tool node.
if we now pass NODE_OPTIONS there, then they will be persisted.

so when renovate later runs with no NODE_OPTIONS, then it will use the unexpected persisted options.

jamietanna and others added 2 commits March 2, 2026 15:07
As noted in renovatebot/renovate#40942,
we need to introduce a means to control the `NODE_OPTIONS` environment
variable to specify the memory limits for a given Node child process.

Before we do this, Containerbase needs to append the `--use-openssl-ca`
option, instead of replacing the existing `NODE_OPTIONS`.

Co-authored-by: Claude Sonnet 4.5 <jamie.tanna+github-copilot@mend.io>
Copy link
Copy Markdown
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

it still doesn't work as we expect

npm_config_update_notifier: 'false',
npm_config_fund: 'false',
NODE_OPTIONS: '--use-openssl-ca',
NODE_OPTIONS: getNodeOptions(penv.NODE_OPTIONS),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is still not the expected behavior.

sample:

on full docker image we run install-tool node.
if we now pass NODE_OPTIONS there, then they will be persisted.

so when renovate later runs with no NODE_OPTIONS, then it will use the unexpected persisted options.

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.

2 participants