-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Advanced Policy Demo changes for v2.6 policy changes #1133
Conversation
LGTM technically, over to @emanic for docs review. |
I also added some draft release notes. |
@@ -178,33 +178,34 @@ PING google.com (216.58.219.206): 56 data bytes | |||
### Prevent outgoing connections from pods | |||
|
|||
Kubernetes NetworkPolicy does not provide a way to prevent outgoing connections from pods. However, | |||
Calico does. In this section we'll create a Policy using `calicoctl` which prevents all outgoing | |||
connections from Kubernetes pods in the advanced-policy-demo Namespace. | |||
Calico does. In this section we'll create egress Policies using `calicoctl` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Policies" --> "policies" (not a proper noun)
Calico does. In this section we'll create a Policy using `calicoctl` which prevents all outgoing | ||
connections from Kubernetes pods in the advanced-policy-demo Namespace. | ||
Calico does. In this section we'll create egress Policies using `calicoctl` | ||
which allow the outgoing connections in the advanced-policy-demo Namespace we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"which" --> "that"
backticks around "advanced-policy-demo" : advanced-policy-demo
"Namespace" --> "namespace" (not a proper noun)
connections from Kubernetes pods in the advanced-policy-demo Namespace. | ||
Calico does. In this section we'll create egress Policies using `calicoctl` | ||
which allow the outgoing connections in the advanced-policy-demo Namespace we | ||
want, all other egress traffic will be denied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"," --> ";" OR even "want, all" --> "want. All"
To do this, we'll need to create a Policy which selects all pods in the Namespace, and denies | ||
traffic that doesn't match another Pod in the Namespace. | ||
To do this, we'll need to create a Policy which selects all pods in the Namespace, and allows | ||
egress traffic to other Pods in the Namespace. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted previously, these are not proper nouns and should be lowercased: "Policy" --> "policy"; "Pods" --> "pods"; "Namespace" --> "namespace"
EOF | ||
``` | ||
|
||
Notice that we've specified an order of 500. This means that this policy will be applied before any | ||
of the Kubernetes policies. | ||
|
||
We also need to create a policy which allows traffic to access kube-dns. Let's create one now in the kube-system Namespace. | ||
We'll specify an order of 400 so that it takes precendent over other policies. | ||
We'll specify an order of 500 so that it takes precendent over the Kubernetes policies also. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"precendent" is not a word :) .... "precendent" --> "precedence"
I would suggest moving "also" before takes. Otherwise, it's confusing as to just what the also refers to.
@tmjd don't forget we need to port these changes to v2.6 when they're ready. |
LGTM thanks @tmjd ! |
Calico API using `calicoctl`. | ||
|
||
We can see that the Namespace has a corresponding [Profile]({{site.baseurl}}/{{page.version}}/reference/calicoctl/resources/profile). | ||
We can see that the namespace has a corresponding [profile]({{site.baseurl}}/{{page.version}}/reference/calicoctl/resources/profile). | ||
|
||
```shell | ||
$ calicoctl get profile -o wide | ||
NAME TAGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh @tmjd one more thing I wanted to mention and maybe you are already aware of this but we still see TAGS
in the heading. Maybe that heading still shows up in the output and just has nothing beneath it...I am not sure. Just wanted to make sure you were aware of this detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for checking but yeah that is how it looks with the TAGS
heading and nothing in that column.
Please don't merge until I squash and apply these to the v2.6 folder also. |
- Now default deny is enabled for egress when egress policy selects a pod, had to adjust policies to reflect that change - Removed Tags from example profile output from calicoctl - Made policy, pod, namespace capitalization consistent
c2282f3
to
5217cde
Compare
…3-upstream-release-v3.20 Automated cherry pick of #1133: Fix release target
Send logs both to file and kubectl logs (cherry picked from commit 880b367)
Merge pull request #1133 from song-jiang/song-felix-log
pod, had to adjust policies to reflect that change
Description
Addresses #1128
Addresses #1129
Todos
Release Note