Add network policies #56

nsklikas · 2023-04-19T13:50:51Z

Apply network policies to deny ingress traffic to the Kratos APIs when it isn't coming from the traefik ingress.

I had to open all the none Kratos ports as I couldn't figure out exactly what is needed by juju/pebble. (need to change that).

To test these changes you need to:

# Pack the charm
$ charmcraft pack
# Deploy it 
$ juju deploy ./kratos_ubuntu-*-amd64.charm --resource oci-image=$(yq eval '.resources.oci-image.upstream-source' metadata.yaml) --trust
# Deploy other charms needed for the test 
$ juju deploy postgresql-k8s --channel edge --trust
$ juju deploy traefik-k8s traefik-admin

# Create relations
$ juju integrate kratos:admin-ingress traefik-k8s
$ juju integrate kratos postgresql-k8s

# Wait for the charms to go to idle
$ juju status

I am first creating the traefik relation and then the postgres one because I had some trouble with dns issues that where causing the migration to fail. First relating traefik and then postgres assures us that those issues are gone.

You must be able to see the network policy by running:

$ microk8s kubectl get -n {namespace} networkpolicies

To validate the networkpolicies you can juju ssh to a random pod (e.g. postgres and try to reach the kratos admin API):

$ juju ssh postgres/0 bash
$ curl {kratos_app_ip}:4434

This should timeout. To get the kratos app ip simply run juju status.
By doing the same test from the traefik pod, the http request must return.

This is fairly complex, will probably have to refactor it

gruyaume · 2023-04-24T10:15:06Z

lib/charms/kratos/v0/kubernetes_network_policies.py

+        """The default policy name that will be created."""
+        return f"{self._charm.app.name}-network-policy"
+
+    def apply_ingress_policy(


We expect a list of policies so we should probably call the method apply_ingress_policies

gruyaume · 2023-04-24T10:15:32Z

lib/charms/kratos/v0/kubernetes_network_policies.py

+            logger.error(msg)
+            raise NetworkPoliciesHandlerError()
+
+    def delete_network_policy(self, name: Optional[str] = None) -> None:


We should standardise on naming, we use ingress policies and network policies interchangeably.

gruyaume · 2023-04-24T10:17:08Z

src/charm.py

+                ]
+            )
+        except NetworkPoliciesHandlerError:
+            event.defer()


Shouldn't we simply change the status to blocked (or not catch the error and let the charm go to error state)? If something is broken we may not want to be deferring forever

lib/charms/kratos/v0/kubernetes_network_policies.py

We can't use the named port because we have named the ports on the Service and not on the Pod

gruyaume · 2023-04-26T15:00:12Z

lib/charms/kratos/v0/kubernetes_network_policies.py

Where are the unit tests for this file 👀

natalian98 · 2023-04-27T13:06:19Z

lib/charms/kratos/v0/kubernetes_network_policies.py

+
+
+class NetworkPoliciesHandlerError(Exception):
+    """Applying the network policies failed."""


Suggested change

"""Applying the network policies failed."""

"""Raised when applying the network policies failed."""

natalian98 · 2023-04-27T13:08:23Z

src/charm.py

@@ -442,6 +449,9 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None:
        """Event Handler for config changed event."""
        self._handle_status_update_config(event)

+    def _cleanup(self, event: RemoveEvent) -> None:
+        self.network_policy_handler.delete_ingress_policies()


It might be a juju bug, but when I try removing kratos it gets into terminated state with unit lost. When I use --force --no-wait, the charm is removed but in both cases the network policy is not deleted.

So I tried testing it a little more and when I test it manually, the policy is removed. I wrote an integration test to validate this and in the test the policy is not removed. This is very weird.

As far as I can tell the stop event is never fired, this is most likely a juju bug and not something we can control. Let's leave this as it is for now, it is the best we can do and juju should (hopefully) fix it in a later version.

natalian98 · 2023-04-27T13:11:41Z

lib/charms/kratos/v0/kubernetes_network_policies.py

+    def apply_ingress_policies(
+        self, policies: List[IngressPolicyDefinition], name: Optional[str] = None
+    ) -> None:
+        """Apply an ingress network policy about a related application.


Suggested change

"""Apply an ingress network policy about a related application.

"""Apply an ingress network policy for a related application.

natalian98 · 2023-04-27T13:29:16Z

tests/integration/conftest.py

@@ -0,0 +1,14 @@
+#!/usr/bin/env python3
+# Copyright 2022 Canonical Ltd.


Suggested change

# Copyright 2022 Canonical Ltd.

# Copyright 2023 Canonical Ltd.

natalian98 · 2023-04-27T13:34:54Z

src/charm.py

+                    (38813, []),
+                ]
+            )
+        except NetworkPoliciesHandlerError:


Should we retry applying the network policy?
Right now if you deploy kratos without trust, it gets into BlockedStatus, but if you trust it then it gets into active without creating the network policy.

nsklikas · 2023-04-28T12:39:02Z

In case applying the network policies fails we have several options:

Log the error and keep the normal operation, if the user wants they can deploy the charm (or re trigger the hook)
Go to a blocked state, this would make state management much more complicated
Raise an exception
Log an error and defer the event until patching succeeds

Since it is not needed for the charm operation and it is not essential for security I think that (1) and (4) make most sense because they allow the charm to run despite any error that might occur and the user can make the decision to stop the charm if they deem it necessary.

If we decide to go with (1) we can also provide an action for applying the network policies to allow the user to retry applying them if something goes wrong. cc @natalian98 @gruyaume

nsklikas requested a review from a team as a code owner April 19, 2023 13:50

nsklikas marked this pull request as draft April 19, 2023 13:51

Add network policies

2edc700

nsklikas force-pushed the IAM-217-network-policies branch from 6c96fb6 to 2edc700 Compare April 19, 2023 13:51

Fix linting

bfe6ecb

nsklikas force-pushed the IAM-217-network-policies branch from d98fa4b to bfe6ecb Compare April 19, 2023 14:26

Fix dns

c72be88

nsklikas force-pushed the IAM-217-network-policies branch 2 times, most recently from c237036 to 4f189f6 Compare April 20, 2023 09:06

Working version

2a4f782

This is fairly complex, will probably have to refactor it

nsklikas force-pushed the IAM-217-network-policies branch from 4f189f6 to 2a4f782 Compare April 20, 2023 09:28

nsklikas marked this pull request as ready for review April 20, 2023 10:12

nsklikas force-pushed the IAM-217-network-policies branch 3 times, most recently from 1afc5cf to 2a4f782 Compare April 21, 2023 07:42

Fix tests

51b8643

nsklikas force-pushed the IAM-217-network-policies branch from c10a54e to 51b8643 Compare April 21, 2023 09:09

Move k8s_network_policies to library

c2e5475

nsklikas force-pushed the IAM-217-network-policies branch from 833948e to c2e5475 Compare April 21, 2023 09:46

Refactor

08c463a

nsklikas force-pushed the IAM-217-network-policies branch from 994419e to 08c463a Compare April 21, 2023 10:02

nsklikas added 3 commits April 21, 2023 13:10

Update CONTRIBUTING.md, update policy

f821b8d

Allow traffic only from pebble ports

d3f5a2f

Simplify implementation

f5b33f7

nsklikas force-pushed the IAM-217-network-policies branch from 3301bd4 to f5b33f7 Compare April 21, 2023 14:49

gruyaume reviewed Apr 24, 2023

View reviewed changes

nsklikas marked this pull request as draft April 24, 2023 10:52

nsklikas added 3 commits April 25, 2023 15:34

Refactor

e000e8e

Add comment

47ea9cb

Target named port

70544be

We can't use the named port because we have named the ports on the Service and not on the Pod

nsklikas force-pushed the IAM-217-network-policies branch from 5d189a0 to 70544be Compare April 25, 2023 12:34

nsklikas marked this pull request as ready for review April 25, 2023 14:56

Do not defer event on failure

5bcc3d0

gruyaume suggested changes Apr 26, 2023

View reviewed changes

lib/charms/kratos/v0/kubernetes_network_policies.py

Copy link

gruyaume Apr 26, 2023

Choose a reason for hiding this comment

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

Where are the unit tests for this file 👀

natalian98 reviewed Apr 27, 2023

View reviewed changes

nsklikas added 3 commits April 28, 2023 13:05

Requested fixes

0b024e9

Add unit tests

a9aa410

Add integration test

b902862

nsklikas force-pushed the IAM-217-network-policies branch from 51c1c17 to b902862 Compare April 28, 2023 12:22

Swallow error if applying network policies failed

e711dd3

nsklikas added 3 commits April 28, 2023 15:42

Mock lk client

02bb830

Add type hint

a43a119

Fix linting

7306c38

nsklikas marked this pull request as draft May 23, 2023 11:41

nsklikas added the nomerge label Aug 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add network policies #56

Add network policies #56

nsklikas commented Apr 19, 2023 •

edited

Loading

gruyaume Apr 24, 2023

gruyaume Apr 24, 2023

gruyaume Apr 24, 2023

gruyaume Apr 26, 2023

natalian98 Apr 27, 2023

natalian98 Apr 27, 2023

nsklikas Apr 28, 2023

nsklikas Apr 28, 2023 •

edited

Loading

natalian98 Apr 27, 2023

natalian98 Apr 27, 2023

natalian98 Apr 27, 2023

nsklikas commented Apr 28, 2023



		class NetworkPoliciesHandlerError(Exception):
		"""Applying the network policies failed."""

	"""Applying the network policies failed."""
	"""Raised when applying the network policies failed."""

	"""Apply an ingress network policy about a related application.
	"""Apply an ingress network policy for a related application.

		@@ -0,0 +1,14 @@
		#!/usr/bin/env python3
		# Copyright 2022 Canonical Ltd.

	# Copyright 2022 Canonical Ltd.
	# Copyright 2023 Canonical Ltd.

Add network policies #56

Are you sure you want to change the base?

Add network policies #56

Conversation

nsklikas commented Apr 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsklikas Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsklikas commented Apr 28, 2023

nsklikas commented Apr 19, 2023 •

edited

Loading

nsklikas Apr 28, 2023 •

edited

Loading