-
Notifications
You must be signed in to change notification settings - Fork 233
infra/image/shdefaults: Add SYS_PTRACE to CAP_DEFAULTS #1351
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
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new Sequence diagram for starting the container with debugging enabledsequenceDiagram
participant S as start.sh
participant C as container_create
participant container_start
participant container_wait_for_journald
participant container_wait_up
S->>S: Parses arguments, including -d
alt -d is provided
S->>C: container_create with SYS_PTRACE capability
else -d is not provided
S->>C: container_create without SYS_PTRACE capability
end
C-->>S: Container created
S->>container_start: container_start
container_start-->>S: Container started
S->>container_wait_for_journald: container_wait_for_journald
container_wait_for_journald-->>S: Journald ready
S->>container_wait_up: container_wait_up
container_wait_up-->>S: Container is up
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @t-woerner - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using more descriptive variable names than single letters in
getopts
. - It might be helpful to add a comment explaining why the SYS_PTRACE capability is required for debugging.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
IMHO, this could be the default behavior. LGTM. |
f843b5a
to
8d688a2
Compare
c61acbc
to
66be7c2
Compare
Debugging is now enabled by default in the containers that are generated with container_create. "+SYS_PTRACE" has been added to CAP_DEFAULTS in shdefaults for this.
66be7c2
to
4323765
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.
LGTM
readarray expects to get an item per line to be added to the array. Printing one item per line with printf fixes this to get the proper formatting for "${CAP_DEFAULTS[@]}" as a valid input for readarray.
2a62826
to
638422e
Compare
Debugging is now enabled by default in the containers that are generated with container_create. "+SYS_PTRACE" has been added to CAP_DEFAULTS in shdefaults for this.