8000 Specify `runAsNonRoot` in `daemon-set` manifests by sigv · Pull Request #3925 · nginx/kubernetes-ingress · GitHub
[go: up one dir, main page]

Skip to content

Conversation

sigv
Copy link
Contributor
@sigv sigv commented May 19, 2023

Proposed changes

This is a no-op change. It aligns the daemon-set manifests to match the deployment manifests. Both of these currently specify an explicit user ID to run as, therefore the container is guaranteed to be run as non-root.

This runAsNonRoot: true instruction would come in as important if the chart no longer specifies runAsUser, and someone is packaging their own image without a USER directive in the Dockerfile. Removing the runAsUser parameter could be useful as to allow OpenShift to override the UID, in a later change.

This PR aligns the deployments/daemon-set/ files to match all following related files:

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@sigv sigv requested a review from a team as a code owner May 19, 2023 16:49
@sigv sigv mentioned this pull request May 19, 2023
6 tasks
@sigv sigv force-pushed the daemonsetRunAsNonRoot branch 2 times, most recently from e99ad53 to a46303e Compare May 22, 2023 14:46
Copy link
@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Thanks @sigv !

@codecov
Copy link
codecov bot commented May 23, 2023

Codecov Report

Merging #3925 (d5670d9) into main (a580c91) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3925      +/-   ##
==========================================
- Coverage   51.82%   51.79%   -0.04%     
==========================================
  Files          59       59              
  Lines       16697    16697              
==========================================
- Hits         8654     8648       -6     
- Misses       7745     7749       +4     
- Partials      298      300       +2     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sigv sigv force-pushed the daemonsetRunAsNonRoot branch 6 times, most recently from a798d23 to bd617ab Compare May 26, 2023 08:35
This is a no-op change. It aligns the `daemon-set` manifests to match
the `deployment` manifests. Both of these currently specify an explicit
user ID to run as, therefore the container is guaranteed to be run
as non-root.

This `runAsNonRoot: true` instruction would come in as important if
the chart no longer specifies `runAsUser`, and someone is packaging
their own image without a USER directive in the Dockerfile.
Removing the `runAsUser` parameter could be useful as to allow
OpenShift to override the UID, in a later change.
@sigv sigv force-pushed the daemonsetRunAsNonRoot branch from bd617ab to d5670d9 Compare May 26, 2023 18:20
@sigv
Copy link
Contributor Author
sigv commented May 30, 2023

@lucacome, would it be possible to have a second set of eyes on this PR, so it's not stuck with a partial review? 👀🤞🏻

@lucacome lucacome requested a review from a team May 30, 2023 17:30
@lucacome lucacome merged commit 2b6aa76 into nginx:main May 30, 2023
@lucacome lucacome self-assigned this May 30, 2023
@lucacome lucacome added the enhancement Pull requests for new features/feature enhancements label May 30, 2023
@lucacome lucacome added this to the v3.2.0 milestone May 30, 2023
@sigv sigv deleted the daemonsetRunAsNonRoot branch May 30, 2023 18:39
@sigv
Copy link
Contributor Author
sigv commented May 30, 2023

Thank you! 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0