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

docs: add design for adding new arguments for TLS #173

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions docs/design/podService.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,36 @@ CSI addons sidecar which is being deployed here needs certificates. Currently we

## Proposed Solution

### Introduce a new argument for TLS

We introduce a new argument in the commands to enable TLS. It is diabled by default. But if this is enabled the deployer is expected to have mounted a secret that contains the required certificates. We will essentially need a certifiate that is compatible with the hostname that the manager will be issuing network calls using it.
### Introduce a new argument for TLS

We introduce a new argument in the commands to enable TLS. It is diabled by default. But if this is enabled the deployer is expected to have mounted a secret that contains the required certificates. We will essentially need a certifiate that is compatible with the hostname that the manager will be issuing network calls using it.

### Operator changes

The Ceph CSI Operator is only responsible for taking in the information mounted to it and project those as volumes in the CSI Addons sidecar. The deployer of CSI Addons should create these certificates and mount it at `/etc/tls/tls.crt` and `/etc/tls/tls.key`. We keep this hardcoded to reduce the number of new arguments introduced. The logger should provide enough information if the user is not mounting this correctly for easy debugging.
The Ceph CSI Operator when deploying the sidecar for CSI Addons should will the secret provided in the OperatorConfig in `/etc/tls` folder. The infomration about the secret should be provided via OperatorConfig which in then will be used during sidecar creation. The secret contains information the certficate that is going to be used by the CSI Addons sidecar.

## Guide to the operator incharge of creating certificates (Optional)

Based on the type of networking that is to be used. Hostnames used to connect to the Sidecar endpoint should be set in the certificate accordingly.

## Changes proposed to OperatorConfig

```
// New struct proposed
type TLSConfiguration struct {
certificateSecretname string
cretificateSecretNamespace string
Comment on lines +30 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is just a Namespaced Name. But I would prefer to use a LocalObjectReference instead

enableTLS bool
Copy link
Collaborator

@nb-ohad nb-ohad Dec 5, 2024

Choose a reason for hiding this comment

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

Why do we need an enable flag? The existance of the section is a good enough indication that we should use TLD

}

## Guide to the deployeer for handling certificates
// OperatorConfigSpec defines the desired state of OperatorConfig
type OperatorConfigSpec struct {
//+kubebuilder:validation:Optional
Log *OperatorLogSpec `json:"log,omitempty"`

Since we use host networking the certificates should have these IP addresses as valid Subject names.
// Allow overwrite of hardcoded defaults for any driver managed by this operator
//+kubebuilder:validation:Optional
DriverSpecDefaults *DriverSpec `json:"driverSpecDefaults,omitempty"`
TLSConfiguration *TLSConfiguration // Conigure TLS certificates for CSI addons
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this needs to have a Driver scope, not an operator scope. The decision to use or not use TLS should be determined on a driver basis

}
```