Skip to content

feat(library) -> add nginx 1.31#280

Open
vTusharr wants to merge 2 commits into
unikraft:mainfrom
vTusharr:add_nginx-1.31
Open

feat(library) -> add nginx 1.31#280
vTusharr wants to merge 2 commits into
unikraft:mainfrom
vTusharr:add_nginx-1.31

Conversation

@vTusharr

@vTusharr vTusharr commented Jun 2, 2026

Copy link
Copy Markdown

Adds an nginx 1.31.0 image based on alpine as a base , modelled on the existing 1.25 image.
circumventing CVE which is present in earlier version

Signed-off-by: Tushar Verma <tusharVermaiota@proton.me>
@razvand razvand requested a review from razvanvirtan June 12, 2026 16:16
@razvand razvand self-assigned this Jun 12, 2026
@razvand razvand added the enhancement New feature or request label Jun 12, 2026
@razvand razvand requested a review from Copilot June 12, 2026 16:16

Copilot AI 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.

Pull request overview

Adds a new Unikraft library image definition for unikraft.org/nginx:1.31, intended to provide an updated Nginx base (Alpine-based) and address the CVE mentioned in the PR description.

Changes:

  • Introduces a new library/nginx/1.31 image definition (Kraftfile + scratch rootfs Dockerfile).
  • Adds a custom Nginx configuration (single-process, tokens off, gzip, basic security headers).
  • Adds the default Nginx welcome page and a versioned README for running the image.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
library/nginx/1.31/wwwroot/index.html Adds the default Nginx welcome page content.
library/nginx/1.31/README.md Documents how to run/query the new unikraft.org/nginx:1.31 image.
library/nginx/1.31/Kraftfile Defines the Unikraft build configuration and lwIP settings for Nginx 1.31.
library/nginx/1.31/Dockerfile Builds a minimal scratch rootfs by copying Nginx + runtime deps from nginx:1.31.0-alpine.
library/nginx/1.31/conf/nginx.conf Adds the Nginx runtime configuration for the image.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread library/nginx/1.31/conf/nginx.conf Outdated
user root root;


worker_rlimit_nofile 1024;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@vTusharr What is the reasoning for adding this limit, is it something enforced by the new version of nginx?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah no , its not required by this version of nginx .
The limit came from a common nginx tuning guideline , most production configs recommend setting it to at least worker_processes * worker_connections * 2 + 100 fd tho libposix_fdtab_maxfds have it default @ 1024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So i think we can either drop this [but i am not sure how will runtime will react when it runs out of fdtabs ]
or like add a comment to kraftfile indicating for libposix_fdtab_maxfds so one can change it to their needs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it is fine to drop this in the standard config. If I understand that config option correctly, with no explicit value set, the process will only be limited by the open fds limit on the OS, which is something to be configured outside of the nginx config.

Comment thread library/nginx/1.31/README.md
Comment thread library/nginx/1.31/README.md Outdated

@razvanvirtan razvanvirtan 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.

Still waiting for WASP permissions to start the testing. @vTusharr In the meantime, please fix the issues mentioned by Copilot. Also, see my follow-up comment.

Comment thread library/nginx/1.31/conf/nginx.conf Outdated
user root root;


worker_rlimit_nofile 1024;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@vTusharr What is the reasoning for adding this limit, is it something enforced by the new version of nginx?

@vTusharr vTusharr force-pushed the add_nginx-1.31 branch 2 times, most recently from cd969f7 to 5eff2f7 Compare June 21, 2026 15:18
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Tushar <104261506+vTusharr@users.noreply.github.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

COPY --from=build /etc/group /etc/group
COPY --from=build /var/log/nginx /var/log/nginx
COPY --from=build /var/cache/nginx /var/cache/nginx
COPY --from=build /run /run

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think thats debian base specific , @razvanvirtan ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like it works even without that. Still, one last question before merging: why using the alpine image instead of the bookworm one? If no significant reason, I would switch to bookworm, to keep this uniform with the nginx 1.25 port

@vTusharr vTusharr Jun 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah ok so first i had tried the official image which now no longer ships with bookworm but with trixie which i think have updated glibc and was crashing so it led me to musl based alphine
One thing we can do is use bookworm as a base and build the binary then use that .

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should be good with alpine as well, just wanted to check we have a reason for it 👍

Comment on lines +137 to +138
CONFIG_LWIP_NUM_TCPCON: 1024
CONFIG_LWIP_NUM_TCPLISTENERS: 64

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

valid

@razvanvirtan razvanvirtan 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.

Thanks @vTusharr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants