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

v1.9 backports 2020-12-15 #14419

Merged
merged 4 commits into from
Dec 16, 2020
Merged

v1.9 backports 2020-12-15 #14419

merged 4 commits into from
Dec 16, 2020

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Dec 15, 2020

Once this PR is merged, you can update the PR labels via:

$ for pr in 14329 14393 14381; do contrib/backporting/set-labels.py $pr done 1.9; done

[ upstream commit b8d65ad ]

[ Backporter's notes: Had to re-do vendoring @ v0.3.14 for branch ]

For integrating work in: google/gops#132

Resync'ed vendor directory to pull in latest gops via following:

  go get github.com/google/gops@latest && go mod vendor && go mod tidy && git add vendor/ go.mod go.sum && git commit -sa

Fixes #14332
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer requested a review from a team as a code owner December 15, 2020 19:42
@joestringer joestringer added backport/1.9 kind/backports This PR provides functionality previously merged into master. labels Dec 15, 2020
@joestringer joestringer marked this pull request as draft December 15, 2020 19:44
@maintainer-s-little-helper
Copy link

Commit 8fbb917a13b6e67b874c29122667b316ff3d2808 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 15, 2020
borkmann and others added 3 commits December 15, 2020 13:37
[ upstream commit 7757d31 ]

[ Backporter's notes: Resolved conflict in exit code for gops errors ]

Lee reported that kube-proxy log had a warning that its bind protection
couldn't bind a specific port in the nodeport range. Turns out gops was
using this particular port already through it's auto-binding (127.0.0.1:0).
Meaning that in case gops collides with a NodePort service, we might
not be able to pull gops data from that port since either kube-proxy or
kube-proxt free variant will redirect us to the actual service instead.

Given this is rather unpredictable wrt which port the agent will bind for
gops, remap it to a fixed default port and add a user configurable knob
that allows to use a different one if necessary. Given the agent, operator,
clustermesh-apiserver and hubble-relay all start the gops listener, add
the --gops-port flag to each of them. The CNI does not have gops enabled
by default but only in debug mode hence no changes there for now given
it's unlikely being used this way in production.

Fixes: #14218
Reported-by: Lee Hu via Slack
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Joe Stringer <joe@cilium.io>
[ upstream commit f3fa271 ]

The `K8sUpdates` deploys the Helm preflight check on top of an existing
older Cilium installation. To deploy the preflight check, the agent
must be disabled. The flag to disable the agent has been renamed in the
Cilium 1.9 charts. Therefore the upgrade test needs to set the proper
value.

This fixes an issue with the upgrade test on GKE where it fails as
follows:

```
coalesce.go:165: warning: skipped value for agent: Not a table.
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: kind: ResourceQuota, namespace: cilium, name: cilium-resource-quota
```

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Joe Stringer <joe@cilium.io>
[ upstream commit 5b911b8 ]

[ Backporter's notes: Minor conflict in go.sum, resolved by re-fetching
                      github.com/cilium/netlink and re-vendoring ]

Commit b6f2e75 set the xfrm output mark for the tunneling mode, but did
not set a mask. As a result, the packet mark is zeroed before writing
the new mark value, clearing our source security context.

In particular, this loss of security context happen when packets are
passed from the lxc devices to cilium_host via the stack. We then
receive a zero source identity in bpf_host and transmit it over the
tunnel to the destination nodes. That causes policy deny drops on remote
nodes when policies are enforced.

The bug can be observed by tracing the packets leaving the containers on
the source node. The source identity becomes unknown when it leaves
cilium_host (-> overlay trace event):

    <- endpoint 684 flow 0x10b975f7 identity 14765->unknown state new ifindex 0 orig-ip 0.0.0.0: 10.0.1.68:41856 -> 10.0.0.206:80 tcp SYN
    -> stack flow 0x10b975f7 identity 14765->30552 state new ifindex 0 orig-ip 0.0.0.0: 10.0.1.68:41856 -> 10.0.0.206:80 tcp SYN
    -> overlay flow 0x10b975f7 identity unknown->unknown state new ifindex cilium_vxlan orig-ip 0.0.0.0: 10.0.1.122 -> 10.0.0.184

This commit fixes the bug by setting the mask appropriately. That
requires us to temporarily use our cilium/netlink fork which has support
for setting the output mask [1], until that support is upstreamed.

1 - https://github.com/cilium/netlink/tree/outputmark-mask
Fixes: b6f2e75 ("cilium: set output mark for tunnel case")
Suggested-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Joe Stringer <joe@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 15, 2020
@joestringer joestringer marked this pull request as ready for review December 15, 2020 21:37
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

LG for my commit

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.

👍 for my PR.

I've checked that the IPSec fix works by cherry-picking and running the upcoming test (cf. #14412).

@pchaigno
Copy link
Member

test-backport-1.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants