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

Ensure Antrea works in any namespace (remove any kube-system hard-coding)? #876

Closed
jayunit100 opened this issue Jun 26, 2020 · 6 comments · Fixed by #878
Closed

Ensure Antrea works in any namespace (remove any kube-system hard-coding)? #876

jayunit100 opened this issue Jun 26, 2020 · 6 comments · Fixed by #878
Assignees

Comments

@jayunit100
Copy link
Contributor

jayunit100 commented Jun 26, 2020

Running antrea in kube-system works fine right now, but the external certs are expected to be in kube-system ns (context here is that secrets living in kube-sys means that operations like rotating them means folks would need to muck w/ kube-system, which is undesirable).

Lets remove the hardcodings to kube-system whereever possible and instead just look up the local namespace of the antrea/// i think you can do this w/ client.Namespace in kubernetes.

https://github.com/vmware-tanzu/antrea/blob/master/docs/securing-control-plane.md#using-cert-manager)

https://github.com/vmware-tanzu/antrea/blob/master/pkg/apiserver/certificate/certificate.go#L48

docs/securing-control-plane.md

@ncdc
Copy link

ncdc commented Jun 26, 2020

Point of clarification: the external certs are expected to be in kube-system if you're providing them using a secret. Rotation is orthogonal.

I think we had previously discussed the contract between Antrea and user-supplied certificates to be a path in the filesystem containing ca.crt, tls.crt, and tls.key. In fact, I think that's what I'm seeing in the code:

https://github.com/vmware-tanzu/antrea/blob/aafd76e8f0e158043393419a9d3f7d1e41927eb5/pkg/apiserver/certificate/certificate.go#L34

and

https://github.com/vmware-tanzu/antrea/blob/aafd76e8f0e158043393419a9d3f7d1e41927eb5/pkg/apiserver/certificate/certificate.go#L74-L97

The TLSSecretNamespace and TLSSecretName constants appear only to be used for test code and in error messages.

I would recommend modifying the error message in

https://github.com/vmware-tanzu/antrea/blob/aafd76e8f0e158043393419a9d3f7d1e41927eb5/pkg/apiserver/certificate/certificate.go#L90

to not mention anything about a Secret, and instead indicate there were issues with the expected files in /var/run/antrea/antrea-controller-tls

I did, however, find some other non-test code in the code base that is using kube-system:

https://github.com/vmware-tanzu/antrea/blob/aafd76e8f0e158043393419a9d3f7d1e41927eb5/pkg/apiserver/certificate/cacert_controller.go#L35

Is this controller used if you BYO certificates?

@jayunit100 could you please retitle this to be more generic: Ensure Antrea works in any namespace (remove any kube-system hard-coding)?

@jianjuns jianjuns self-assigned this Jun 26, 2020
@jayunit100 jayunit100 changed the title Allow certificates to live outside kube-system namespace Ensure Antrea works in any namespace (remove any kube-system hard-coding)? Jun 26, 2020
@antoninbas
Copy link
Contributor

Yes, looks like it will work as long as you create the secret in the same namespace as Antrea.

The cacert controller is used regardless of whether you BYO CA cert or you let Antrea generate one. It is how we publish it to the Antrea agent in a uniform fashion. When you BYO, the cacert controller will just publish it directly using the antrea-ca ConfigMap in the kube-system namespace. To resolve this issue, this is probably what we need to change. The namespace should be whichever one Antrea is running in.

We may need to adjust some pieces of the documentation to mention that the provided instructions only apply when using the manifest "as is", and that the namespace in which the secret is created (e.g. by cert-provider) must match the namespace in which Antrea is running.

@jianjuns
Copy link
Contributor

You guys are right. I checked securing-control-plane.md. It just uses kube-system as an example. Actually many our docs follow this way and use kube-system in examples. Probably let us just add a note in securing-control-plane.md, that it should be the namespace antrea deploys.

@antoninbas
Copy link
Contributor

@jianjuns we still need to create the antrea-ca configMap in the correct namespace

@jayunit100
Copy link
Contributor Author

jayunit100 commented Jun 26, 2020

Its by default created in the correct ns on master ? so seems like the default creation behaviour for self signed certs is correct.?

(⎈ |kind-antrea:default)➜  cluster-api-provider-vsphere git:(master) ✗ kubectl get cm -n tkg-system
NAME                       DATA   AGE
antrea-ca                  0      142m
antrea-config-cd7tt4t2f8   3      142m

@antoninbas
Copy link
Contributor

Sorry I misspoke: yes it will be created in the correct namespace, but the cacert controller that Andy pointed to will look for it in the kube-system namespace when trying to update it, so the contents will be wrong.

jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 26, 2020
No more use the fixed "kube-system" Namespace for the CA ConfigMap.
Also updated the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 26, 2020
No more use the fixed "kube-system" Namespace for the CA ConfigMap.
Also updated the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 26, 2020
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 27, 2020
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 27, 2020
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 28, 2020
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit to jianjuns/antrea that referenced this issue Jun 29, 2020
Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
jianjuns added a commit that referenced this issue Jun 30, 2020
#878)

Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: #876
GraysonWu pushed a commit to GraysonWu/antrea that referenced this issue Sep 22, 2020
antrea-io#878)

Stop using the fixed "kube-system" Namespace for the CA ConfigMap.
Also update the deployment YAML and docs/securing-control-plane.md
about the descriptions about CA ConfigMap and TLS Secret Namespace.

Fixes: antrea-io#876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants