weave-npc to check local addresses only #2979
Conversation
bboreham
changed the title
npc/controller.go
Outdated
| @@ -86,6 +89,10 @@ func (npc *controller) AddPod(obj *coreapi.Pod) error { | |||
| npc.Lock() | |||
| defer npc.Unlock() | |||
|
|
|||
| if npc.nodeName != "" && obj.Spec.NodeName != npc.nodeName { | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -92,6 +92,11 @@ spec: | |||
| cpu: 10m | |||
| securityContext: | |||
| privileged: true | |||
| env: | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -92,6 +92,11 @@ spec: | |||
| cpu: 10m | |||
| securityContext: | |||
| privileged: true | |||
| env: | |||
| - name: MY_NODE_NAME | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
There was a problem hiding this comment.
This makes a lot of sense and LGTM.
- just a couple nitpicks and
- waiting for it to be rebased after Add a network test that isn't ping #2918 has been merged & will merge in turn.
|
Note that our linter seems to complain about: |
bboreham
force-pushed
the
issues/2622-npc-local-node-only
branch
2 times, most recently
from
af47eba to
152e9a4
Compare
This means we only filter connections at the destination end, which is a better match for the ingress-only rules.
bboreham
force-pushed
the
issues/2622-npc-local-node-only
branch
from
152e9a4 to
8a1f8d5
Compare
| nodeName = os.Getenv("HOSTNAME") | ||
| } | ||
| if nodeName == "" { | ||
| common.Log.Fatalf("Must set node name via --node-name or $HOSTNAME") |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Cherry-picked to 1.9 branch at 2d3e070 |
| func root(cmd *cobra.Command, args []string) { | ||
| common.SetLogLevel(logLevel) | ||
| common.Log.Infof("Starting Weaveworks NPC %s", version) | ||
| if nodeName == "" { | ||
| // HOSTNAME is set by Kubernetes for pods in the host network namespace |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This branch is based off the branch for #2978, because I need the rule to contact kube-dns.
Only the last four commits are relevant to this PR.
This PR does two things: it fixes #2622 by only adding IP addresses from pods on the current node; and it then corrects a bug exposed by that change: weave-npc used to filter on the destination address at both ends of a connection.
I suspect this is the cause of #2973.
To address that, I create a new IPset which has all local pod addresses, and if a new connection comes from a local pod but does not target a local pod we let it through. In this way all the real filtering is done at the destination end of the connection.