-
Notifications
You must be signed in to change notification settings - Fork 163
LOG-7572: NetworkPolicy for ClusterLogForwarder #3101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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,104 @@ | ||
= NetworkPolicy for Collector Pods | ||
|
||
The Cluster Logging Operator automatically creates and manages a `NetworkPolicy` for its collector pods to ensure they function in restrictive network environments, even if a cluster-wide `AdminNetworkPolicy` would otherwise block their traffic. | ||
|
||
== Overview | ||
|
||
When a `ClusterLogForwarder` is deployed, the operator creates a permissive `NetworkPolicy` that allows all ingress and egress traffic for the collector pods. This ensures that log collection can function properly even when: | ||
|
||
* Restrictive default `NetworkPolicies` are in place | ||
* `AdminNetworkPolicy` configurations limit pod communications | ||
* Namespace-level network restrictions are applied | ||
|
||
The `NetworkPolicy` is automatically created and removed along with the collector deployment lifecycle. | ||
|
||
The `NetworkPolicy` can directly be edited by the cluster administrator and won't be reconciled by the operator upon updates. | ||
|
||
== NetworkPolicy Configuration | ||
|
||
The operator creates a `NetworkPolicy` for the collector with the following characteristics: | ||
|
||
```yaml | ||
apiVersion: networking.k8s.io/v1 | ||
kind: NetworkPolicy | ||
metadata: | ||
name: <COLLECTOR-INSTANCE-NAME> | ||
namespace: <COLLECTOR-NAMESPACE> | ||
labels: | ||
app.kubernetes.io/name: vector | ||
app.kubernetes.io/instance: <COLLECTOR-INSTANCE-NAME> | ||
app.kubernetes.io/component: collector | ||
app.kubernetes.io/part-of: cluster-logging | ||
app.kubernetes.io/managed-by: cluster-logging-operator | ||
app.kubernetes.io/version: <CLO-VERSION> | ||
spec: | ||
podSelector: | ||
matchLabels: | ||
app.kubernetes.io/name: vector | ||
app.kubernetes.io/instance: <COLLECTOR-INSTANCE-NAME> | ||
app.kubernetes.io/component: collector | ||
app.kubernetes.io/part-of: cluster-logging | ||
app.kubernetes.io/managed-by: cluster-logging-operator | ||
policyTypes: | ||
- Ingress | ||
- Egress | ||
ingress: | ||
- {} # Allow all ingress traffic | ||
egress: | ||
- {} # Allow all egress traffic | ||
``` | ||
|
||
== AdminNetworkPolicy Delegation | ||
|
||
When an `AdminNetworkPolicy` (ANP) is used in your cluster to enforce network restrictions, you may need to configure delegation to allow the collector's `NetworkPolicy` to take precedence for log collection traffic. | ||
|
||
=== Understanding the Hierarchy | ||
|
||
OpenShift network policy precedence (highest to lowest priority): | ||
|
||
1. **AdminNetworkPolicy** - Cluster-admin controlled, highest priority | ||
2. **BaselineAdminNetworkPolicy** - Default fallback rules | ||
3. **NetworkPolicy** - Namespace-level policies (where collector policies reside) | ||
|
||
=== Delegation Configuration | ||
|
||
To ensure collector pods can communicate properly when an `AdminNetworkPolicy` is blocking traffic, create an `AdminNetworkPolicy` rule that delegates to `NetworkPolicy` for collector traffic: | ||
|
||
==== Example: Delegating Collector Traffic | ||
|
||
```yaml | ||
apiVersion: policy.networking.k8s.io/v1alpha1 | ||
kind: AdminNetworkPolicy | ||
metadata: | ||
name: allow-logging-collector-delegation | ||
spec: | ||
priority: 50 # Adjust based on your cluster's ANP priority scheme. Lower number means higher priority | ||
subject: | ||
pods: # Target the collector pods | ||
namespaceSelector: | ||
matchLabels: | ||
kubernetes.io/metadata.name: openshift-logging # or collector namespace | ||
podSelector: | ||
matchLabels: | ||
app.kubernetes.io/name: vector | ||
app.kubernetes.io/instance: my-clf # or collector instance name | ||
app.kubernetes.io/managed-by: cluster-logging-operator | ||
app.kubernetes.io/part-of: cluster-logging | ||
app.kubernetes.io/component: collector | ||
ingress: | ||
- name: "delegate-to-collector-ingress" | ||
action: "Pass" # Pass to collector NetworkPolicy | ||
from: | ||
- {} # Delegate decisions for traffic coming from any source | ||
egress: | ||
- name: "delegate-to-collector-egress" | ||
action: "Pass" # Pass to collector NetworkPolicy | ||
to: | ||
- {} # Delegate decisions for traffic going to any destination | ||
``` | ||
|
||
== References | ||
|
||
- https://docs.redhat.com/en/documentation/openshift_container_platform/4.19/html/network_security/admin-network-policy#adminnetworkpolicy_ovn-k-anp[Openshift AdminNetworkPolicy] | ||
- https://docs.openshift.com/container-platform/latest/networking/network_policy/about-network-policy.html[OpenShift Network Policy] | ||
- https://kubernetes.io/docs/concepts/services-networking/network-policies/[Kubernetes NetworkPolicy Documentation] |
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,39 @@ | ||
package factory | ||
|
||
import ( | ||
"github.com/openshift/cluster-logging-operator/internal/constants" | ||
"github.com/openshift/cluster-logging-operator/internal/runtime" | ||
networkingv1 "k8s.io/api/networking/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
) | ||
|
||
// NewNetworkPolicy creates a NetworkPolicy for clfs. | ||
// It configures the policy to allow all ingress and egress traffic for the collector pods. | ||
func NewNetworkPolicy(namespace, policyName, instanceName string, visitors ...func(o runtime.Object)) *networkingv1.NetworkPolicy { | ||
// Create the base NetworkPolicy | ||
np := runtime.NewNetworkPolicy(namespace, policyName, visitors...) | ||
|
||
// Set up pod selector to match collector pods for this instance | ||
podSelector := runtime.Selectors(instanceName, constants.CollectorName, constants.VectorName) | ||
|
||
// Configure the NetworkPolicy spec | ||
np.Spec = networkingv1.NetworkPolicySpec{ | ||
PodSelector: metav1.LabelSelector{ | ||
MatchLabels: podSelector, | ||
}, | ||
PolicyTypes: []networkingv1.PolicyType{ | ||
networkingv1.PolicyTypeIngress, | ||
networkingv1.PolicyTypeEgress, | ||
}, | ||
// Allow all ingress traffic | ||
Ingress: []networkingv1.NetworkPolicyIngressRule{ | ||
{}, // Empty rule allows all ingress | ||
}, | ||
// Allow all egress traffic | ||
Egress: []networkingv1.NetworkPolicyEgressRule{ | ||
{}, // Empty rule allows all egress | ||
}, | ||
} | ||
|
||
return np | ||
} |
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,77 @@ | ||
package factory | ||
|
||
import ( | ||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
"github.com/openshift/cluster-logging-operator/internal/constants" | ||
"github.com/openshift/cluster-logging-operator/internal/runtime" | ||
"github.com/openshift/cluster-logging-operator/version" | ||
networkingv1 "k8s.io/api/networking/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
) | ||
|
||
var _ = Describe("#NewNetworkPolicy", func() { | ||
|
||
var ( | ||
np *networkingv1.NetworkPolicy | ||
namespace = "openshift-logging" | ||
policyName = "policy-name" | ||
instanceName = "instance-name" | ||
commonLabels = func(o runtime.Object) { | ||
runtime.SetCommonLabels(o, constants.VectorName, instanceName, constants.CollectorName) | ||
} | ||
) | ||
|
||
BeforeEach(func() { | ||
np = NewNetworkPolicy(namespace, policyName, instanceName, commonLabels) | ||
}) | ||
|
||
It("should set name and namespace correctly", func() { | ||
Expect(np.Name).To(Equal(policyName)) | ||
Expect(np.Namespace).To(Equal(namespace)) | ||
}) | ||
|
||
It("should set labels correctly", func() { | ||
expectedLabels := map[string]string{ | ||
constants.LabelK8sName: constants.VectorName, | ||
constants.LabelK8sInstance: instanceName, | ||
constants.LabelK8sComponent: constants.CollectorName, | ||
constants.LabelK8sPartOf: constants.ClusterLogging, | ||
constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, | ||
constants.LabelK8sVersion: version.Version, | ||
} | ||
|
||
Expect(np.Labels).To(Equal(expectedLabels)) | ||
}) | ||
|
||
It("should set pod selector to match collector pods", func() { | ||
expectedPodSelector := metav1.LabelSelector{ | ||
MatchLabels: map[string]string{ | ||
constants.LabelK8sName: constants.VectorName, | ||
constants.LabelK8sInstance: instanceName, | ||
constants.LabelK8sComponent: constants.CollectorName, | ||
constants.LabelK8sPartOf: constants.ClusterLogging, | ||
constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, | ||
}, | ||
} | ||
Expect(np.Spec.PodSelector).To(Equal(expectedPodSelector)) | ||
}) | ||
|
||
It("should include both Ingress and Egress policy types", func() { | ||
expectedPolicyTypes := []networkingv1.PolicyType{ | ||
networkingv1.PolicyTypeIngress, | ||
networkingv1.PolicyTypeEgress, | ||
} | ||
Expect(np.Spec.PolicyTypes).To(Equal(expectedPolicyTypes)) | ||
}) | ||
|
||
It("should have ingress rules that allow all traffic", func() { | ||
Expect(np.Spec.Ingress).To(HaveLen(1)) | ||
Expect(np.Spec.Ingress[0]).To(Equal(networkingv1.NetworkPolicyIngressRule{})) | ||
}) | ||
|
||
It("should have egress rules that allow all traffic", func() { | ||
Expect(np.Spec.Egress).To(HaveLen(1)) | ||
Expect(np.Spec.Egress[0]).To(Equal(networkingv1.NetworkPolicyEgressRule{})) | ||
}) | ||
}) |
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.
This reminds me that we need an ingress NP for LFME since it is scraped by metrics. Ideally this could simply be added to the manifest which means maybe we can wait? I would be nice not to need to manage this policy
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.
Yes I think that would be possible since we can just target the LFME pods
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.
Let's add a JIRA to deploy policy based upon the occurrence of an LFME deployment. I think its reasonable to do the same and I don't want to have to wait for the OLM changes to land
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.
Added JIRA for LFME NP