Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Update calico to v3.14.1 #415

Merged
merged 4 commits into from
May 29, 2020
Merged

Update calico to v3.14.1 #415

merged 4 commits into from
May 29, 2020

Conversation

surajssd
Copy link
Member

@surajssd surajssd commented May 11, 2020 •
edited
Loading

NOTE: See the commits one at a time to understand what this PR does in detail.

  • Update to v3.14.0.
  • Remove the hostendpoint controller.
  • Leverage the default kube controllers shipped by Calico.

Fixes #341

TODOs:

  • Manual testing to see if the HostEndpoints created work.
  • See if these tests can be automated.

@surajssd surajssd requested a review from invidian May 11, 2020 17:11
@iaguis
Copy link
Contributor

iaguis commented May 11, 2020

I'm not sure we can get rid of the hostendpoint controller since it also handles VPN interfaces, which I don't think we can translate to Calico.

@invidian
Copy link
Member

Manual testing to see if the HostEndpoints created work.
See if these tests can be automated.

Those tests exist and it seems they pass? See https://github.com/kinvolk/lokomotive/blob/master/test/calico/calico_test.go

=== RUN   TestHostEndpoints
time="2020-05-11T17:31:47Z" level=info msg="Using Calico IPAM"
    TestHostEndpoints: util.go:55: using KUBECONFIG=/root/lokoctl-assets/cluster-assets/auth/kubeconfig
--- PASS: TestHostEndpoints (0.72s)

I'd still do a manual test, just to be sure 😄

@surajssd
Copy link
Member Author

I'm not sure we can get rid of the hostendpoint controller since it also handles VPN interfaces, which I don't think we can translate to Calico.

Can we restrict the calico-hostendpoint-controller controller to work only on vpn interface? And this controller can be enabled by special variable flag?

Having two controllers simultaneously can cause conflicts?

@invidian
Copy link
Member

Having two controllers simultaneously can cause conflicts?

It shouldn't be a huge deal, worst case you get some errors that the resource already exist.

Can we restrict the calico-hostendpoint-controller controller to work only on vpn interface? And this controller can be enabled by special variable flag?

That must be implemented here I guess: https://github.com/kinvolk/calico-hostendpoint-controller/blob/master/run#L98

@surajssd
Copy link
Member Author

I have two nodes:

$ kg nodes
NAME                               STATUS   ROLES    AGE   VERSION
suraj-lk-cluster-controller-0      Ready    <none>   10m   v1.18.2
suraj-lk-cluster-pool-1-worker-0   Ready    <none>   10m   v1.18.2

And I have allowed ssh access only from my home public IP 106.51.107.95:

$ kg globalnetworkpolicy ssh -o yaml
apiVersion: crd.projectcalico.org/v1
kind: GlobalNetworkPolicy
metadata:
  name: ssh
spec:
  applyOnForward: true
  ingress:
  - action: Allow
    destination:
      ports:
      - 22
    protocol: TCP
    source:
      nets:
      - 106.51.107.95
  order: 0
  preDNAT: true
  selector: has(host-endpoint)

And after deploying this PR following hostendpoints are created automatically.

$ kg hostendpoints -o yaml
apiVersion: v1
items:
- apiVersion: crd.projectcalico.org/v1
  kind: HostEndpoint
  metadata:
    annotations:
    labels:
      beta.kubernetes.io/arch: amd64
      beta.kubernetes.io/os: linux
      kubernetes.io/arch: amd64
      kubernetes.io/hostname: suraj-lk-cluster-controller-0
      kubernetes.io/os: linux
      lokomotive.alpha.kinvolk.io/public-ipv4: 139.1.1.63
      node.kubernetes.io/controller: "true"
      node.kubernetes.io/master: ""
      projectcalico.org/created-by: calico-kube-controllers
    name: suraj-lk-cluster-controller-0-auto-hep
  spec:
    expectedIPs:
    - 10.88.81.1
    - 10.2.82.0
    interfaceName: '*'
    node: suraj-lk-cluster-controller-0
    profiles:
    - projectcalico-default-allow
- apiVersion: crd.projectcalico.org/v1
  kind: HostEndpoint
  metadata:
    annotations:
    labels:
      beta.kubernetes.io/arch: amd64
      beta.kubernetes.io/os: linux
      kubernetes.io/arch: amd64
      kubernetes.io/hostname: suraj-lk-cluster-pool-1-worker-0
      kubernetes.io/os: linux
      lokomotive.alpha.kinvolk.io/public-ipv4: 139.1.1.91
      metallb.universe.tf/my-asn: "65000"
      metallb.universe.tf/peer-address: 10.88.81.2
      metallb.universe.tf/peer-asn: "65530"
      node.kubernetes.io/node: ""
      projectcalico.org/created-by: calico-kube-controllers
    name: suraj-lk-cluster-pool-1-worker-0-auto-hep
  spec:
    expectedIPs:
    - 10.88.81.3
    - 10.2.153.0
    interfaceName: '*'
    node: suraj-lk-cluster-pool-1-worker-0
    profiles:
    - projectcalico-default-allow
kind: List
metadata:

But I am still able to ssh from non-whitelisted IP:

$ ssh core@139.1.1.91
Last login: Fri May 15 12:49:44 UTC 2020 from 136.8.7.17 on pts/0
Flatcar Container Linux by Kinvolk stable (2247.7.0)
Update Strategy: No Reboots
Failed Units: 1
  tcsd.service
core@suraj-lk-cluster-pool-1-worker-0 ~ $

What could be wrong here?

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 19671f4 to 79e4114 Compare May 15, 2020 12:58
@iaguis
Copy link
Contributor

iaguis commented May 15, 2020

From https://docs.projectcalico.org/reference/resources/hostendpoint:

Calico provides a default profile resource named projectcalico-default-allow that consists of allow-all ingress and egress rules. Host endpoints with the projectcalico-default-allow profile attached will have “allow-all” semantics instead of “deny-all” in the absence of policy.

Note: If you have custom iptables rules, using host endpoints with allow-all rules (with no policies) will accept all traffic and therefore bypass those custom rules.

and

Auto host endpoints specify the projectcalico-default-allow profile so they behave similary to pod workload endpoints.

So maybe we need a deny all rule and then our own allow GNP rules should have precedence.

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 79e4114 to 53222be Compare May 18, 2020 01:08
@surajssd surajssd mentioned this pull request May 18, 2020
Open
@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 53222be to f134554 Compare May 18, 2020 14:22
@invidian
Copy link
Member

@surajssd rebase is needed here. Maybe we can just do the update here and don't remove anything for now?

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from f134554 to a7a7213 Compare May 22, 2020 13:16
@surajssd
Copy link
Member Author

@invidian

@surajssd rebase is needed here. Maybe we can just do the update here and don't remove anything for now?

We still need to make sure that the newly created HostendPoints don't break the security of the nodes?

@invidian
Copy link
Member

We still need to make sure that the newly created HostendPoints don't break the security of the nodes?

Yes, I guess so. Though IIRC, the controller works by using the wildcard in HostEndpoint object, so as we don't specify it, the behavior should not change for now.

@surajssd
Copy link
Member Author

Yes, I guess so. Though IIRC, the controller works by using the wildcard in HostEndpoint object, so as we don't specify it, the behavior should not change for now.

The wildcard is not breaking the security but the profile that it adds automatically.

@invidian
Copy link
Member

I'm not sure I understand. Can you please elaborate?

@surajssd
Copy link
Member Author

Yes, I guess so. Though IIRC, the controller works by using the wildcard in HostEndpoint object, so as we don't specify it, the behavior should not change for now.

The wildcard is not breaking the security but the profile that it adds automatically.

By wildcard I meant that it is being applied on all the interfaces (see following example, interfaceName).

$ kg hostendpoints -o yaml
apiVersion: v1
items:
- apiVersion: crd.projectcalico.org/v1
  kind: HostEndpoint
  metadata:
    annotations:
    labels:
      beta.kubernetes.io/arch: amd64
      beta.kubernetes.io/os: linux
      kubernetes.io/arch: amd64
      kubernetes.io/hostname: suraj-lk-cluster-controller-0
      kubernetes.io/os: linux
      lokomotive.alpha.kinvolk.io/public-ipv4: 139.1.1.63
      node.kubernetes.io/controller: "true"
      node.kubernetes.io/master: ""
      projectcalico.org/created-by: calico-kube-controllers
    name: suraj-lk-cluster-controller-0-auto-hep
  spec:
    expectedIPs:
    - 10.88.81.1
    - 10.2.82.0
    interfaceName: '*'
    node: suraj-lk-cluster-controller-0
    profiles:
    - projectcalico-default-allow
- apiVersion: crd.projectcalico.org/v1
  kind: HostEndpoint
  metadata:
    annotations:
    labels:
      beta.kubernetes.io/arch: amd64
      beta.kubernetes.io/os: linux
      kubernetes.io/arch: amd64
      kubernetes.io/hostname: suraj-lk-cluster-pool-1-worker-0
      kubernetes.io/os: linux
      lokomotive.alpha.kinvolk.io/public-ipv4: 139.1.1.91
      metallb.universe.tf/my-asn: "65000"
      metallb.universe.tf/peer-address: 10.88.81.2
      metallb.universe.tf/peer-asn: "65530"
      node.kubernetes.io/node: ""
      projectcalico.org/created-by: calico-kube-controllers
    name: suraj-lk-cluster-pool-1-worker-0-auto-hep
  spec:
    expectedIPs:
    - 10.88.81.3
    - 10.2.153.0
    interfaceName: '*'
    node: suraj-lk-cluster-pool-1-worker-0
    profiles:
    - projectcalico-default-allow
kind: List
metadata:

So the wildcard entry for interfaces is not breaking the security. But the profiles section is breaking the secrurity.

...
profiles:
- projectcalico-default-allow
...

Like I have asked in the issue on the upstream projectcalico/calico#3566, how do we manipulate the CR so that we can either disable any policy in there or add a way to speacify a HostEndpoint.

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 1040e1c to 631fc0c Compare May 26, 2020 23:52
@surajssd
Copy link
Member Author

Now that we are not removing our hostendpoint controller, I see that the security is intact.

$ ssh -v core@17.5.9.7
OpenSSH_7.6p1 Ubuntu-4ubuntu0.3, OpenSSL 1.0.2n  7 Dec 2017
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: /etc/ssh/ssh_config line 19: Applying options for *
debug1: Connecting to 17.5.9.7 [17.5.9.7] port 22.
debug1: connect to address 17.5.9.7 port 22: Connection timed out
ssh: connect to host 17.5.9.7 port 22: Connection timed out

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 631fc0c to 1cf92df Compare May 26, 2020 23:54
@surajssd
Copy link
Member Author

I think this PR is ready for final review, and can be merged eventually.

What I am confused about is how can we automate testing for this?

@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch 2 times, most recently from a93b211 to 74c4119 Compare May 28, 2020 07:46
invidian
invidian previously approved these changes May 28, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM

@surajssd surajssd requested a review from iaguis May 28, 2020 08:49
@surajssd
Copy link
Member Author

@iaguis PTAL

@invidian
Copy link
Member

3.14.1 is out now. We should update before merging.

@surajssd
calico: Update to v3.14.1
9ee8295 
This commit updates calico from `v3.13.3` to `v3.14.1`. Release Notes:
https://github.com/projectcalico/calico/releases/tag/v3.14.1.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
calico: Update RBAC permissions of the controllers
d69b092 
This commit updates the ClusterRole `calico-kube-controllers` with
enhanced permissions.

It allows the access to CRD `hostendpoints` and
`kubecontrollersconfigurations`.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
calico: Enable kube-controller to create HostEndpoint
6f40224 
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
calico: Add CRD KubeControllersConfiguration
e0a766d 
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd surajssd force-pushed the surajssd/update-calico-3.14.0 branch from 74c4119 to e0a766d Compare May 29, 2020 10:39
@surajssd
Copy link
Member Author

3.14.1 is out now. We should update before merging.

Is done.

@invidian invidian changed the title Update calico to v3.14.0 Update calico to v3.14.1 May 29, 2020
invidian
invidian approved these changes May 29, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Thanks @surajssd !

@surajssd
Copy link
Member Author

@iaguis PTAL.

iaguis
iaguis approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

lgtm

@surajssd surajssd merged commit 017f6e9 into master May 29, 2020
@surajssd surajssd deleted the surajssd/update-calico-3.14.0 branch May 29, 2020 12:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@iaguis iaguis iaguis approved these changes

@invidian invidian invidian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update Calico to 3.14.0
3 participants
@surajssd @iaguis @invidian