Skip to content

Conversation

@diogoasouza
Copy link
Contributor

@diogoasouza diogoasouza commented Aug 1, 2025

Issue #291

@diogoasouza diogoasouza marked this pull request as ready for review August 6, 2025 01:40
@diogoasouza diogoasouza requested a review from a team as a code owner August 6, 2025 01:40
@diogoasouza diogoasouza requested a review from a team August 7, 2025 00:25
Comment on lines 194 to 196
# capabilities:
# drop:
# - ALL
Copy link
Member

Choose a reason for hiding this comment

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

Question: can't we drop all caps?

Comment on lines 232 to 235
# capabilities:
# drop:
# - ALL
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above.

# capabilities:
# drop:
# - ALL
# privileged: false
Copy link
Member

Choose a reason for hiding this comment

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

And what about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't understand your concern here

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, the comment is about line 197 - # privileged: false, but the UI rendered it as if it was for everything above.
My question is if we can explicitly set it to privileged: false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! I changed it to explicitly set the value to false

@diogoasouza diogoasouza force-pushed the security-best-practices branch from f13cf04 to f5f6c93 Compare August 12, 2025 00:01
Copy link
Member

@macedogm macedogm left a comment

Choose a reason for hiding this comment

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

Made a suggestion on a line that I believe was missed. Additionally, can we set?

    capabilities:
      drop:
      - ALL

@diogoasouza diogoasouza force-pushed the security-best-practices branch from f5f6c93 to 40bc06d Compare August 12, 2025 22:40
@diogoasouza diogoasouza requested a review from macedogm August 12, 2025 22:40
@jbiers
Copy link
Member

jbiers commented Aug 13, 2025

As @macedogm mentioned, if possible we should drop all capabilities by default. If it disrupts the application flow, then we can at least try dropping as many capabilities as we can by default.

Otherwise LGTM

@diogoasouza diogoasouza force-pushed the security-best-practices branch from 40bc06d to 2b5519a Compare August 14, 2025 00:17
@diogoasouza
Copy link
Contributor Author

@macedogm @jbiers I updated the PR with the requested changes

@diogoasouza diogoasouza merged commit 3e6606b into rancher:main Aug 14, 2025
9 checks passed
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