Fix cosmetic bug in error msg where a nonexistent CIDRGroup is referenced #25020
Assignees
Labels
good-first-issue
Good starting point for new developers, which requires minimal understanding of Cilium.
help-wanted
Please volunteer for this by adding yourself as an assignee!
kind/bug
This is a bug in the Cilium logic.
priority/low
This is considered nice to have.
sig/agent
Cilium agent related.
sig/policy
Impacts whether traffic is allowed or denied based on user-defined policies.
Comments
christarazi
added
kind/bug
|
Hey @christarazi, |
christarazi
added a commit
to christarazi/cilium
that referenced
this issue
Apr 24, 2023
Previously, the logic to detect whether a CIDRGroupRef has been set did
not consider CNP policies which only reference a CIDR. This is relevant
because CIDRs and CIDRGroupRefs are mutually exclusive, i.e. oneOf
validation is applied on them.
This triggered the following warning log to be triggered quite often as
each update to the CNP would run through the translation logic:
```
level=warning msg="Unable to translate all cidr groups to cidrs" cidrGroupRef="[group-1 ]" ciliumNetworkPolicyName=tofqdn-dns-visibility error="cidr group \"group-1\" not found, skipping translation; cidr group \"\" not found, skipping translation" k8sApiVersion= k8sNamespace=default subsys=k8s-watcher
```
For example, the following will trigger the buggy behavior:
```
ingress:
- fromCIDRSet:
- cidr: "3.3.3.3/32"
```
With the fix, we detect that a CIDR has been set and therefore, we avoid
returning an empty CIDRGroupRef name.
Fixes: dd4aba4 ("k8s: Add support for cidrGroupRef field in
CNP/CCNP")
Fixes: cilium#25020
Signed-off-by: Chris Tarazi <chris@isovalent.com>
christarazi
added a commit
that referenced
this issue
Apr 24, 2023
Previously, the logic to detect whether a CIDRGroupRef has been set did
not consider CNP policies which only reference a CIDR. This is relevant
because CIDRs and CIDRGroupRefs are mutually exclusive, i.e. oneOf
validation is applied on them.
This triggered the following warning log to be triggered quite often as
each update to the CNP would run through the translation logic:
```
level=warning msg="Unable to translate all cidr groups to cidrs" cidrGroupRef="[group-1 ]" ciliumNetworkPolicyName=tofqdn-dns-visibility error="cidr group \"group-1\" not found, skipping translation; cidr group \"\" not found, skipping translation" k8sApiVersion= k8sNamespace=default subsys=k8s-watcher
```
For example, the following will trigger the buggy behavior:
```
ingress:
- fromCIDRSet:
- cidr: "3.3.3.3/32"
```
With the fix, we detect that a CIDR has been set and therefore, we avoid
returning an empty CIDRGroupRef name.
Fixes: dd4aba4 ("k8s: Add support for cidrGroupRef field in
CNP/CCNP")
Fixes: #25020
Signed-off-by: Chris Tarazi <chris@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If I have one CIDRGroup referenced in a CNP, but it doesn't exist, the error msg reports that two groups are missing: the real one and one with an empty name:
The text was updated successfully, but these errors were encountered: