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

hubble: Display proxy redirects in policy verdict events #17411

Merged
merged 1 commit into from Sep 17, 2021

Conversation

pchaigno
Copy link
Member

Before this commit, Hubble was ignoring proxy redirection information from the policy-verdict events it received from the datapath. For example, a cilium monitor event such as:

Policy verdict log: flow 0x0 local EP ID 1531, remote ID 35429, proto 17, egress, action redirect, match L3-L4, 10.240.0.62:37282 -> 10.240.0.63:53 udp

would be displayed in hubble observe as:

Sep 15 17:23:11.960: cilium-test/client-6488dcf5d4-f9kfl:37282 -> kube-system/coredns-d4866bcb7-zh5jv:53 L3-L4 FORWARDED (UDP)

This pull request adds a new verdict REDIRECTED to signal such event. Such events now appear as:

default/pod-to-external-fqdn-allow-google-cnp-5ff4986c89-n87h2:58314 -> kube-system/coredns-755cd654d4-j4vzh:53 UNKNOWN 5 (UDP)

A subsequent patch to the Hubble command line will display value 5 as REDIRECTED.

Before this commit, Hubble was ignoring proxy redirection information
from the policy-verdict events it received from the datapath. For
example, a cilium monitor event such as:

    Policy verdict log: flow 0x0 local EP ID 1531, remote ID 35429, proto 17, egress, action redirect, match L3-L4, 10.240.0.62:37282 -> 10.240.0.63:53 udp

would be displayed in hubble observe as:

    Sep 15 17:23:11.960: cilium-test/client-6488dcf5d4-f9kfl:37282 -> kube-system/coredns-d4866bcb7-zh5jv:53 L3-L4 FORWARDED (UDP)

This commit adds a new verdict REDIRECTED to signal such event. Such
events now appear as:

    default/pod-to-external-fqdn-allow-google-cnp-5ff4986c89-n87h2:58314 -> kube-system/coredns-755cd654d4-j4vzh:53 UNKNOWN 5 (UDP)

A subsequent patch to the Hubble command line will display value 5 as
"REDIRECTED".

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/hubble Impacts hubble server or relay needs-backport/1.10 labels Sep 15, 2021
@pchaigno pchaigno requested a review from a team as a code owner September 15, 2021 19:18
@pchaigno pchaigno requested review from a team, nathanjsweet and glibsm September 15, 2021 19:18
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.5 Sep 15, 2021
Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@pchaigno
Copy link
Member Author

pchaigno commented Sep 15, 2021

test-me-please

Job 'Cilium-PR-K8s-1.19-kernel-5.4' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig AutoDirectNodeRoutes Check connectivity with sockops and direct routing

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.19-kernel-5.4/src/github.com/cilium/cilium/test/k8sT/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-c5vk5 policy revision: cannot get revision from json output '': could not parse JSON from command "kubectl exec -n kube-system cilium-c5vk5 -- cilium policy get -o json"

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.19-kernel-5.4 so I can create a new GitHub issue to track it.

@aanm aanm merged commit c40ed79 into cilium:master Sep 17, 2021
@pchaigno pchaigno deleted the hubble-proxy-redirects-policy-verdicts branch September 17, 2021 09:11
pchaigno added a commit to cilium/hubble that referenced this pull request Sep 17, 2021
To pull in cilium/cilium#17411 which defines
Verdict_REDIRECTED.

Signed-off-by: Paul Chaignon <paul@cilium.io>
pchaigno added a commit to cilium/hubble that referenced this pull request Sep 20, 2021
To pull in cilium/cilium#17411 which defines
Verdict_REDIRECTED.

Signed-off-by: Paul Chaignon <paul@cilium.io>
rolinh pushed a commit to cilium/hubble that referenced this pull request Sep 20, 2021
To pull in cilium/cilium#17411 which defines
Verdict_REDIRECTED.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.5 Sep 29, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.5 Oct 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.5 Oct 2, 2021
errordeveloper added a commit to cilium/hubble-otel that referenced this pull request Nov 10, 2021
This is required to avoid `unknown Cilium policy verdict event`
spans that happen due to addition of new REDIRECTED verdict
(see cilium/cilium#17411).

New version of Hubble hasn't been released yet, but the change
has been backported to v0.8 branch (cilium/hubble#639).
errordeveloper added a commit to cilium/hubble-otel that referenced this pull request Nov 10, 2021
This is required to avoid `unknown Cilium policy verdict event`
spans that happen due to addition of new REDIRECTED verdict
(see cilium/cilium#17411).

New version of Hubble hasn't been released yet, but the change
has been backported to v0.8 branch (cilium/hubble#639).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.10.5
Backport done to v1.10
Development

Successfully merging this pull request may close these issues.

None yet

7 participants