clusterlink-net / clusterlink

A Gateway for connecting application services in different domains, networks, and cloud infrastructures
https://clusterlink.net
Other
17 stars 18 forks source link

CLI: Add zero bytes to empty certificates #638

Closed kfirtoledo closed 3 weeks ago

kfirtoledo commented 4 weeks ago

Add zero bytes to empty certificates that are used for cleanup by ClusterLink CLI.

elevran commented 4 weeks ago

can you explain why this is needed? Why can't you delete those instead of setting to zero?

kfirtoledo commented 4 weeks ago

In the cleanup phase, we delete all the secrets. For deletion is enough to know the name of the secret object, but there is still validation that runs on the secrets. This fix ignores the error validation of the empty secrets.

elevran commented 4 weeks ago

In the cleanup phase, we delete all the secrets. For deletion is enough to know the name of the secret object, but there is still validation that runs on the secrets. This fix ignores the error validation of the empty secrets.

I don't understand why there's validation on a secret that's deleted. Who is validating? ClusterLink, Envoy, K8s? The secrets are mounted into Deployment volumes, might this be the problem? Perhaps, you can't delete a secret that is mounted and that's why it needs to hold a value? If that's the case, then we need to make sure we delete the objects in reverse order of creation: secrets were created before Pods so deletion should be first CL Pods/Deployments and then secrets after they're no longer needed.

So while I understand it works with the zero value, not knowing the reason why we get the error, feels to me as if we're masking a failure instead of fixing it. Perhaps we need a different function in platform/k8s to delete the entire deployment and related object instead of creating a zero value set of secrets?

kfirtoledo commented 3 weeks ago

@elevran, This code was accidentally deleted in the TLS refactoring PR https://github.com/clusterlink-net/clusterlink/pull/619/files#:~:text=%22fabricCA%22%3A%20%20%20%20%20%20%20%20%20%22%22,dataplaneKey%22%3A%20%20%20%20%20%22%22%2C

Without this code, the secret looks:

  ---
  apiVersion: v1
  kind: Secret
  metadata:
    name: cl-ca
    namespace: <no value>
  data:
    ca: <no value>
  ---
  apiVersion: v1
  kind: Secret
  metadata:
    name: cl-controlplane
    namespace: <no value>
  data:
    cert: <no value>
    key: <no value>
  ---

Those are not valid certificates. I can create a secret object for each one and delete it, But I prefer to reuse the code and delete it in the same way as the secrets were created. The validation is done as part of the package sigs.k8s.io/e2e-framework/klient/k8s/resources, because this type of secret is not legal. About the order of the deletion, it doesn't matter because we use the DeleteIgnoreNotFound function.