actool: allow patch-manifest to override caps isolators #638
Conversation
|
/cc @alban as per private discussion This is the same behavior that patch-manifest has for patching seccomp isolators. |
| return err | ||
| } | ||
| app.Isolators = append(app.Isolators, *isolator) | ||
| // Override existing capabilities isolators |
There was a problem hiding this comment.
Comment doesn't belong here
lucab
force-pushed
the
to-upstream/actool-patch-caps
branch
from
16d4dfc
to
7b258ea
Compare
|
Rebased |
|
@lucab it looks good. As a smoke test, did you run the rkt tests (TestCaps*) with that patch? |
|
@alban not yet, but I can give it a run right now. |
|
@alban tested locally, no regressions spotted. |
| if err != nil { | ||
| return fmt.Errorf("cannot parse capability retain set %q: %v", patchCaps, err) | ||
| } | ||
| capsIsolator, err = caps.AsIsolator() |
There was a problem hiding this comment.
This is not doing what you want. On line 217 you're shadowing the error on line 214. So it's never going to get correctly assigned here.
There was a problem hiding this comment.
Change 213 to
var capsAsIsolator types.AsIsolator
Then line 217 and 225 can change to just assignments, and you can call AsIsolator() outside of the branches.
There was a problem hiding this comment.
You are painfully right on that, I didn't notice the variable shadowing going on. Fixed as suggested.
This commit unifies patch-manifest behavior so that --capability and --revoke-capability are able to override existing isolators. It will also remove one possible way of generating invalid manifests with multiple conflicting capabilitiess isolators.
lucab
force-pushed
the
to-upstream/actool-patch-caps
branch
from
7b258ea
to
b9f7648
Compare
|
LGTM |
This commit unifies patch-manifest behavior so that --capability
and --revoke-capability are able to override existing isolators.
It will also remove one possible way of generating invalid
manifests with multiple conflicting capabilitiess isolators.