feat(common): add kube-apiserver networkpolicy helper#1413
Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughNew Helm template functions have been added under the charts/common/templates directory. One function determines the network policy type based on the value of Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant NPType as "common.networkPolicy.type"
participant API as "Cilium API Check"
Caller->>NPType: Invoke with .Values.global.networkPolicy.type
alt value == "auto"
NPType->>API: Check for cilium.io/v2/CiliumNetworkPolicy
API-->>NPType: API available?
alt API available
NPType-->>Caller: Return "cilium"
else API unavailable
NPType-->>Caller: Return "native"
end
else
NPType-->>Caller: Return specified type
end
sequenceDiagram
participant Caller
participant IdentityFunc as "common.networkPolicy.identity.kube-apiserver"
participant KubeAPI as "Kube API Check"
Caller->>IdentityFunc: Request identity list
IdentityFunc->>KubeAPI: Check for konnectivity-agent DaemonSet in kube-system
alt DaemonSet exists
KubeAPI-->>IdentityFunc: DaemonSet found
IdentityFunc-->>Caller: Return identity with konnectivity-agent details
else
KubeAPI-->>IdentityFunc: DaemonSet not found
IdentityFunc-->>Caller: Return identity with kube-apiserver details
end
sequenceDiagram
participant Caller
participant RuleFunc as "common.networkPolicy.rule.from.kube-apiserver"
participant Identity as "Identity Retrieval"
Caller->>RuleFunc: Invoke with ports and parameters
RuleFunc->>Identity: Retrieve identities
Identity-->>RuleFunc: Return identities list
alt Using Cilium
RuleFunc->>RuleFunc: Construct rule using entity details
else Using native Kubernetes
RuleFunc->>RuleFunc: Construct rule using namespace and pod selectors
end
RuleFunc-->>Caller: Output YAML network policy rules
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/common/templates/_networkpolicy.tpl (1)
1-26: Good approach for identifying kube-apiserver endpoints.The function correctly handles different cluster configurations by checking for the konnectivity-agent DaemonSet and falling back to direct kube-apiserver identification if not present. This provides good compatibility across cluster setups.
Consider adding a way to override these identities through values for clusters with custom configurations. This would provide additional flexibility for users with non-standard setups.
{{- define "common.networkPolicy.identity.kube-apiserver" -}} {{- $identities := list -}} + {{- if .Values.global.networkPolicy.kubeApiserver.overrideIdentities -}} + {{- $identities = .Values.global.networkPolicy.kubeApiserver.overrideIdentities -}} + {{- else -}} {{- $kubeSystemNamespace := "kube-system" -}} {{- $konnectivityName := "konnectivity-agent" -}} {{- if lookup "apps/v1" "DaemonSet" $kubeSystemNamespace $konnectivityName -}} {{- $identities = append $identities (dict "endpoint" (dict "namespace" $kubeSystemNamespace "pod" (dict "k8s-app" $konnectivityName ) "serviceAccount" $konnectivityName )) -}} {{- else -}} {{- $identities = append $identities (dict "endpoint" (dict "namespace" $kubeSystemNamespace "pod" (dict "tier" "control-plane" "component" "kube-apiserver" ) )) -}} {{- $identities = append $identities (dict "entity" "kube-apiserver") -}} {{- end -}} + {{- end -}} {{- toYaml $identities -}} {{- end -}}charts/common/templates/_ciliumnetworkpolicy.tpl (1)
2-49: Well-implemented network policy rule generator with good support for both Cilium and native policies.The template handles the complexity of creating appropriate network policy rules for both Cilium and native Kubernetes formats. It properly processes input ports, supports both entity-based and endpoint-based rules for Cilium, and constructs appropriate namespace and pod selectors for native Kubernetes network policies.
The code could benefit from some internal comments explaining the different approaches for Cilium vs. native network policies, especially for maintainers who might not be familiar with both formats.
{{- define "common.networkPolicy.rule.from.kube-apiserver" -}} {{- $useCilium := eq .cilium true -}} {{- $identities := include "common.networkPolicy.identity.kube-apiserver" (dict) | fromYamlArray -}} {{- $rules := list -}} {{- $ports := list -}} + {{- /* Process ports from input, handling both string and list protocols */ -}} {{- if hasKey . "ports" -}} {{- range $port, $protocols := (.ports | default (list)) -}} {{- $_protocols := $protocols -}} {{- if kindIs "string" $protocols -}} {{- $_protocols = list $protocols -}} {{- end -}} {{- range $protocol := $_protocols -}} {{- $ports = append $ports (dict "port" $port "protocol" $protocol) -}} {{- end }} {{- end -}} {{- end -}} + {{- /* Create rules for each identity based on network policy type */ -}} {{- range $identity := $identities -}} {{- $rule := dict -}} {{- $endpoint := $identity.endpoint -}} {{- if $useCilium -}} + {{- /* For Cilium: use entity-based or endpoint-based rules */ -}} {{- if hasKey $identity "entity" -}} {{- $rule = dict "fromEntities" (list $identity.entity) -}} {{- else -}} {{- $matchLabels := dict "io.kubernetes.pod.namespace" $endpoint.namespace -}} {{- if hasKey $endpoint "serviceAccount" -}} {{- $matchLabels = set $matchLabels "io.cilium.k8s.policy.serviceaccount" $endpoint.serviceAccount -}} {{- else -}} {{- $matchLabels = mustMerge $matchLabels $endpoint.pod -}} {{- end -}} {{- $rule = dict "fromEndpoints" (list (dict "matchLabels" $matchLabels)) -}} {{- end -}} {{- if $ports }} {{- $rule = set $rule "toPorts" (list (dict "ports" $ports)) -}} {{- end -}} {{- else -}} + {{- /* For native K8s: use namespaceSelector and podSelector */ -}} {{- $from := dict "namespaceSelector" (dict "matchLabels" (dict "kubernetes.io/metadata.name" $endpoint.namespace)) -}} {{- $from = set $from "podSelector" (dict "matchLabels" $endpoint.pod) -}} {{- $rule = set $rule "from" (list $from) -}} {{- if $ports -}} {{- $rule = set $rule "ports" $ports -}} {{- end -}} {{- end -}} {{- $rules = append $rules $rule -}} {{- end -}} {{- toYaml $rules -}} {{- end -}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/common/templates/_ciliumnetworkpolicy.tpl(1 hunks)charts/common/templates/_cni.tpl(1 hunks)charts/common/templates/_networkpolicy.tpl(1 hunks)
🔇 Additional comments (1)
charts/common/templates/_cni.tpl (1)
1-11: Well-structured auto-detection for network policy type.The template function efficiently determines whether to use Cilium or native Kubernetes network policies based on configuration and capability detection. The code follows good Helm templating practices with consistent formatting and clear conditional logic.
cwrau
force-pushed
the
feat/common/add-kube-apiserver-networkpolicy-helper
branch
from
ea1b941 to
8587b23
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/common/templates/_networkpolicyrules.tpl (1)
48-52: Consider adding error handling for empty rule sets.While the implementation is correct, consider adding a check for empty rule sets to provide clearer feedback or default behavior when no identities are found.
{{- end -}} {{- $rules = append $rules $rule -}} {{- end -}} + {{- if not $rules -}} + {{/* Return a comment or default rule when no identities are found */}} + {{- dict | toYaml -}} + {{- else -}} {{- toYaml $rules -}} + {{- end -}} {{- end -}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/common/templates/_cni.tpl(1 hunks)charts/common/templates/_networkpolicy.tpl(1 hunks)charts/common/templates/_networkpolicyrules.tpl(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/common/templates/_networkpolicy.tpl
- charts/common/templates/_cni.tpl
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint helm chart (common)
- GitHub Check: wait-for-checks
🔇 Additional comments (6)
charts/common/templates/_networkpolicyrules.tpl (6)
1-2: Well-structured Helm template function added for network policy rules.The function name clearly communicates its purpose of generating network policy rules specifically for the kube-apiserver.
7-17: Good handling of port specifications with flexible input formats.The code properly handles both single protocol and multiple protocol formats for ports, providing flexibility in how network policies can be configured.
23-37: Comprehensive Cilium-specific rule implementation.The Cilium implementation correctly handles both entity-based and endpoint-based rules with appropriate matching labels for namespaces, service accounts, and pods.
39-46: Proper native Kubernetes NetworkPolicy implementation.The native Kubernetes implementation correctly uses namespace and pod selectors to target the kube-apiserver, following Kubernetes NetworkPolicy specifications.
3-4:Details
✅ Verification successful
Ensure dependency template function exists in the codebase.
This line depends on an external template function
common.networkPolicy.identity.kube-apiserverwhich isn't defined in this file. Verify that this dependency exists in the other files mentioned in the AI summary.
🏁 Script executed:
#!/bin/bash # Check if the dependent template function exists grep -r "define \"common.networkPolicy.identity.kube-apiserver\"" charts/common/templates/Length of output: 195
Dependency Template Function Verified
The script confirms that the dependency template function
common.networkPolicy.identity.kube-apiserveris defined incharts/common/templates/_networkpolicy.tpl. No further action is required.
1-52:Details
❓ Verification inconclusive
Verify integration with global network policy configuration.
This helper function appears to be part of a larger network policy framework. Ensure it integrates correctly with the global network policy type settings mentioned in the AI summary (
.Values.global.networkPolicy.type).
🏁 Script executed:
#!/bin/bash # Check how this template integrates with global network policy settings grep -r "global.networkPolicy.type" charts/common/templates/Length of output: 229
Global Network Policy Integration Verification
I checked the repository and found that the global network policy type (
.Values.global.networkPolicy.type) is referenced only in the_cni.tplfile. In contrast, the_networkpolicyrules.tplhelper relies on the.ciliumflag and identity-based logic. Please confirm that this decoupling is intentional—that is, that this helper is invoked only in contexts where the global configuration is managed externally. If integration with the global setting is needed here as well, consider aligning the logic (or adding clarifying comments) to ensure consistency across the network policy framework.
cwrau
force-pushed
the
feat/common/add-kube-apiserver-networkpolicy-helper
branch
from
8587b23 to
96303dd
Compare
🤖 I have created a release *beep* *boop* --- ## [1.5.0](common-v1.4.0...common-v1.5.0) (2025-06-06) ### Features * **common/telemetry:** add telemetry-collector as a generic service name ([#1432](#1432)) ([fdf3b21](fdf3b21)) * **common:** add kube-apiserver networkpolicy helper ([#1413](#1413)) ([58ed072](58ed072)) ### Miscellaneous Chores * **common/dependencies:** update helm release common to v2.27.0 ([#1389](#1389)) ([9e7587f](9e7587f)) * **common:** add documentation about template usage ([#1426](#1426)) ([638add8](638add8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit