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

Add blog post for getting started with ANP #146

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Sep 29, 2023

Adding the first blog post for the network-policy-api website, covering basics of AdminNetworkPolicy API usage and interoperation with NetworkPolicy

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 29, 2023
@netlify
Copy link

netlify bot commented Sep 29, 2023

Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name Link
🔨 Latest commit 6f17d5f
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/65c11b3344398e0008e9c363
😎 Deploy Preview https://deploy-preview-146--kubernetes-sigs-network-policy-api.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 29, 2023
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Sep 29, 2023

/cc @rahulkjoshi @npinaeva

@k8s-ci-robot
Copy link
Contributor

@Dyanngg: GitHub didn't allow me to request PR reviews from the following users: rahulkjoshi.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @rahulkjoshi @npinaeva

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Member

@npinaeva npinaeva left a comment

Choose a reason for hiding this comment

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

Very nice explanation, and pictures make it easy to understand!
nit: I see .DB_Store files sneaked into this commit :)


1. There are certain traffic patterns in the cluster that, as a cluster admin, I want to always allow. For example, all namespaces should allow ingress traffic from a monitoring application in the cluster for compliance reasons, and all namespaces should allow egress toward kube-dns running in the kube-system namespace.

2. As a cluster admin I want to change the default security model for my cluster, so that all inter-tenant traffic (traffic between namespaces with different tenant labels) is blocked by default. Namespace owners will need to use NetworkPolicies to explicitly allow known traffic.
Copy link
Member

Choose a reason for hiding this comment

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

not sure if this list should cover all new objects, but should we add BANP use case here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I'm assuming the second use case covers BANP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased it.

Copy link
Member

Choose a reason for hiding this comment

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

ah so second point is BANP+tenancy?

blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved

- `AdminNetworkPolicy` and `BaselineAdminNetworkPolicy` are cluster-scoped resources. The subject of the policy can be the entire cluster namespaces, a select set of namespaces, or even select set of pods in a select set of namespaces, depending on the actual use of the policy.

- AdminNetworkPolicy rules are enforced in a hierarchical order based on priority. `AdminNetworkPolicy` objects with a smaller numerical `priority` in its spec have higher precedence over those with a larger numerical `priority`. Furthermore, rules inside a policy are enforced in the order they are written (ingress and egress rules does not have relevant order among each other though). To avoid potential conflict in rules, we generally advise against creating multiple `AdminNetworkPolicy` objects at the same `priority`.
Copy link
Member

Choose a reason for hiding this comment

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

does it mean we don't have validation to check there are no 2 objects with the same priority?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we do not have validation. It is due to the fact that there could be different ANPs in the cluster applied to entirely separate workloads and control distinct traffic patterns, but serves similar purposes so logically it makes sense to be created at the same priority.

Copy link
Member

Choose a reason for hiding this comment

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

if policies at the same prio are applied to separate workloads, then they may have different priorities as well with the same effect right? Is it worth it risking undefined behaviour by allowing same priority ANPs in that case? (not related to this PR ofc, just wondering)

blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved

![Diagram2](Diagram2.png)

1. Traffic within the two sales tenant namespaces is allowed. Firstly, no `AdminNetworkPolicy` rule matches this type of traffic. Both tenant=sales namespaces are indeed selected by the "cluster-basic-allow" `AdminNetworkPolicy`, but the flow in question is neither going to dns components nor from the monitoring namespace. Moving further down the hierarchy, no `NetworkPolicy` can possibly be matched as well since there are none of them in the cluster currently. Finally, the "default" `BaselineAdminNetworkPolicy` is evaluated, and this traffic flow matches the first rule of the `BaselineAdminNetworkPolicy` egress section: for packets sent by the `app=db` pod towards the `app=web` pod, the first egress rule is matched from the perspective of namespace `s1`, since the rule dictates that for all namespaces any egress traffic going to namespaces with the same `tenant` label should be given the green light. The same logic applies for the reverse traffic.
Copy link
Member

Choose a reason for hiding this comment

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

I think it may be easier to read and understand if we go in the decreasing priority order, so that you don't need to check if there is anything with a higher prio every time, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ummm I'm not too sure I follow what you mean here. In a waterfall-like priority rule evaluation model, we would need to first check the higher priority rules and move on if they don't match, right?

Copy link
Member

Choose a reason for hiding this comment

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

yeah I mean since cluster-basic-allow is the highest priority policy, it may be easier to start with connection 3 and 4, and then you won't need Both tenant=sales namespaces are indeed selected by the "cluster-basic-allow" AdminNetworkPolicy, but the connection in question is neither going to dns components nor from the monitoring namespace. for flow 1 because we have already reviewed all higher-prio policies?

blog/Getting Started with ANP.md Outdated Show resolved Hide resolved

![Diagram3](Diagram3.png)

On the other hand, there's a caveat that many will find surprising: traffic from `s1`, the other sales tenant namespace, will be blocked(7) when it reaches the pod in `s2`. Did we not create a baseline allow rule for each namespace in the same tenant to connect to each other? Yes we did. But as mentioned before, the AdminNetworkPolicy API does not change the behavior of NetworkPolicy objects. The `allow-access-to-web` NetworkPolicy, in this case, can be read as "all pods in s2 are isolated in the ingress direction, except for traffic from a1 which is allowed". So connections initiated from `s1` to `s2` will be dropped due to this implicit isolation effect before the `BaselineAdminNetworkPolicy` rules are even evaluated.
Copy link
Member

Choose a reason for hiding this comment

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

I mean, very good point

Copy link
Member

@astoycos astoycos 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 doing this @Dyanngg Sorry for all the comments :/

I think overall I like the message and demo scenarios... but there are a few things I think we should think about:

  • Do we need so much first-person
  • I think we want to try a bit less detailed in some of the traffic flow descriptions as it's fairly easy to get lost with all the "as above" references etc

blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
traffic from `s1`. The important aspect here though, is that once a NetworkPolicy is applied
to a pod, it will control it security posture at entirety, unless a `AdminNetworkPolicy` rule
overrides it from a priority above. `BaselineAdminNetworkPolicy` rules are only effective in
the absence of those policies.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe have a quick wrap up paragraph with details on how to get involved in the group?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will work on this

@astoycos
Copy link
Member

astoycos commented Jan 8, 2024

@Dyanngg Are you ready for another pass here?

@Dyanngg
Copy link
Contributor Author

Dyanngg commented Jan 12, 2024

@Dyanngg Are you ready for another pass here?

Yep I think the vast majority of your first round comments are addressed

@astoycos
Copy link
Member

This looks great @Dyanngg!! Please just Add it to the blog https://network-policy-api.sigs.k8s.io/blog/ by moving to https://github.com/kubernetes-sigs/network-policy-api/tree/main/site-src/blog/posts updating the index etc

Otherwise it should be GTG

Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

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

Great idea to make a blog post, this looks awesome, just had a couple thoughts

traffic (most commonly dns) when they apply NetworkPolicies. This is due to the implicit
deny nature of the NetworkPolicy semantics: pods selected by NetworkPolicies are automatically
isolated except for any ingress/egress traffic that is explicitly allowed.
So, without any admin-facing policy APIs, to "ensure" that dns always works and monitoring
Copy link
Contributor

Choose a reason for hiding this comment

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

The reader may benefit from section headers

Seems like we have:

  • Problem Overview
  • NetPolV1 Solution and Limitations
  • Improved Solution with Admin policies
  • Connectivity example with AdminNetworkPolicy
  • Continued example with BaselineAdminNetworkPolicy
  • Takeaways & Next Steps

blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
blog/Getting Started with ANP.md Outdated Show resolved Hide resolved
@Dyanngg Dyanngg changed the title [WIP] Add blog post for getting started with ANP Add blog post for getting started with ANP Jan 30, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2024
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Feb 2, 2024

@astoycos Can you check if you have any other comments that need to be addressed? Otherwise let's get our first blog on to the website :P

@astoycos
Copy link
Member

astoycos commented Feb 2, 2024

@Dyanngg Content looks good but the blog still isn't rendering correctly I don't think :)

https://deploy-preview-146--kubernetes-sigs-network-policy-api.netlify.app/blog/

Signed-off-by: Dyanngg <dingyang@vmware.com>
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Feb 2, 2024

@Dyanngg Content looks good but the blog still isn't rendering correctly I don't think :)

https://deploy-preview-146--kubernetes-sigs-network-policy-api.netlify.app/blog/

Thanks for checking Andrew!! Now it renders correctly but seems the entire blog content is on the index page. Maybe it's because there's only one blog as of now?

Signed-off-by: astoycos <astoycos@redhat.com>
@astoycos
Copy link
Member

astoycos commented Feb 5, 2024

I fixed it @Dyanngg :) 🥳

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astoycos, Dyanngg, huntergregory

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 6b1cf9f into kubernetes-sigs:main Feb 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants