nlreturn linter requires a new line before return
and branch statements except when the return is alone
inside a statement group (such as an if statement) to
increase code clarity. This commit addresses such issues.
Updates: #1586
Signed-off-by: Rakshith R <rar@redhat.com>
This commit resolves errorlint issues
which checks for the code that will cause
problems with the error wrapping scheme.
Updates: #1586
Signed-off-by: Yati Padia <ypadia@redhat.com>
snapshot controller make sure the pvc which is the source for the
snapshot request wont get deleted while snapshot is getting created,
so we dont need to check for any ongoing delete operation here on the
volume.
Subjected code path in snapshot controller:
```
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
.
..
pvcClone.ObjectMeta.Finalizers = append(pvcClone.ObjectMeta.Finalizers, utils.PVCFinalizer)
_, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(..)
```
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
Problem:
-------
For rbd nbd userspace mounter backends, after a restart of the nodeplugin
all the mounts will start seeing IO errors. This is because, for rbd-nbd
backends there will be a userspace mount daemon running per volume, post
restart of the nodeplugin pod, there is no way to restore the daemons
back to life.
Solution:
--------
The volume healer is a one-time activity that is triggered at the startup
time of the rbd nodeplugin. It navigates through the list of volume
attachments on the node and acts accordingly.
For now, it is limited to nbd type storage only, but it is flexible and
can be extended in the future for other backend types as needed.
From a few feets above:
This solves a severe problem for nbd backed csi volumes. The healer while
going through the list of volume attachments on the node, if finds the
volume is in attached state and is of type nbd, then it will attempt to
fix the rbd-nbd volumes by sending a NodeStageVolume request with the
required volume attributes like secrets, device name, image attributes,
and etc.. which will finally help start the required rbd-nbd daemons in
the nodeplugin csi-rbdplugin container. This will allow reattaching the
backend images with the right nbd device, thus allowing the applications
to perform IO without any interruptions even after a nodeplugin restart.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
clone controller make sure there is no delete operation happens
on the source PVC which has been referred as the datasource of
clone PVC, we are safe to operate without looking at delete
operation lock in this case.
Subjected code in the controller:
...
if claim.Spec.DataSource != nil && rc.clone {
err = p.setCloneFinalizer(ctx, claim)
...
}
if !checkFinalizer(claim, pvcCloneFinalizer) {
claim.Finalizers = append(claim.Finalizers, pvcCloneFinalizer)
_, err := p.client.CoreV1().PersistentVolumeClaims(claim.Namespace).Update(..claim..)
}
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
parseTenantConfig() only allowed configuring a defined set of options,
and KMSs were not able to re-use the implementation. Now, the function
parses the ConfigMap from the Tenants Namespace and returns a map with
options that the KMS supports.
The map that parseTenantConfig() returns can be inspected by the KMS,
and applied to the vaultTenantConnection type by calling parseConfig().
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This new KMS uses a Kubernetes ServiceAccount from a Tenant (Namespace)
to connect to Hashicorp Vault. The provisioner and node-plugin will
check for the configured ServiceAccount and use the token that is
located in one of the linked Secrets. Subsequently the Vault connection
is configured to use the Kubernetes token from the Tenant.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This makes the Tenant configuration for Hashicorp Vault KMS connections
more modular. Additional KMS implementations that use Hashicorp Vault
with per-Tenant options can re-use the new vaultTenantConnection.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This commit resolves parallel test issues
and also excludes internal/util/conn_pool_test.go
as those test can't run in parallel.
Updates: #1586
Signed-off-by: Yati Padia <ypadia@redhat.com>
This commit resolves godot linter issue
which says "Comment should end in a period (godot)".
Updates: #1586
Signed-off-by: Yati Padia <ypadia@redhat.com>
This commit adds t.Helper() to the test helper
function. With this call go test prints correct
lines of code for failed tests. Otherwise,
printed lines will be inside helpers functions.
For more details check: https://github.com/kulti/thelper
Updates: #1586
Signed-off-by: Yati Padia <ypadia@redhat.com>
* Make kernel version parsing to support more (valid) version strings
* Put version string parsing into a separate, testable function
* Fixes#2248 (Kernel Subversion Parsing Failure)
Signed-off-by: Jonas Zeiger <jonas.zeiger@talpidae.net>
This commit adds capability to `metadata` encryption
to be able to fetch `encryptionPassphrase` from user
specified secret name and namespace(if not specified,
will default to namespace where PVC was created).
This behavior is followed if `secretName` key is found
in the encryption configuration else defaults to fetching
`encryptionPassphrase` from storageclass secrets.
Closes: 2107
Signed-off-by: Rakshith R <rar@redhat.com>
New go-ceph admin package api's expects to
pass the rados connection as argument.
added new method called GetRBDAdmin
to get admin connection to administrate
rbd volumes.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
We have many declarations and invocations..etc with long lines which are
very difficult to follow while doing code reading. This address the issues
in 'internal/util' package files to restrict the line length to 120 chars.
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
The Go linter paralleltest checks that the t.Parallel
gets called for the test method and for the range of
test cases within the test.
Updates: #2025
Signed-off-by: Yati Padia <ypadia@redhat.com>
This commit addresses the following issue:
'nolint:gocyclo // complexity needs to be reduced.'
is unused for linter "gocyclo" (nolintlint)
Updates:#2025
Signed-off-by: Yati Padia <ypadia@redhat.com>
This commit returns actual error returned by the go-ceph API
to the function GetPoolName(..) instead of just returning
ErrPoolNotFound everytime there is error getting the pool id.
There is a issue reported in which the snapshot creation
takes much more time to reach True state
(i.e., between 2-7 mins) and keeps trying to create with
below error though pool is present:
rpc error: code = NotFound desc = pool not found: pool ID (21)
not found in Ceph cluster.
Since we cannot interpret the actual error for the delay in
snapshot creation, it is required to return the actual error
as well so that we can uderstand the reason.
Signed-off-by: Yati Padia <ypadia@redhat.com>
as callers are already taking care of returing
the GRPC error code return the actual error
from the IsMountPoint function.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
This commit addresses ifshort linter issues which
checks if short syntax for if-statements is possible.
updates: #1586
Signed-off-by: Rakshith R <rar@redhat.com>
By default, the write buffer size in libfuse2 is 2KiB
`fuse_big_writes = true` option is used to override this limit.
This commit makes `fuse_big_writes = true` option as default
in ceph.conf.
Closes: #1928
Signed-off-by: Rakshith R <rar@redhat.com>
It helps to get a stack trace when debugging issues. Certain things are
considered bugs in the code (like missing attributes in a struct), and
might cause a panic in certain occasions.
In this case, a missing string will not panic, but the behaviour will
also not be correct (DEKs getting encrypted, but unable to decrypt).
Clearly logging this as a BUG is probably better than calling panic().
Signed-off-by: Niels de Vos <ndevos@redhat.com>
The new StoreCryptoPassphrase() method makes it possible to store an
unencrypted passphrase newly encrypted in the DEKStore.
Cloning volumes will use this, as the passphrase from the original
volume will need to get copied as part of the metadata for the volume.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
When the KMS configuration can not be found, it is useful to know what
configurations are available. This aids troubleshooting when typos in
the KMS ID are made.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Refactored deeply nested if statement in vault_tokens.go to
reduce cognitive complexity by adding fetchTenantConfig function.
Signed-off-by: Rakshith R <rar@redhat.com>
The new Amazon Metadata KMS provider uses a CMK stored in AWS KMS to
encrypt/decrypt the DEK which is stored in the volume metadata.
Updates: #1921
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Amazon KMS expects a Secret with sensitive account and key information
in the Kubernetes Namespace where the Ceph-CSI Pods are running. It will
fetch the contents of the Secret itself.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
These functions can now be re-used easier. The Amazon KMS needs to know
the Namespace of the Pod for reading a Secret with more key/values.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
After translating options from the ConfigMap into the common Vault
parameters, the generated configuration is not used. Instead, the
untranslated version of the configuration is passed on to the
vaultConnection initialization function, which then can detects missing
options.
By passing the right configuration to the initializatino function,
things work as intended.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
When using .BinaryData, the contents of the configuration is not parsed
correctly. Whereas the parsing works fine whet .Data is used.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
There is no need for each EncryptionKMS to implement the same GetID()
function. We have a VolumeEncryption type that is more suitable for
keeping track of the KMS-ID that was used to get the configuration of
the KMS.
This does not change any metadata that is stored anywhere.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
GetKMS() is the public API that initilizes the KMS providers on demand.
Each provider identifies itself with a KMS-Type, and adds its own
initialization function to a switch/case construct. This is not well
maintainable.
The new GetKMS() can be used the same way, but uses the new kmsManager
interface to create and configure the KMS provider instances.
All existing KMS providers are converted to use the new kmsManager
plugins API.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
The KMSProvider struct is a simple, extendable type that can be used to
register KMS providers with an internal kmsManager.
Helper functions for creating and configuring KMS providers will also be
located in the new kms.go file. This makes things more modular and
better maintainable.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Connections are reference counted, so just assigning the connection to
an other object for re-use is not correct. This can cause connections to
be garbage collected while something else is still using it.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This new KMS is based on the (default) SecretsKMS, but instead of using
the passphrase for all volumes, the passphrase is used to
encrypt/decrypt a Data-Encryption-Key that is stored in the metadata of
the volume.
CC: Patrick Uiterwijk <puiterwijk@redhat.com> - for encryption guidance
Signed-off-by: Niels de Vos <ndevos@redhat.com>
By adding these methods, a KMS can explicitly encrypt/decrypt the DEK if
there is no transparent way of doing so.
Hashicorp Vault encrypts the DEK when it it stored, and decrypts it when
fetched. Therefor there is no need to do any encryption in this case.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Use DEKStore API for Fetching and Storing passphrases.
Drop the fallback for the old KMS interface that is now provided as
DEKStore. The original implementation has been re-used for the DEKStore
interface.
This also moves GetCryptoPassphrase/StoreNewCryptoPassphrase functions
to methods of VolumeEncryption.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
DEKStore is a new interface that will be used for Storing and Fetching
DEKs. The existing implementations for KMS already function as a
DEKStore, and will be updated to match the interface.
By splitting KMS and DEKStore into two components, the encryption
configuration for volumes becomes more modular. This makes it possible
to implement a DEKStore where the encrypted DEK for a volume is stored
in the metadata of the volume (RBD image).
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Prepare for grouping encryption related functions together. The main
rbdVolume object should not be cluttered with KMS or DEK procedures.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Prepared for an enhanced API to communicate with a KMS and keep the DEK
storage separate. The crypto.go file is already mixed with different
functions, so moving the KMS part into its own file, just like we have
for Hashicorp Vault KMS's.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Storing a passphrase is now done while the volume is created. There is
no need to (re)generate a passphrase when it can not be found.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Have the provisioner create the passphrase for the volume, instead of
doign it lazily at the time the volume is used for the 1st time. This
prevents potential races where pods on different nodes try to store
different passphrases at the (almost) same time.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
"VAULT_SKIP_VERIFY" is a standard Hashicorp Vault environment variable
(a string) that needs to get converted to the "vaultCAVerify"
configuration option in the Ceph-CSI format.
The value of "VAULT_SKIP_VERIFY" means the reverse of "vaultCAVerify",
this part was missing in the original conversion too.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
the tenant/namespace is needed to read the certificates,
this commit sets the tenant in kms object.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
currently, the keys for kms certificates/keys in a
secret is ca.cert, tls.cert and
tls.key, this commit changes the key from ca.cert
and tls.cert to cert and tls.key to key.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
if are reading the kms data from the file.
than only we need to unmarshal. If we are reading
from the configmap it already returns the unmarshal
data.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
The configuration option `EnvVaultInsecure` is expected to be a string,
not a boolean. By converting the bool back to a string (after
verification), it is now possible to skip the certificate validation
check by setting `vaultCAVerify: false` in the Vault configuration.
Fixes: #1852
Reported-by: Bryon Nevis <bryon.nevis@intel.com>
Signed-off-by: Niels de Vos <ndevos@redhat.com>
When the KMS VaultTokens is configured through a Kubernetens ConfigMap,
the `VAULT_SKIP_VERIFY` option was not taken into account. The option
maps to the `vaultCAVerify` value in the configuration file, but has the
reverse meaning.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Currently cephcsi is returning an error
if the ENV variable is set, but it should not.
This commit fixes the the POD_NAMESPACE env
variable issue and as well as the KMS_CONFIG_NAME
ENV variable.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
if the kms encryption configmap is not mounted
as a volume to the CSI pods, add the code to
read the configuration from the kubernetes. Later
the code to fetch the configmap will be moved to
the new sidecar which is will talk to respective
CO to fetch the encryption configurations.
The k8s configmap uses the standard vault spefic
names to add the configurations. this will be converted
back to the CSI configurations.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
A tenant can place a ConfigMap in their Kubernetes Namespace with
configuration options that differ from the global (by the Storage Admin
set) values.
The ConfigMap needs to be located in the Tenants namespace, as described
in the documentation
See-also: docs/design/proposals/encryption-with-vault-tokens.md
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Added a option to pass the client certificate
and the client certificate key for the vault token
based encryption.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Tenants (Kubernetes Namespaces) can use their own Vault Token to manage
the encryption keys for PVCs. The working is documented in #1743.
See-also: #1743Closes: #1500
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Add a new method to the EncryptionKMS interface so that resources can be
freed when EncryptionKMS instances get freed.
With the move to using the libopenstorage API, a temporary file needs to
store the optional CA certificate. The Destroy() method of the
vaultConnection type now removes this file.
The rbdVolume uses the EncryptionKMS type now, so call the new Destroy()
method from withing rbdVolume.Destroy().
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Make it possible to calle initConnection() multiple times. This enables
the VaultTokensKMS type to override global settings with options from a
per-tenant configuration.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This makes it possible to pass a more complex configuration to the
initialize functions for KMS's. The upcoming VaultTokensKMS can use
overrides for configiration options on a per tenant basis. Without this
change, it would not be possible to consume the JSON configuration file.
See-also: #1743
Signed-off-by: Niels de Vos <ndevos@redhat.com>
In order to re-use the configuration of Vault, split a new
vaultConnection type from the VaultKMS type.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Instead of the hand-rolled Vault usage, use the libopenstorage/secrets
package that provides a nice API. The support for Vault becomes much
simpler and maintainable that way.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
An rbd image can have a maximum number of
snapshots defined by maxsnapshotsonimage
On the limit is reached the cephcsi will
start flattening the older snapshots and
returns the ABORT error message, The Request
comes after this as to wait till all the
images are flattened (this will increase the
PVC creation time. Instead of waiting till
the maximum snapshots on an RBD image, we can
have a soft limit, once the limit reached
cephcsi will start flattening the task to
break the chain. With this PVC creation time
will only be affected when the hard limit
(minsnapshotsonimage) reached.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
We have below exported function in credentials.go which is not
called from anywhere in the repo. Removing it for the same reason.
```
// NewCredentials generates new credentials when id and key
// are provided.
func NewCredentials(id, key string) (*Credentials, error) {
...
```
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
`cap` builtin function returns the capacity of a type. Its not
good practice to use this builtin function for other variable
names, removing it here
Ref# https://golang.org/pkg/builtin/#cap
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
The go-ceph CephFS Admin API needs a rados.Conn to connect the Admin
interface to the Ceph cluster. As the rados.Conn is internal to the
ClusterConnection type, add GetFSAdmin() to create a new FSAdmin
instance for a specific connection.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
rename FatalLog to FatalLogMsg to keep functions
in parity functions ends with Msg if its only message
logging.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
as we have 2 functions for logging. one for logging
with message and another one is for logging with
context. renamed ErrorLog to ErrorLogMsg to log
with messages.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
