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.

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

Introduction of new environment variables for HOST and PORT add non-overridable regression to kubernetes deployments #1138

Closed
2 tasks done
marcules opened this issue Jan 7, 2022 · 1 comment · Fixed by #1141
Labels
bug Something isn't working

Comments

@marcules
Copy link
Contributor

marcules commented Jan 7, 2022

⚠️ Please verify that this bug has NOT been raised before.

  • I checked and didn't find similar issue

🛡️ Security Policy

Description

The introduction of UPTIME_KUMA_PORT introduces a regression in kubernetes deployments, if the deployment is called "uptime-kuma" kubernetes will auto-generate the environment variable UPTIME_KUMA_PORT with a non-parsable value for parseInt()

image

The commit that introduces environment variable parsing is here:
f75c9e4#diff-8cf1ffe3788af127768748703e2f15dcfb3a05ffd20b4c2375223637e3260938R75

Similar issue reported here:

Similar issue in helm repository:
dirsigler/uptime-kuma-helm#26 (has the name of the deployment set to "uptime-kuma", see kubelet Created container uptime-kuma)

Similar issue with redis:
kubernetes/kubernetes#60099 (comment)

JavaScript will only "skip" the values, if they are not set with the current implementation, but produces a NaN value, if one of the values inside the else-shorthand is set.

image

As a proposal, please either change the order in which the parameters are parsed (so it can be overridden in the deployment manifest in kubernetes) or implement it with validation, e.g. like this:

const port = [process.env.UPTIME_KUMA_PORT, process.env.PORT, args.port, 3001].map(el => parseInt(el)).find(el => !isNaN(el));

image

👟 Reproduction steps

  • Create an environment variable named UPTIME_KUMA_PORT with a non-integer value
  • Start server

👀 Expected behavior

Uptime Kuma will fall back to the next parsable integer value in the priority list
(UPTIME_KUMA_PORT -> PORT -> ARGS.PORT -> 3001)

😓 Actual Behavior

Uptime Kuma crashes with the port being NaN

🐻 Uptime-Kuma Version

1.11.1-alpine

💻 Operating System and Arch

node:14-alpine3.12

🌐 Browser

none

🐋 Docker Version

k8s

🟩 NodeJS Version

No response

📝 Relevant log output

Load JWT secret from database.
No user, need setup
Adding route
Adding socket handler
Init the server
Trace: RangeError [ERR_SOCKET_BAD_PORT]: options.port should be >= 0 and < 65536. Received NaN.
    at new NodeError (internal/errors.js:322:7)
    at validatePort (internal/validators.js:216:11)
    at Server.listen (net.js:1457:5)
    at /app/server/server.js:1336:12 {
  code: 'ERR_SOCKET_BAD_PORT'
}
    at process.<anonymous> (/app/server/server.js:1542:13)
    at process.emit (events.js:400:28)
    at processPromiseRejections (internal/process/promises.js:245:33)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)
If you keep encountering errors, please report to https://github.com/louislam/uptime-kuma/issues
Shutdown requested
Called signal: SIGTERM
Stopping all monitors
Trace: Error [ERR_SERVER_NOT_RUNNING]: Server is not running.
    at new NodeError (internal/errors.js:322:7)
    at Server.close (net.js:1624:12)
    at Object.onceWrapper (events.js:519:28)
    at Server.emit (events.js:412:35)
    at emitCloseNT (net.js:1677:8)
    at processTicksAndRejections (internal/process/task_queues.js:81:21) {
  code: 'ERR_SERVER_NOT_RUNNING'
}
    at process.<anonymous> (/app/server/server.js:1542:13)
    at process.emit (events.js:400:28)
    at processPromiseRejections (internal/process/promises.js:245:33)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)
If you keep encountering errors, please report to https://github.com/louislam/uptime-kuma/issues
Closing the database
SQLite closed
Graceful shutdown successful!
@chakflying
Copy link
Collaborator

Your fix for avoiding NaN looks good to me! Would you like to create a PR for this?

@marcules marcules mentioned this issue Jan 7, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants