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

feat: add network policies reference #425

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Conversation

SdgJlbl
Copy link
Contributor

@SdgJlbl SdgJlbl commented Jun 26, 2024

Fixes fl-1580

Copy link

linear bot commented Jun 26, 2024

@SdgJlbl SdgJlbl requested a review from guilhem-barthes June 26, 2024 15:03
@guilhem-barthes
Copy link
Contributor

Can you move your content to another file and include it in the index (Cf #424)

Copy link
Contributor

@guilhem-barthes guilhem-barthes left a comment

Choose a reason for hiding this comment

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

Thanks for your work! I made some comments on some flows, let me know if you think they are relevant!

+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------+
| Pod | Incoming | Outgoing |
+==========================+=========================================================================================================================================+=====================================================================================================================================+
| orchestrator-server | backend-api-events, backend-worker-events, backend-scheduler, backend-scheduler-worker, backend-server, backend-builder, backend-worker | orchestrator-database |
Copy link
Contributor

Choose a reason for hiding this comment

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

Incoming also from internet, for other organisations. Basically it is opened to everything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK with prefacing that all these connections come from other organisations?
The idea is to give the option, if people deploying the network are using a more advanced network plugin, to know what connections exactly need to be allowed

+==========================+=========================================================================================================================================+=====================================================================================================================================+
| orchestrator-server | backend-api-events, backend-worker-events, backend-scheduler, backend-scheduler-worker, backend-server, backend-builder, backend-worker | orchestrator-database |
+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------+
| orchestrator-database | orchestrator-server | NONE |
Copy link
Contributor

Choose a reason for hiding this comment

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

Incoming: also orchestrator-migrations

Comment on lines 34 to 36
| orchestrator-server | backend-api-events, backend-worker-events, backend-scheduler, backend-scheduler-worker, backend-server, backend-builder, backend-worker | orchestrator-database |
+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------+
| orchestrator-database | orchestrator-server | NONE |
Copy link
Contributor

Choose a reason for hiding this comment

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

All pods are given access to kube-dns on port 53, I don't know if it worth specifying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are there cases where pods in kube are not given access to the DNS resolver?
maybe I can add a footnote to cover all cases?

docs/source/reference/index.rst Outdated Show resolved Hide resolved
docs/source/reference/index.rst Outdated Show resolved Hide resolved
Comment on lines 79 to 80
It defines a set of roles (minimal network policies that block or allow a given connection) and relies on label selectors
to apply these roles to appropriate pods.
Copy link
Contributor

Choose a reason for hiding this comment

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

SIn some, we also uses IP ranges

docs/source/reference/index.rst Outdated Show resolved Hide resolved
docs/source/reference/index.rst Outdated Show resolved Hide resolved
docs/source/reference/index.rst Outdated Show resolved Hide resolved
@SdgJlbl SdgJlbl force-pushed the feat/add-netpol-reference branch 4 times, most recently from ac171a0 to b79d5e6 Compare July 2, 2024 16:08
@SdgJlbl SdgJlbl force-pushed the feat/add-netpol-reference branch from b79d5e6 to 5cb702c Compare July 3, 2024 06:55
@SdgJlbl SdgJlbl requested a review from guilhem-barthes July 3, 2024 07:09
Copy link
Contributor

@guilhem-barthes guilhem-barthes left a comment

Choose a reason for hiding this comment

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

LGTM

@SdgJlbl SdgJlbl merged commit ec0ca34 into main Jul 3, 2024
3 checks passed
@SdgJlbl SdgJlbl deleted the feat/add-netpol-reference branch July 3, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants