Add securityContext for individual containers in flyte-binary Deployment #6168
Conversation
Signed-off-by: marrrcin <m.zablo@gmail.com>
Code Review Agent Run #3fc36aActionable Suggestions - 1
Review Details
AI Code Review powered by |
Changelist by BitoThis pull request implements the following key changes.
|
| @@ -300,6 +304,8 @@ deployment: | |||
| # extraPodSpec Specify additional configuration for Flyte pod | |||
| # This can be used for adding affinity, tolerations, hostNetwork, etc. | |||
| extraPodSpec: {} | |||
| # securityContext Specify security context for Flyte container | |||
| securityContext: {} | |||
There was a problem hiding this comment.
Consider adding default security context settings for the wait-for-db init container to follow security best practices. The empty security context {} may not provide adequate security controls.
Code suggestion
Check the AI-generated fix before applying
| securityContext: {} | |
| securityContext: | |
| runAsNonRoot: true | |
| allowPrivilegeEscalation: false | |
| readOnlyRootFilesystem: true |
Code Review Run #3fc36a
Is this a valid issue, or was it incorrectly flagged by the Agent?
- it was incorrectly flagged
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6168 +/- ##
=======================================
Coverage 37.04% 37.04%
=======================================
Files 1318 1318
Lines 132583 132583
=======================================
+ Hits 49109 49115 +6
+ Misses 79223 79217 -6
Partials 4251 4251
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Why are the changes needed?
Some k8s clusters (including the one I work with) have strict requirements when it comes to
securityContextfor pods and containers, enforced by tools likeGatekeeper.sh.These changes introduce the ability to configure
securityContextat container level (init containers and the main container) withinflyte-binaryHelm Chart.Specifying a
securityContextis a Kubernetes best practice, as it allows you to configure various security-related settings (e.g., running as a non-root user, setting read-only file systems, etc.). This helps ensure that deployments adhere to stricter security guidelines and can comply with organizational or regulatory requirements.What changes were proposed in this pull request?
securityContextfields under:deployment.waitForDB.securityContextfor thewait-for-dbinit container.deployment.genAdminAuthSecret.securityContextfor thegen-admin-auth-secretinit container.deployment.securityContextfor the main Flyte container.deployment.yaml) to conditionally render thesesecurityContextblocks if they are defined invalues.yaml.These changes allow end users to specify privileged or non-privileged modes, user/group IDs, and other security settings in a flexible manner.
How was this patch tested?
securityContextconfigurations to ensure that the containers correctly applied the specified context (e.g., run as non-root, set read-only file system).securityContextinvalues.yamlfalls back to the existing default behavior, ensuring backward compatibility.No automated tests were added specifically for this change because it primarily involves YAML configuration and conditional Helm templating. However, the local deployment tests validated that the feature works as expected and does not introduce regressions.
Labels
securityContext)Setup process
No special setup is required. Users can opt in to the new functionality by defining the relevant
securityContextfields in theirvalues.yaml.Screenshots
N/A – the feature pertains to YAML/config changes and does not alter UI/visual output.
Check all the applicable boxes
Related PRs
N/A
Docs link
N/A
Summary by Bito
This PR enhances the security configuration capabilities of the flyte-binary Helm chart by introducing container-level securityContext settings. The implementation enables individual security context configuration for multiple containers including wait-for-db init container, gen-admin-auth-secret init container, and the main Flyte container. The changes include conditional rendering of securityContext blocks and updated manifest files with new haSharedSecret and checksum values.Unit tests added: False
Estimated effort to review (1-5, lower is better): 2