Kopia Integration: Unified Repository Provider - Repo Password #5167
Conversation
Lyndon-Li
changed the title
Lyndon-Li
force-pushed
the
udmrepo-dev-01
branch
from
b1464d1 to
69b32ff
Compare
| credentialsSecretName = "velero-restic-credentials" | ||
| credentialsKey = "repository-password" | ||
|
|
||
| encryptionKey = "static-passw0rd" |
There was a problem hiding this comment.
which means that our kopia repo paasword is plain text and invariable？ and which may lead to some security issue?
There was a problem hiding this comment.
This is the existing code and file, this keys.go is moved from Restic package to the repository package.
As the agreement, in v1.10, we will keep the existing behaviors for Kopia integration.
In future, as you can see, the provider construct accepts a CredentialGetter, in which, we can add whatever implementation to get a password. For example, we may get repo password from KMS in future, then we can add something like KmsStore interface into it.
internal/credentials/secret_store.go
Outdated
| } | ||
|
|
||
| // Buffer returns the secret key defined by the given selector. | ||
| func (n *namespacedSecretStore) Buffer(selector *corev1api.SecretKeySelector) (string, error) { |
There was a problem hiding this comment.
whether we should change it to one easily understood function name instead of Buffer? this function is to get a secret for a specific namespace
There was a problem hiding this comment.
This Buffer means getting a buffer stream, since the interface has already named as SecretStore, we know that it is to get the stream buffer from Secret, so no need to add something like "secret" in the method name. This is also to comply with FileStore's Path method.
As the discuss, we leave it as it in this PR. Later, if there is any conflict or better name, we come back to change it.
|
|
||
| // SecretStore defines operations for interacting with credentials | ||
| // that are stored in Secret. | ||
| type SecretStore interface { |
There was a problem hiding this comment.
Seems it isn't a store? It's just a secret getter and stores nothing. Can we call kubeClient.Get() to get the secret directly without defining this interface?
If it's needed, I have the same comment with @qiuming-best , the function name Buffer confuses me
There was a problem hiding this comment.
We add this SecretStore in parallel with the existing FileStore, the FileStore gets the password from Secret then save to a temp file; SecretStore gets the password from Secret and then return a stream buffer.
We don't call kubeClient.Get() directly for below purpose:
- We may get the password/credential keys from different sources, for example, from a temp file, from memory, from KMS, so we wrap the interfaces for all the sources in the
CredentialGetterstructure. - At present, in Unified Repo, we are using repo password got from memory, cloud provider credentials from a temp file
- In future, we may add a new interface called something like KmsStore
There was a problem hiding this comment.
For the method name of SecretStore, changed from Buffer to Get
Signed-off-by: Lyndon-Li <[email protected]>
Lyndon-Li
force-pushed
the
udmrepo-dev-01
branch
from
69b32ff to
335082c
Compare
Signed-off-by: Lyndon-Li <[email protected]>
Add changes for Kopia Integration: Unified Repository Provider - Repo Password
Related Issue: #5079