Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: address containerd errata #2865

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

jackfrancis
Copy link
Member

Reason for Change:

This PR addresses three observed issues w/ the Azure-curated containerd implementation:

  • Ensure that we are using the v2 configuration on agent as well as master nodes
  • Ensure that we configure iptables for IP forwarding
  • Ensure that we configure (via sysctl) the IPv4 stack for IP forwarding

Issue Fixed:

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

Merging #2865 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2865   +/-   ##
=======================================
  Coverage   72.46%   72.46%           
=======================================
  Files         140      140           
  Lines       25675    25675           
=======================================
  Hits        18606    18606           
  Misses       5993     5993           
  Partials     1076     1076

owner: root
content: |
[Service]
ExecStartPre=/sbin/iptables -P FORWARD ACCEPT
Copy link
Member

Choose a reason for hiding this comment

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

WDYT if instead of changing the default forwarding policy if we do something like:

-A FORWARD -i azure0 -o azure0 -j ACCEPT
-A FORWARD -i azure0 ! -o azure0 -j ACCEPT

This way we only forward packets for azure0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, let's do this as a follow-up for both docker and containerd (as far as I can tell this additional targeted hardening is appropriate for any CRI).

@jackfrancis jackfrancis merged commit a4ea1ca into Azure:master Mar 10, 2020
@jackfrancis jackfrancis deleted the containerd-v2-fixes branch March 10, 2020 18:31
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants