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

docs: improve Host Firewall GSG #13673

Merged
merged 1 commit into from
Oct 22, 2020
Merged

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Oct 21, 2020

As part of the validation of GSGs (#13627) for the 1.9 release, here are several improvement suggestions for the Host Firewall GSG.

Fixes:

  • Fix Helm option: --set devices=ethX,ethY does not work (missing curly braces and quotes).
  • Rename one occurrence of access=ssh into node-access=ssh, for consistency.
  • Add a missing closing parenthesis at the end of the command that sets the HOST_EP_ID variable.
  • Use jsonpath and kubectl's options in one command to avoid piping the output to jq.

Additions:

  • Explain what the interfaces passed to the --devices option are for. This is mostly copied from the description in the Host Policies description (Layer 3 Examples).
  • Explain that per-endpoint routing is not compatible with the host firewall and must be disabled, in particular on managed environments like GKE.
  • Add node label removal to the clean-up section.
  • Format shell session code as shell session code.

@qmonnet qmonnet added priority/release-blocker area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. area/host-firewall Impacts the host firewall or the host endpoint. labels Oct 21, 2020
@qmonnet qmonnet requested a review from pchaigno October 21, 2020 16:46
@qmonnet qmonnet requested a review from a team as a code owner October 21, 2020 16:46
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.0-rc3 Oct 21, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.5 Oct 21, 2020
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks! It's great to get 👀 on this guide 🎉

Documentation/gettingstarted/host-firewall.rst Outdated Show resolved Hide resolved
Documentation/gettingstarted/host-firewall.rst Outdated Show resolved Hide resolved
Documentation/gettingstarted/host-firewall.rst Outdated Show resolved Hide resolved
Documentation/gettingstarted/host-firewall.rst Outdated Show resolved Hide resolved
Fixes:

- Fix Helm option: "--set devices=ethX,ethY" does not work (missing
  curly braces and quotes).
- Rename one occurrence of "access=ssh" into "node-access=ssh", for
  consistency.
- Add a missing closing parenthesis at the end of the command that sets
  the HOST_EP_ID variable.
- Use jsonpath and kubectl's options in one command to avoid piping the
  output to jq.

Additions:

- Explain what the interfaces passed to the "--devices" option are for.
  This is mostly copied from the description in the Host Policies
  description (Layer 3 Examples).
- Explain that per-endpoint routing is not compatible with the host
  firewall and must be disabled, in particular on managed environments
  like GKE.
- Add node label removal to the clean-up section.
- Format shell session code as shell session code.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet force-pushed the pr/qmonnet/host_fw_gsg_fixes branch from 6556843 to 7ac2bb4 Compare October 22, 2020 08:42
@qmonnet
Copy link
Member Author

qmonnet commented Oct 22, 2020

Amended according to Paul's feedback.

Incremental diff
diff --git a/Documentation/gettingstarted/host-firewall.rst b/Documentation/gettingstarted/host-firewall.rst
index c4b3a704659d..535069f4d172 100644
--- a/Documentation/gettingstarted/host-firewall.rst
+++ b/Documentation/gettingstarted/host-firewall.rst
@@ -22,16 +22,16 @@ Enable the Host Firewall in Cilium
 
 Deploy Cilium release via Helm:
 
-.. code:: shell-session
+.. parsed-literal ::
 
-    $ helm install cilium |CHART_RELEASE|      \\
+    helm install cilium |CHART_RELEASE|        \\
       --namespace kube-system                  \\
       --set hostFirewall=true                  \\
       --set devices='{ethX,ethY}'
 
-The ``devices`` refer to the network devices Cilium is configured on such as
-``eth0``. Omitting this option leads Cilium to auto-detect what interfaces the
-host firewall applies to.
+The ``devices`` flag refers to the network devices Cilium is configured on such
+as ``eth0``. Omitting this option leads Cilium to auto-detect what interfaces
+the host firewall applies to.
 
 At this point, the Cilium-managed nodes are ready to enforce network policies.
 
@@ -51,11 +51,6 @@ At this point, the Cilium-managed nodes are ready to enforce network policies.
 Attach a Label to the Node
 ==========================
 
-.. note::
-
-    This must be done *after* deploying Cilium, or the agent may not pick up
-    the change.
-
 In this guide, we will apply host policies only to nodes with the label
 ``node-access=ssh``. We thus first need to attach that label to a node in the
 cluster.
@@ -87,8 +82,8 @@ recommended for production deployment*.
     $ HOST_EP_ID=$(kubectl -n $CILIUM_NAMESPACE exec $CILIUM_POD_NAME -- cilium endpoint list -o jsonpath='{[?(@.status.identity.id==1)].id}')
     $ kubectl -n $CILIUM_NAMESPACE exec $CILIUM_POD_NAME -- cilium endpoint config $HOST_EP_ID PolicyAuditMode=Enabled
     Endpoint 3353 configuration updated successfully
-    $ kubectl -n $CILIUM_NAMESPACE exec $CILIUM_POD_NAME -- cilium endpoint config $HOST_EP_ID -o jsonpath='{.realized.options.PolicyAuditMode}'
-    Enabled
+    $ kubectl -n $CILIUM_NAMESPACE exec $CILIUM_POD_NAME -- cilium endpoint config $HOST_EP_ID | grep PolicyAuditMode
+    PolicyAuditMode          Enabled
 
 
 Apply a Host Network Policy

@qmonnet qmonnet requested a review from pchaigno October 22, 2020 08:43
@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 22, 2020
@borkmann borkmann merged commit 251fd53 into master Oct 22, 2020
@borkmann borkmann deleted the pr/qmonnet/host_fw_gsg_fixes branch October 22, 2020 11:30
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.5 Oct 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.5 Oct 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.0-rc3 Oct 23, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.0-rc3 Oct 23, 2020
@joestringer joestringer moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.0-rc3 Oct 31, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.5 Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/host-firewall Impacts the host firewall or the host endpoint. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.5
Backport done to v1.8
1.9.0-rc3
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

6 participants