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

build: switch nginx docker base image to unprivileged version #902

Merged
merged 1 commit into from Sep 25, 2023

Conversation

aschaber1
Copy link
Contributor

@aschaber1 aschaber1 commented Sep 22, 2023

Hello,

I switched the base image from nginx (privileged) to nginx unprivileged. Also updated the nginx config and README.

Looking forward to your feedback.

@github-actions
Copy link

Copy link
Member

@acelaya acelaya left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm generally in favor of not running the image as root, but I added some questions before going forward.

EDIT: Also, please rebase when you have a minute. I merged some dependabot PRs and there seems to have some conflicts now.

config/docker/nginx.conf Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@aschaber1
Copy link
Contributor Author

Hello, thanks for looking into this. I tried to give more insights to the changes. Please let me know if there are any more questions I can help with.

I'll rebase it later this weekend.

@acelaya
Copy link
Member

acelaya commented Sep 23, 2023

Thanks @aschaber1. I'm ok going forward with this, just make sure you rebase from develop and remove the hardcoded registry names.

After that I will test the image and merge it if everything goes right.

@acelaya acelaya added this to the 4.0.0 milestone Sep 23, 2023
@aschaber1
Copy link
Contributor Author

Hello @acelaya, I just pushed the updated commit. Did the updates in terms of registry as you suggested. Let me know, if there's something else I can do :)

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3c4fa33) 86.61% compared to head (7eae695) 86.61%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #902   +/-   ##
========================================
  Coverage    86.61%   86.61%           
========================================
  Files           54       54           
  Lines         2226     2226           
  Branches       291      291           
========================================
  Hits          1928     1928           
  Misses           8        8           
  Partials       290      290           

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

@acelaya acelaya merged commit ec10a4a into shlinkio:develop Sep 25, 2023
7 checks passed
@aschaber1 aschaber1 deleted the unpriv-container branch September 25, 2023 06:57
@aschaber1
Copy link
Contributor Author

@acelaya I found this blog post from Red Hat regarding using shortnames, and how it might also break things.

https://www.redhat.com/sysadmin/container-image-short-names

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants