feat/policy-names: Add network policy names when they are known. #727

kgtw · 2023-11-26T13:46:21Z

Introduces new Egress allowed by policies and Ingress allowed by policies info blocks within the the respective flows detailed sidebar information.

Policy names are only shown when they have successfully been correlated to a flow by cilium-agent. In a handful of known scenarios where cilium is allowing traffic internally (like allowing localhost access) we try to map the policy to a human friendly name with the value taken from the reserved:io.cilium.policy.derived-from label.

Fixes: cilium/hubble#1100

Example screenshot demonstrating a custom ingress policy, as well as showing the cilium internal policy which is prefixed with <cilium-internal>/.

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

kimstacy · 2023-12-09T00:34:16Z

@kgtw Thank you for your contribution! Will you please provide the custom policies used to test this feature?

kgtw · 2023-12-10T10:00:37Z

Hey @kimstacy, these were the policies that were used to test this change with.

---
apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
  name: custom-ingress
  namespace: default
spec:
  endpointSelector: {}
  ingress:
    - fromEntities:
      - world
      - cluster

---
apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
  name: custom-egress
  namespace: default
spec:
  endpointSelector: {}
  egress:
    - toEntities:
      - world
      - cluster

---
apiVersion: cilium.io/v2
kind: CiliumClusterwideNetworkPolicy
metadata:
  name: cluster-wide-custom-policy
spec:
  endpointSelector: {}
  egress:
    - toEntities:
      - all
  ingress:
    - fromEntities:
      - all

kimstacy · 2023-12-13T08:08:17Z

Thank you! LGTM.

kimstacy · 2023-12-15T17:29:39Z

Hi @kgtw 👋

There seems to be a minor CI issue. The first relates to CodeQL:

2023/12/13 08:09:18 Error running go tooling: err: exit status 1: stderr: go: errors parsing go.mod:
/home/runner/work/hubble-ui/hubble-ui/backend/go.mod:3: invalid go version '1.21.0': must match format 1.23
/home/runner/work/hubble-ui/hubble-ui/backend/go.mod:5: unknown directive: toolchain
  
2023/12/13 08:09:18 Extraction failed: exit status 1
Error: We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. Encountered a fatal error while running "/opt/hostedtoolcache/CodeQL/2.15.4/x64/codeql/go/tools/autobuild.sh". Exit code was 1 and last log line was: 2023/12/13 08:09:18 Extraction failed: exit status 1. See the logs for more details.

For the second, per test / backend check's output, would you please try running ./ctl.sh update-proto and resubmitting your changes?

…e known and correlated to flows. Fixes: cilium/hubble#1100 Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

kgtw · 2023-12-18T09:43:03Z

Hi @kimstacy, I'm hoping the latest commit should have resolved the issues with the CI.

I'm not entirely sure why it is happening, but when using ./ctl.sh update-proto with the current go.mod version of 1.21 it was adding the problematic toolchain go1.21.4 line.

Signed-off-by: Dmitry Kharitonov <dmitry@isovalent.com>

geakstr · 2023-12-20T13:40:43Z

Hi @kgtw, thanks for your contribution! There was some strange issue with go version and dependencies indeed, so I updated them and pushed the commit. CI is happy now but there are some merge conflicts. Could you rebase and resolve them? I think we are good to go after that.

mbounaceur · 2024-03-04T10:26:08Z

Hi, this feature is very useful. I was looking for it

Any update please @kgtw ?

Thanks !

kgtw · 2024-04-05T15:32:24Z

Apologies for the delay, I've just returned from a sabbatical from work and will be looking to update this PR in the coming days.

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

kgtw · 2024-04-06T21:17:26Z

@geakstr this should be ready for another review now. Sorry for the slow response times.

Something I did notice while re-testing PR is that policy correlation is only happening on the first SYN packet for the connection. I haven't had time to investigate if this is a bug or expected within cilium-agent.

Update 15/04: The above makes sense as policy evaluation is a costly action. I'm wondering if we should make additional UX improvements to help identify the initial packet flow that was associated with the policy in a way which doesn't lead to more confusion.

yannikmesserli

Thanks! This looks good. I've left suggestion to improve the code, e.g. div instead span, but I'll approuve this PR as this isn't substantial changes IMHO. Up to you if you want to adopt them.

Something I did notice while re-testing PR is that policy correlation is only happening on the first SYN packet for the connection

Yes, we do populate the policy field only on verdict events. @gandro implemented originally this feature, he can shed some lights about the reason behind it.

I don't think it would be sane that UI try fix this somehow, so maybe we can either open a feature on the Cilium repo, or document this somehow? WDYT @geakstr @kgtw ?

yannikmesserli · 2024-04-19T07:54:26Z

src/components/FlowsTable/SidebarComponents.tsx

@@ -242,3 +242,29 @@ export const PodEntry = memo<PodItemProps>(function FlowsTableSidebarPodEntry(pr
    </span>
  );
 });
+
+export interface PolicyEntryProps {


Suggested change

export interface PolicyEntryProps {

export interface PoliciesListProps {

yannikmesserli · 2024-04-19T07:54:36Z

src/components/FlowsTable/SidebarComponents.tsx

+  allowedBy: string[];
+}
+
+export const PolicyEntry = memo<PolicyEntryProps>(


Suggested change

export const PolicyEntry = memo<PolicyEntryProps>(

export const PoliciesList = memo<PoliciesListProps>(

yannikmesserli · 2024-04-19T07:54:52Z

src/components/FlowsTable/SidebarComponents.tsx

+  name: string;
+}
+
+export const PolicyEntryItem = memo<PolicyEntryItemProps>(


Suggested change

export const PolicyEntryItem = memo<PolicyEntryItemProps>(

export const PoliciesListItem = memo<PoliciesListItemProps>(

yannikmesserli · 2024-04-19T07:54:59Z

src/components/FlowsTable/SidebarComponents.tsx

+  },
+);
+
+export interface PolicyEntryItemProps {


Suggested change

export interface PolicyEntryItemProps {

export interface PoliciesListItemProps {

yannikmesserli · 2024-04-19T07:59:03Z

src/components/FlowsTable/SidebarComponents.tsx

+
+export const PolicyEntryItem = memo<PolicyEntryItemProps>(
+  function FlowsTableSidebarPolicyEntry(props) {
+    return <span className={css.policy}>{props.name}</span>;


Suggested change

return <span className={css.policy}>{props.name}</span>;

return <div>{props.name}</div>;

yannikmesserli · 2024-04-19T07:59:30Z

src/components/FlowsTable/styles.scss

+      .policies {
+        .policy {
+          display: block;
+        }
+      }
+


Suggested change

.policies {

.policy {

display: block;

}

}

Let's use a div, so no need of this.

yannikmesserli · 2024-04-19T08:28:22Z

src/components/FlowsTable/SidebarComponents.tsx

+export const PolicyEntry = memo<PolicyEntryProps>(
+  function FlowsTableSidebarLabelsEntry(props) {
+    return (
+      <div className={css.policies}>


Suggested change

<div className={css.policies}>

<div>

kgtw added 2 commits November 26, 2023 21:11

chore/deps: bump go pkgs

ba0d54b

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

chore/deps: update protobuf

05eebce

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

kgtw requested a review from a team as a code owner November 26, 2023 13:46

kgtw requested review from yandzee and removed request for a team November 26, 2023 13:46

kimstacy self-requested a review December 7, 2023 17:06

kgtw force-pushed the feat/policy-names branch from fd2c4ce to 8e4f5bb Compare December 18, 2023 09:23

feat/policy-names: Add network policy names in hubble UI when they ar…

bd38b66

…e known and correlated to flows. Fixes: cilium/hubble#1100 Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

kgtw force-pushed the feat/policy-names branch from 8e4f5bb to bd38b66 Compare December 18, 2023 09:28

geakstr force-pushed the feat/policy-names branch from a7ed8ff to 01ea202 Compare December 20, 2023 13:27

deps: bump go to 1.21.5. bump go pkgs to latest

dd617e1

Signed-off-by: Dmitry Kharitonov <dmitry@isovalent.com>

geakstr force-pushed the feat/policy-names branch from 01ea202 to dd617e1 Compare December 20, 2023 13:34

geakstr mentioned this pull request Jan 24, 2024

Release 0.12.3 #768

Merged

geakstr self-assigned this Apr 2, 2024

kgtw added 2 commits April 6, 2024 21:40

rebase and resolve conflicts

af3ad60

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

generate protobuf

11c210f

Signed-off-by: Kris Gambirazzi <kris.gambirazzi@transferwise.com>

yannikmesserli approved these changes Apr 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/policy-names: Add network policy names when they are known. #727

feat/policy-names: Add network policy names when they are known. #727

kgtw commented Nov 26, 2023

kimstacy commented Dec 9, 2023

kgtw commented Dec 10, 2023

kimstacy commented Dec 13, 2023

kimstacy commented Dec 15, 2023 •

edited

kgtw commented Dec 18, 2023

geakstr commented Dec 20, 2023

mbounaceur commented Mar 4, 2024

kgtw commented Apr 5, 2024

kgtw commented Apr 6, 2024 •

edited

yannikmesserli left a comment

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

yannikmesserli Apr 19, 2024

	export interface PolicyEntryProps {
	export interface PoliciesListProps {

	export const PolicyEntry = memo<PolicyEntryProps>(
	export const PoliciesList = memo<PoliciesListProps>(

	export const PolicyEntryItem = memo<PolicyEntryItemProps>(
	export const PoliciesListItem = memo<PoliciesListItemProps>(

	export interface PolicyEntryItemProps {
	export interface PoliciesListItemProps {

	return <span className={css.policy}>{props.name}</span>;
	return <div>{props.name}</div>;

feat/policy-names: Add network policy names when they are known. #727

Are you sure you want to change the base?

feat/policy-names: Add network policy names when they are known. #727

Conversation

kgtw commented Nov 26, 2023

kimstacy commented Dec 9, 2023

kgtw commented Dec 10, 2023

kimstacy commented Dec 13, 2023

kimstacy commented Dec 15, 2023 • edited

kgtw commented Dec 18, 2023

geakstr commented Dec 20, 2023

mbounaceur commented Mar 4, 2024

kgtw commented Apr 5, 2024

kgtw commented Apr 6, 2024 • edited

yannikmesserli left a comment

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

yannikmesserli Apr 19, 2024

Choose a reason for hiding this comment

kimstacy commented Dec 15, 2023 •

edited

kgtw commented Apr 6, 2024 •

edited