feat: share security contexts globally

[roderik]

Summary

• add global.securityContexts defaults for pod/container security context inheritance
• merge global + chart-level security contexts when rendering bootstrapper and node workloads
• refresh chart docs to reflect the new global configuration knobs

Testing

• bun run check
• bun run typecheck
• bun run test

[sourcery-ai]

Sorry @roderik, you have reached your weekly rate limit of 1500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[gemini-code-assist]

Summary of Changes

Hello @roderik, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the Helm chart's configuration capabilities by introducing global security contexts for Kubernetes pods and containers. This change centralizes the management of security settings, allowing users to define common security policies at a higher level that are then inherited by subcharts. It ensures that security contexts are consistently applied across different components of the deployed network, while still providing the flexibility for individual subcharts to specify their own security settings, which will be merged with the global defaults.

Highlights

• Global Security Contexts: Introduced global.securityContexts to the main Helm chart, allowing for centralized definition of pod and container security contexts.
• Security Context Inheritance: Implemented logic in subcharts (network-bootstrapper and network-nodes) to merge global security contexts with chart-specific overrides, ensuring consistent application while maintaining flexibility.
• Documentation Updates: Updated README.md files for the main chart and subcharts to reflect the new global security context configuration and inheritance behavior.
• Persistence Default Correction: Corrected a default value for persistence.enabled in the network-nodes subchart's README.md from false to true.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

To view in Slack, search for: 1758204120.092509

[gemini-code-assist]

Code Review

This pull request introduces a valuable feature by adding global security contexts, allowing for centralized management of pod and container security settings across the Helm chart. The implementation correctly merges global and local contexts, with local settings taking precedence. The documentation has also been updated to reflect these new values.

I have a few suggestions for minor code cleanup in the Helm templates to remove some redundant code. Additionally, I've noted a change to the default value of persistence.enabled in the network-nodes subchart, which seems unrelated to this PR's main goal and could be a breaking change for users. It would be good to clarify if this change is intentional.

[gemini-code-assist]

The default value for persistence.enabled has been changed from false to true. This is a significant change in the chart's default behavior and could be a breaking change for users who rely on the previous default. This change also seems unrelated to the main purpose of this pull request, which is about global security contexts. Please confirm if this change is intentional. If it is, it should be highlighted in the pull request summary as a notable change.

[gemini-code-assist]

The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map, even if its inputs are empty maps. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

      {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }}
      {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

[gemini-code-assist]

The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

  {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }}
  {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

[gemini-code-assist]

The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

  {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }}
  {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

[seer-by-sentry]

Potential bug: The arguments to the Helm merge function are reversed, causing global security contexts to incorrectly override local ones, contrary to the documented intent.

• Description: The Helm merge function gives precedence to its first argument (the destination). In the current implementation, the global security context is passed as the first argument and the local .Values.podSecurityContext is the second. This causes global settings to always override local, chart-specific settings. The intended behavior, as indicated by comments in values.yaml, is for local values to take precedence over global ones. This logical error will cause user-provided local security contexts to be silently ignored whenever a global context is also defined, leading to incorrect configurations being applied without any warning.

• Suggested fix: Reverse the order of the arguments in the merge function call. The local context, (default (dict) .Values.podSecurityContext), should be the first argument, and the global context, (deepCopy (dig "pod" $globalSecurityContexts (dict))), should be the second. This ensures local values correctly override global defaults.
  _{severity: 0.65, confidence: 0.98}

_{Did we get this right? 👍 / 👎 to inform future reviews.}