-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat(executor): Support accessing output parameters by PNS executor running as non-root #5564
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5564 +/- ##
==========================================
+ Coverage 47.00% 47.08% +0.07%
==========================================
Files 240 240
Lines 15002 15002
==========================================
+ Hits 7052 7064 +12
+ Misses 7053 7039 -14
- Partials 897 899 +2
Continue to review full report at Codecov.
|
970763f
to
84b2fb9
Compare
84b2fb9
to
9d98c38
Compare
9d98c38
to
b506435
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a powerful PR
4d3d2e1
to
b6cc892
Compare
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
b6cc892
to
7a05cca
Compare
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
@@ -23,6 +23,15 @@ func (s *RunAsNonRootSuite) TestRunAsNonRootWorkflow() { | |||
WaitForWorkflow(fixtures.ToBeSucceeded) | |||
} | |||
|
|||
func (s *RunAsNonRootSuite) TestRunAsNonRootWithOutputParams() { | |||
s.Need(fixtures.None(fixtures.Docker, fixtures.K8SAPI, fixtures.Kubelet)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor - do you want to include emissary
in your test? instead maybe just
s.Need(fixtures.PNS)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test passes under emissary, and it is a valid use case (I can attest to that!) so I think we want to keep it for emissary to ensure there are no regressions in the future. That is unless you have reasons to explicitly not support this use case for emissary. Please let me know if that's the case and I will drop it for emissary.
@@ -0,0 +1,25 @@ | |||
apiVersion: argoproj.io/v1alpha1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please append -pipeline.yaml
to name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: Vlad Losev <vladimir.losev@sage.com>
The Argo PNS executor requires the SYS_CHROOT and the SYS_PTRACE capabilities for access to the workflow output parameters when they are saved inside the image. When it is run as a root, those capabilities are available to it immediately because they are added by the workflow controller to the container's security context. But when it's run as a non-root user, the capabilities are available in the process's inherited capability set but neither in its effective nor permitted sets. Thus, a chroot attempt in such mode currently fails. This PR adds the requisite capability bits to the
argoexec
binary in the container, which allows the executor process to gain these capabilities when launched, and use those syscalls successfully.Checklist: