fix: filter job container options with an allow list #83
viceice
Kwonunn
Labels
No labels
Compat/Breaking
Kind/Bug
Kind
Chore
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
4 participants
No due date set.
Dependencies
No dependencies set.
Reference
forgejo/act!83
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "earl-warren/act:wip-pid"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The workflow can only contain the following options for a container:
Note to reviewers: there are three commits to facilitate review. The first two are refactor and change nothing functionally. The third one is where the change happen.
cascading-pr updated at forgejo/runner#399
715ded44915c99e63e9afix: make --pids in container options a noopto fix: make --pid in container options a noop5c99e63e9aee45496cf7New commits pushed, approval review dismissed automatically according to repository settings
cascading-pr updated at forgejo/runner#399
🤔 pr title and content discrepancy
Should not be merged as is
ee45496cf7647f32b6a1fix: make --pid in container options a noopto WIP: fix: make --pid in container options a noop647f32b6a125bb41a0a3cascading-pr updated at forgejo/runner#399
@viceice sorry about that, marked as WIP now.
#84 is ready for review though 😇
25bb41a0a392d04f24f1cascading-pr updated at forgejo/runner#399
WIP: fix: make --pid in container options a noopto WIP: fix: filter job container options with an allow list92d04f24f105b15cfe1205b15cfe12ada5210a84WIP: fix: filter job container options with an allow listto fix: filter job container options with an allow listcascading-pr updated at forgejo/runner#399
Note to reviewers: there are three commits to facilitate review. The first two are refactor and change nothing functionally. The third one is where the change happen.
@ -250,0 +263,4 @@Volumes: map[string]struct{}{"somevolume": {}},},hostConfig: &container.HostConfig{Binds: []string{"/foo:/bar", "/frob:/nitz"},those are filtered on a later stage if not allowed by runner config?
Yes, in
sanitizeConfigi'm confused about one bit of code, otherwise it looks good to me ^-^
@ -409,3 +397,3 @@return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.Options, err)return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.ConfigOptions, err)}hostConfig.Binds = bindsi don't really understand what's going in from L390 to L402.
first,
hostConfig.Bindsis assigned to, then that value is also assigned tobinds, and thenbindsis assigned tohostConfig.Bindsagain?Binds, Mounts and NetworkMode are exceptions to the merge. binds save the value before mergo.Merge which has a side effect potentially modifying it and then restores the original value. Same for the other two.
Does that make sense?
got it, i think this is good to merge then
ada5210a84928120bf6acascading-pr updated at forgejo/runner#399