Skip to content

feat(chart): Improved default security context#7279

Merged
jonathan-innis merged 1 commit intoaws:mainfrom
stevehipwell:chart-security-context
Apr 17, 2025
Merged

feat(chart): Improved default security context#7279
jonathan-innis merged 1 commit intoaws:mainfrom
stevehipwell:chart-security-context

Conversation

@stevehipwell
Copy link
Copy Markdown
Contributor

Fixes #N/A

Description
This PR improves the default security context posture and adds support for setting container values which should be user defined.

How was this change tested?
The Helm chart was templated with the new values.

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@stevehipwell stevehipwell requested a review from a team as a code owner October 25, 2024 15:53
@stevehipwell stevehipwell requested a review from tzneal October 25, 2024 15:53
@netlify
Copy link
Copy Markdown

netlify bot commented Oct 25, 2024

Deploy Preview for karpenter-docs-prod ready!

Name Link
🔨 Latest commit b24b786
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/6800d394d8940a0008b4cb5c
😎 Deploy Preview https://deploy-preview-7279--karpenter-docs-prod.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@stevehipwell
Copy link
Copy Markdown
Contributor Author

CC @jonathan-innis

@stevehipwell stevehipwell force-pushed the chart-security-context branch from e211a98 to 0e90ebf Compare November 4, 2024 15:42
@stevehipwell stevehipwell force-pushed the chart-security-context branch from 0e90ebf to dacfc30 Compare November 18, 2024 09:47
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@tzneal could you take a look at this please?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 2, 2024

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis @tzneal could someone please take a look at this?

@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis @tzneal could someone please take a look at this?

@saurav-agarwalla
Copy link
Copy Markdown
Contributor

I'll take a look at this.

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

@stevehipwell
Copy link
Copy Markdown
Contributor Author

/not-stale

@stevehipwell
Copy link
Copy Markdown
Contributor Author

@saurav-agarwalla did you manage to take a look at this?

@saurav-agarwalla
Copy link
Copy Markdown
Contributor

@stevehipwell apologies for the delay, I started reviewing this but then got side-tracked with the holidays and other things. Planning to get back to it this week.

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 17, 2025

Pull Request Test Coverage Report for Build 14513302984

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 66.444%

Totals Coverage Status
Change from base Build 14504624666: 0.02%
Covered Lines: 6768
Relevant Lines: 10186

💛 - Coveralls

@stevehipwell stevehipwell force-pushed the chart-security-context branch 4 times, most recently from a81a0d6 to 35471b8 Compare January 20, 2025 11:12
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@saurav-agarwalla I think there is some confusion over what a first party Helm chart should be. A first party Helm chart should be opinionated and follow both industry and project best practices. As it owns the constraints it doesn't need to open everything up for customization.

Karpenter's constraints are that it runs on Linux, as a non-root user, and doesn't need any additional OS permissions; therefore there is no valid reason to allow an end-user to lower this security posture.

CC @jonathan-innis @tzneal @bwagner5

@jonathan-innis
Copy link
Copy Markdown
Contributor

Hey @stevehipwell -- sorry that we've been slow on this -- I just took a look, most of the changes look reasonable -- just a few questions about potential breaking changes and why you need certain levels of customization

@stevehipwell stevehipwell force-pushed the chart-security-context branch from 35471b8 to 934ae1b Compare February 7, 2025 14:20
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis I've replied to your comments and rebased.

@stevehipwell stevehipwell force-pushed the chart-security-context branch 2 times, most recently from b66b744 to 86b5dd8 Compare February 14, 2025 10:23
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis I've reverted the pod non-root default.

@stevehipwell stevehipwell force-pushed the chart-security-context branch from 86b5dd8 to 6d4e9fa Compare March 5, 2025 10:33
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis I've rebased this PR again and it'd be great to get it merged.

@stevehipwell stevehipwell force-pushed the chart-security-context branch from 6d4e9fa to 439bdf8 Compare March 13, 2025 10:29
@stevehipwell
Copy link
Copy Markdown
Contributor Author

👀 @jonathan-innis

@jonathan-innis
Copy link
Copy Markdown
Contributor

I think that's my last question -- everything else looks good to ship -- just if there is a strong reason from removing RuntimeDefault

@stevehipwell stevehipwell force-pushed the chart-security-context branch 2 times, most recently from 6d4e9fa to fa974a6 Compare April 10, 2025 15:54
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@stevehipwell stevehipwell force-pushed the chart-security-context branch from fa974a6 to b24b786 Compare April 17, 2025 10:10
@stevehipwell
Copy link
Copy Markdown
Contributor Author

@jonathan-innis could we get this merged before the release for v1.4.0?

@jonathan-innis
Copy link
Copy Markdown
Contributor

could we get this merged before the release for v1.4.0

I think we're going to have to punt this into v1.5.0 at this point -- we've already staged out the changes for v1.4.0. I wouldn't expect us to have a huge lead time to the next minor version, though -- we've got a number of perf things that we'd like to get through and release soon-ish

Copy link
Copy Markdown
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@jonathan-innis jonathan-innis enabled auto-merge (squash) April 17, 2025 16:23
@jonathan-innis jonathan-innis merged commit 444ff5a into aws:main Apr 17, 2025
18 checks passed
@stevehipwell stevehipwell deleted the chart-security-context branch April 17, 2025 16:48
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