Skip to content
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

Warnings for NO_AF_LOG_ADDR are only relevant for non-mixed filters #324

Closed
ankenyr opened this issue Jul 19, 2023 — with Slack · 2 comments · Fixed by #329 or #336
Closed

Warnings for NO_AF_LOG_ADDR are only relevant for non-mixed filters #324

ankenyr opened this issue Jul 19, 2023 — with Slack · 2 comments · Fixed by #329 or #336
Assignees

Comments

Copy link
Collaborator

ankenyr commented Jul 19, 2023

When writing policy a mixed filter should contain the totality of terms even when some are skipped because of missing ipv4/ipv6. In the case of a ipv4 or ipv6 only filter it may be surprising to users that a filter is now no longer being output when it is expected to.

We should change logging so that NO_AF_LOG_ADDR is only emited in ipv4/6 only filters rather than mixed.

@ankenyr
Copy link
Collaborator Author

ankenyr commented Jul 19, 2023

@XioNoX
Copy link
Contributor

XioNoX commented Aug 7, 2023

@jtwb Thanks! Unfortunately there is one missing case, which is

self.NO_AF_LOG_PROTO.substitute(
for icmp vs. icmp6 terms.
For example I have those 2 terms in the same mixed policy file:

      - name: allow_ok_icmp4
        protocol: icmp
        icmp-type: echo-request echo-reply unreachable time-exceeded source-quench
        policer: policer-2m
        action: accept
      - name: allow_ok_icmp6
        protocol: icmpv6
        action: accept

Which outputs:

WARNING:absl:Term allow_ok_icmp6 will not be rendered, as it has icmpv6 match specified but the ACL is of inet address family.
WARNING:absl:Term allow_ok_icmp4 will not be rendered, as it has icmp match specified but the ACL is of inet6 address family.

But still creates the proper v4 and v6 terms in the relevant output file. Expected behavior is to not show those WARNING.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants