generate network policy per operand #1246
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -128,11 +128,28 @@ type TempoStackSpec struct { | |||||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Observability" | ||||||
| Observability ObservabilitySpec `json:"observability,omitempty"` | ||||||
|
|
||||||
| // NetworkingSpec defines how networking configs are handled. | ||||||
| // | ||||||
| // +optional | ||||||
| // +kubebuilder:validation:Optional | ||||||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Networking" | ||||||
| Networking NetworkingSpec `json:"networking,omitempty"` | ||||||
|
|
||||||
| // +optional | ||||||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Extra Configurations" | ||||||
| ExtraConfig *ExtraConfigSpec `json:"extraConfig,omitempty"` | ||||||
| } | ||||||
|
|
||||||
| // ObservabilitySpec defines how networking configs are handled. | ||||||
|
There was a problem hiding this comment. The comment incorrectly states 'ObservabilitySpec' when it should be 'NetworkingSpec'.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||
| type NetworkingSpec struct { | ||||||
| // Enabled determines whether network policies are generated for the operands. | ||||||
| // | ||||||
| // +optional | ||||||
| // +kubebuilder:default:="true" | ||||||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Enable Networkpolicies" | ||||||
| Enabled bool `json:"enabled,omitempty"` | ||||||
| } | ||||||
|
|
||||||
| // ObservabilitySpec defines how telemetry data gets handled. | ||||||
| type ObservabilitySpec struct { | ||||||
| // Metrics defines the metrics configuration for operands. | ||||||
|
|
@@ -954,3 +971,4 @@ func (tempo *TempoStack) GetStatus() any { | |||||
| func (tempo *TempoStack) SetStatus(s any) { | ||||||
| tempo.Status = s.(TempoStackStatus) | ||||||
| } | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,171 @@ | ||||||||
| package networking | ||||||||
|
There was a problem hiding this comment. a similar comment as the previous one. I would be more specific and rename it to |
||||||||
|
|
||||||||
| import ( | ||||||||
| corev1 "k8s.io/api/core/v1" | ||||||||
| networkingv1 "k8s.io/api/networking/v1" | ||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||
| "k8s.io/apimachinery/pkg/util/intstr" | ||||||||
| "k8s.io/utils/ptr" | ||||||||
|
|
||||||||
| "github.com/grafana/tempo-operator/api/tempo/v1alpha1" | ||||||||
| "github.com/grafana/tempo-operator/internal/manifests/manifestutils" | ||||||||
| "github.com/grafana/tempo-operator/internal/manifests/naming" | ||||||||
| ) | ||||||||
|
|
||||||||
| func generatePolicyFor(tempo v1alpha1.TempoStack, componentName string) *networkingv1.NetworkPolicy { | ||||||||
| np := &networkingv1.NetworkPolicy{ | ||||||||
| ObjectMeta: metav1.ObjectMeta{ | ||||||||
| Name: naming.Name(componentName, tempo.Name), | ||||||||
| Namespace: tempo.Namespace, | ||||||||
| Labels: manifestutils.ComponentLabels(componentName, tempo.Name), | ||||||||
| }, | ||||||||
| Spec: networkingv1.NetworkPolicySpec{ | ||||||||
| PodSelector: metav1.LabelSelector{ | ||||||||
| MatchLabels: manifestutils.ComponentLabels(componentName, tempo.Name), | ||||||||
| }, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| rels := componentRelations(tempo)[componentName] | ||||||||
|
|
||||||||
| for target, ports := range rels { | ||||||||
| for _, conn := range ports { | ||||||||
| peer := policyPeerFor(target, tempo) | ||||||||
| np.Spec.Egress = append(np.Spec.Egress, networkingv1.NetworkPolicyEgressRule{ | ||||||||
| Ports: []networkingv1.NetworkPolicyPort{conn}, | ||||||||
| To: []networkingv1.NetworkPolicyPeer{peer}, | ||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| if len(np.Spec.Egress) > 0 { | ||||||||
| np.Spec.PolicyTypes = append(np.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) | ||||||||
| } | ||||||||
|
|
||||||||
| reverse := reverseRelations(componentRelations(tempo)) | ||||||||
| for source, ports := range reverse { | ||||||||
| for target, conn := range ports { | ||||||||
| if target != componentName { | ||||||||
| continue | ||||||||
| } | ||||||||
| peer := policyPeerFor(source, tempo) | ||||||||
| np.Spec.Ingress = append(np.Spec.Ingress, networkingv1.NetworkPolicyIngressRule{ | ||||||||
| Ports: conn, | ||||||||
| From: []networkingv1.NetworkPolicyPeer{peer}, | ||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| if len(np.Spec.Ingress) > 0 { | ||||||||
| np.Spec.PolicyTypes = append(np.Spec.PolicyTypes, networkingv1.PolicyTypeIngress) | ||||||||
| } | ||||||||
|
|
||||||||
| return np | ||||||||
| } | ||||||||
|
|
||||||||
| func policyPeerFor(name string, tempo v1alpha1.TempoStack) networkingv1.NetworkPolicyPeer { | ||||||||
| switch name { | ||||||||
| case netPolicys3Storage, netPolicyOtelTargets: | ||||||||
| return networkingv1.NetworkPolicyPeer{ | ||||||||
| IPBlock: &networkingv1.IPBlock{CIDR: "0.0.0.0/0"}, | ||||||||
| } | ||||||||
| case netPolicyClusterComponents: | ||||||||
| return networkingv1.NetworkPolicyPeer{ | ||||||||
| NamespaceSelector: &metav1.LabelSelector{}, | ||||||||
| } | ||||||||
| default: | ||||||||
| return networkingv1.NetworkPolicyPeer{ | ||||||||
| PodSelector: &metav1.LabelSelector{ | ||||||||
| MatchLabels: manifestutils.ComponentLabels(name, tempo.Name), | ||||||||
| }, | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| // networkRelations define connections: from -> to using NetworkPolicyPort. | ||||||||
| type networkRelations = map[string]map[string][]networkingv1.NetworkPolicyPort | ||||||||
|
|
||||||||
| const ( | ||||||||
| netPolicys3Storage = "s3" | ||||||||
| netPolicyOtelTargets = "otel" | ||||||||
| netPolicyClusterComponents = "cluster" | ||||||||
| ) | ||||||||
|
|
||||||||
| func componentRelations(tempo v1alpha1.TempoStack) networkRelations { | ||||||||
| var ( | ||||||||
| s3Conn = []networkingv1.NetworkPolicyPort{ | ||||||||
| { // TODO: get this from secret? | ||||||||
| Protocol: ptr.To(corev1.ProtocolTCP), | ||||||||
| Port: ptr.To(intstr.FromInt(443)), | ||||||||
| }, | ||||||||
| { // TODO: get this from secret? | ||||||||
| Protocol: ptr.To(corev1.ProtocolTCP), | ||||||||
| Port: ptr.To(intstr.FromInt(9000)), | ||||||||
| }, | ||||||||
|
Comment on lines
+96
to
+104
There was a problem hiding this comment. Hardcoded ports 443 and 9000 for S3 connections should be configurable or derived from the storage configuration. The TODO comments indicate this is a known issue that should be addressed. Copilot uses AI. Check for mistakes. |
||||||||
| } | ||||||||
| tempoGrpcConn = networkingv1.NetworkPolicyPort{ | ||||||||
| Protocol: ptr.To(corev1.ProtocolTCP), | ||||||||
| Port: ptr.To(intstr.FromString(manifestutils.GrpcPortName)), | ||||||||
| } | ||||||||
| otelHttpConn = networkingv1.NetworkPolicyPort{ | ||||||||
| Protocol: ptr.To(corev1.ProtocolTCP), | ||||||||
| Port: ptr.To(intstr.FromInt(4318)), | ||||||||
| } | ||||||||
| otelGrpcConn = networkingv1.NetworkPolicyPort{ | ||||||||
| Protocol: ptr.To(corev1.ProtocolTCP), | ||||||||
| Port: ptr.To(intstr.FromInt(4317)), | ||||||||
| } | ||||||||
| ) | ||||||||
| clusterIngress := map[string][]networkingv1.NetworkPolicyPort{} | ||||||||
| if tempo.Spec.Template.Gateway.Enabled { // TODO: add cluster -> gateway access | ||||||||
|
There was a problem hiding this comment. Empty port slice assignment should include a comment explaining why no specific ports are defined for gateway access or implement the TODO mentioned in the comment above.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||||
| clusterIngress[manifestutils.GatewayComponentName] = []networkingv1.NetworkPolicyPort{} | ||||||||
| } | ||||||||
|
|
||||||||
| if tempo.Spec.Template.QueryFrontend.JaegerQuery.Enabled { // TODO: add cluster -> jaegerQuery access | ||||||||
|
There was a problem hiding this comment. Empty port slice assignment should include a comment explaining why no specific ports are defined for Jaeger frontend access or implement the TODO mentioned in the comment above.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||||
| clusterIngress[manifestutils.JaegerFrontendComponentName] = []networkingv1.NetworkPolicyPort{} | ||||||||
| } | ||||||||
| return map[string]map[string][]networkingv1.NetworkPolicyPort{ | ||||||||
| netPolicyClusterComponents: clusterIngress, | ||||||||
| manifestutils.DistributorComponentName: { | ||||||||
| manifestutils.IngesterComponentName: { | ||||||||
| tempoGrpcConn, | ||||||||
| }, | ||||||||
| netPolicyOtelTargets: { | ||||||||
frzifus marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| otelGrpcConn, | ||||||||
| otelHttpConn, | ||||||||
| }, | ||||||||
| }, | ||||||||
| manifestutils.IngesterComponentName: { | ||||||||
| netPolicys3Storage: s3Conn, | ||||||||
| }, | ||||||||
| manifestutils.QuerierComponentName: { | ||||||||
| manifestutils.IngesterComponentName: { | ||||||||
| tempoGrpcConn, | ||||||||
| }, | ||||||||
| netPolicys3Storage: s3Conn, | ||||||||
| }, | ||||||||
| manifestutils.QueryFrontendComponentName: { | ||||||||
| manifestutils.QuerierComponentName: { | ||||||||
| tempoGrpcConn, | ||||||||
| }, | ||||||||
| }, | ||||||||
| manifestutils.CompactorComponentName: { | ||||||||
| netPolicys3Storage: s3Conn, | ||||||||
| }, | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| func reverseRelations(rels map[string]map[string][]networkingv1.NetworkPolicyPort) map[string]map[string][]networkingv1.NetworkPolicyPort { | ||||||||
| reverse := map[string]map[string][]networkingv1.NetworkPolicyPort{} | ||||||||
|
|
||||||||
| for from, targets := range rels { | ||||||||
| for to, ports := range targets { | ||||||||
| if _, ok := reverse[to]; !ok { | ||||||||
| reverse[to] = map[string][]networkingv1.NetworkPolicyPort{} | ||||||||
| } | ||||||||
| reverse[to][from] = append(reverse[to][from], ports...) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| return reverse | ||||||||
| } | ||||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should re-work the naming, this seems to ambigous
The config does not indicate anything about network policies