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

fix: uid instead of named user (fixes #2746) #3108

Merged
merged 1 commit into from
Jun 6, 2020

Conversation

t-v
Copy link
Contributor

@t-v t-v commented Feb 11, 2020

When specifying:

containerSecurityContext:
  runAsNonRoot: true
  capabilities:
    drop:
      - all

You keep getting the error:

    state:
      waiting:
        message: container has runAsNonRoot and image has non-numeric user (argocd),
          cannot verify user is non-root
        reason: CreateContainerConfigError

And since we are creating user 999 inside the container, it is better to use this user by UID than by username.

issue: #2746

Checklist:

  • this is a bug fix
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Optional. My organization is added to the README.
  • I've signed the CLA and my build is green (troubleshooting builds).

@t-v t-v changed the title use uid instead of named user to fix #2746 uid instead of named user (fixes #2746) Feb 11, 2020
@t-v t-v changed the title uid instead of named user (fixes #2746) fix: uid instead of named user (fixes #2746) Feb 11, 2020
@codecov
Copy link

codecov bot commented Feb 11, 2020

Codecov Report

Merging #3108 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3108   +/-   ##
======================================
  Coverage    38.6%   38.6%           
======================================
  Files         168     168           
  Lines       18269   18269           
  Branches      237     237           
======================================
  Hits         7053    7053           
  Misses      10342   10342           
  Partials      874     874

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea57d15...cc3bc93. Read the comment docs.

@jannfis
Copy link
Member

jannfis commented Feb 17, 2020

Hm, I'm wondering - would specifying runAsUser and runAsGroup in the pod's security context have the same effect?

@t-v
Copy link
Contributor Author

t-v commented Mar 3, 2020

it would, but wouldn't it be better support runAsNonRoot by default?
I currently run it with the runAsUser and runAsGroup option, so it works, but I think if it would be cleaner if you didn't need to specify it.

@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@jannfis jannfis left a comment

Choose a reason for hiding this comment

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

LGTM - Thank you for your submission and I'm very sorry for the long delay @t-v

@alexmt Do you have any objections to this change?

@jannfis jannfis requested a review from alexmt May 1, 2020 11:04
@alex1989hu
Copy link

I would be happy to see it merged (also met this error message). Do you need any assistance?

@jannfis
Copy link
Member

jannfis commented Jun 6, 2020

Slipped through - sorry. Merging now :)

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.

4 participants