Add support for Calico, use RBAC and secrets encryption. #283

21repierre · 2023-07-18T09:10:54Z

Add support for Calico (CNI: Consider switching from Flannel to Calico (VXLAN mode) #152)
Starts kube-apiserver with RBAC (launch kube-apiserver with --authorization-mode=Node,RBAC #166)
Use secretbox encryption for secrets by default

Tested with a single node cluster on Centos 9.

install.sh

AkihiroSuda · 2023-07-20T05:36:00Z

manifests/calico/tigera-operator.yml

Can't review this amount of YAML 😓

Can we just curl this YAML from the upstream?
https://github.com/projectcalico/calico/blob/v3.26.1/manifests/tigera-operator.yaml

Absolutly, it actually comes from this repo

This should be gitignored and curled on make.

AkihiroSuda · 2023-07-20T05:36:48Z

manifests/secrets-encryption.yml

+ providers:
+ - secretbox:
+ keys:
+ - name: key1


How is this key consumed?

According to the documentation the key should be used by kube-apiserver to encrypt the secrets.

Can we have more descriptive name?

I have confirmed we can use something better, currently I set it to secrets_key.
Do you have something in mind that would be better ?

AkihiroSuda · 2023-07-20T05:37:17Z

install.sh

@@ -51,7 +51,7 @@ function usage() {
 echo
 echo " --start=UNIT Enable and start the specified target after the installation, e.g. \"u7s.target\". Set to an empty to disable autostart. (Default: \"$start\")"
 echo " --cri=RUNTIME Specify CRI runtime, \"containerd\" or \"crio\". (Default: \"$cri\")"
- echo ' --cni=RUNTIME Specify CNI, an empty string (none) or "flannel". (Default: none)'
+ echo ' --cni=RUNTIME Specify CNI, an empty string (none), \"calico\" or "flannel". (Default: none)'


Can we test calico in CI ?

usernetes/.github/workflows/main.yaml

Line 24 in 58df6ea

- name: "Smoke test (multi-node cluster with Flannel)"

We should be able to, I have never used Github CI but I can look into it.

AkihiroSuda · 2023-07-20T05:38:24Z

boot/kube-apiserver.sh

@@ -19,6 +19,8 @@ exec $(dirname $0)/nsenter.sh kube-apiserver \
 --service-account-signing-key-file=$XDG_CONFIG_HOME/usernetes/master/service-account-key.pem \
 --advertise-address=$(cat $XDG_RUNTIME_DIR/usernetes/parent_ip) \
 --allow-privileged \
+ --encryption-provider-config=$XDG_CONFIG_HOME/usernetes/master/secrets-encryption.yml \
+ --authorization-mode=Node,RBAC \


Could you split and squash the commits so that one commit has one topic?

Probably, this PR should have the following three commits:

RBAC

Secrets encryption

Calico

How should I handle install.sh as it has edits for RBAC, Calico and secrets encryption ?

Just make commit 3 depend on commit 1 and 2?

AkihiroSuda · 2023-07-20T14:48:06Z

manifests/secrets-encryption.yml

The suffix should be something like .yaml.template, as this is not a complete manifest

AkihiroSuda · 2023-07-20T14:48:32Z

manifests/secrets-encryption.yml

+ - secretbox:
+ keys:
+ - name: key1
+ secret: 


Can we use envsubst or something for filling up the secret value?

AkihiroSuda · 2023-07-20T16:09:33Z

README.md

@@ -129,7 +129,7 @@ iptable_filter
 ip6table_filter
 nf_tables
 x_tables
-xt_MASQUERADE
+xt_MASQUERADE # on older kernels: ipt_MASQUERADE and ip6t_MASQUERADE


A comment should be a separate line for ease of parsing

Makefile

README.md

AkihiroSuda · 2023-07-21T08:53:12Z

install.sh

@@ -51,7 +51,7 @@ function usage() {
 echo
 echo " --start=UNIT Enable and start the specified target after the installation, e.g. \"u7s.target\". Set to an empty to disable autostart. (Default: \"$start\")"
 echo " --cri=RUNTIME Specify CRI runtime, \"containerd\" or \"crio\". (Default: \"$cri\")"
- echo ' --cni=RUNTIME Specify CNI, an empty string (none) or "flannel". (Default: none)'
+ echo ' --cni=RUNTIME Specify CNI, an empty string (none), \"calico\" or "flannel". (Default: none)'


Could you update README.md too?

Signed-off-by: Pierre Boudvillain <[email protected]>

AkihiroSuda · 2023-08-05T17:39:04Z

Do you plan to add a test?
#283 (comment)

21repierre · 2023-08-18T09:28:41Z

Do you plan to add a test? #283 (comment)

I have tried to work on that by making a similar test to the flannel one (replacing flannel by calico in the docker-compose file and publishing ports according to the calico requirements), however I wasn't able to make it work.
It seems that the nodes can't reach the master:

Logs from calico-kube-controller running on node-crio: 2023-08-18 08:48:29.468 [INFO][1] main.go 138: Failed to initialize datastore error=Get "https://10.0.0.1:443/apis/crd.projectcalico.org/v1/clusterinformations/default": dial tcp 10.0.0.1:443: i/o timeout
Logs from coredns running on node-containerd: [ERROR] plugin/errors: 2 6277665988150219199.6938599929280645902. HINFO: read udp 10.0.100.194:37370->10.0.102.3:53: i/o timeout
Trying to curl the kubernetes API from a node also results on a timeout: curl: (7) Failed to connect to 10.0.100.100 port 6443 after 94953 ms: Couldn't connect to server

I'm not sure where the issue might come from.

21repierre force-pushed the master branch from 00229a6 to d0a7733 Compare July 18, 2023 09:28

AkihiroSuda reviewed Jul 20, 2023

View reviewed changes

install.sh Outdated Show resolved Hide resolved

AkihiroSuda reviewed Jul 20, 2023

View reviewed changes

manifests/secrets-encryption.yml Outdated

Copy link

Member

AkihiroSuda Jul 20, 2023

Choose a reason for hiding this comment

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

The suffix should be something like .yaml.template, as this is not a complete manifest

AkihiroSuda reviewed Jul 20, 2023

View reviewed changes

21repierre force-pushed the master branch from 1209970 to b0421a6 Compare July 21, 2023 08:43

AkihiroSuda reviewed Jul 21, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

AkihiroSuda reviewed Jul 21, 2023

View reviewed changes

README.md Show resolved Hide resolved

AkihiroSuda reviewed Jul 21, 2023

View reviewed changes

Add RBAC

2f40065

Signed-off-by: Pierre Boudvillain <[email protected]>

21repierre force-pushed the master branch from b0421a6 to da94dbb Compare July 21, 2023 09:43

21repierre added 3 commits July 21, 2023 17:04

Add secrets encryption

2d6ca2f

Signed-off-by: Pierre Boudvillain <[email protected]>

Add Calico

199275e

Signed-off-by: Pierre Boudvillain <[email protected]>

Update README

a1c5e12

Signed-off-by: Pierre Boudvillain <[email protected]>

21repierre force-pushed the master branch from da94dbb to a1c5e12 Compare July 21, 2023 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Calico, use RBAC and secrets encryption. #283

Add support for Calico, use RBAC and secrets encryption. #283

21repierre commented Jul 18, 2023

AkihiroSuda Jul 20, 2023

21repierre Jul 20, 2023

AkihiroSuda Jul 20, 2023

AkihiroSuda Jul 20, 2023

21repierre Jul 20, 2023

AkihiroSuda Jul 20, 2023

21repierre Jul 21, 2023

AkihiroSuda Jul 20, 2023

21repierre Jul 20, 2023

AkihiroSuda Jul 20, 2023

21repierre Jul 20, 2023

AkihiroSuda Jul 20, 2023

AkihiroSuda Jul 20, 2023

AkihiroSuda Jul 20, 2023

AkihiroSuda Jul 20, 2023

AkihiroSuda Jul 21, 2023

AkihiroSuda commented Aug 5, 2023

21repierre commented Aug 18, 2023

Add support for Calico, use RBAC and secrets encryption. #283

Are you sure you want to change the base?

Add support for Calico, use RBAC and secrets encryption. #283

Conversation

21repierre commented Jul 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented Aug 5, 2023

21repierre commented Aug 18, 2023