Commit Graph

189 Commits

Author SHA1 Message Date
Niels de Vos
4a3b1181ce cleanup: move KMS functionality into its own package
A new "internal/kms" package is introduced, it holds the API that can be
consumed by the RBD components.

The KMS providers are currently in the same package as the API. With
later follow-up changes the providers will be placed in their own
sub-package.

Because of the name of the package "kms", the types, functions and
structs inside the package should not be prefixed with KMS anymore:

    internal/kms/kms.go:213:6: type name will be used as kms.KMSInitializerArgs by other packages, and that stutters; consider calling this InitializerArgs (golint)

Updates: #852
Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-30 16:31:40 +00:00
Niels de Vos
778b5e86de cleanup: move k8s functions to the util/k8s package
By placing the NewK8sClient() function in its own package, the KMS API
can be split from the "internal/util" package. Some of the KMS providers
use the NewK8sClient() function, and this causes circular dependencies
between "internal/utils" -> "internal/kms" -> "internal/utils", which
are not alowed in Go.

Updates: #852
Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-30 16:31:40 +00:00
Niels de Vos
6d00b39886 cleanup: move log functions to new internal/util/log package
Moving the log functions into its own internal/util/log package makes it
possible to split out the humongous internal/util packages in further
smaller pieces. This reduces the inter-dependencies between utility
functions and components, preventing circular dependencies which are not
allowed in Go.

Updates: #852
Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-26 09:34:05 +00:00
Niels de Vos
68588dc7df util: fix unit-test for GetClusterMappingInfo()
Unit-testing often fails due to a race condition while writing the
clusterMappingConfigFile from multiple go-routines at the same time.
Failures from `make containerized-test` look like this:

    === CONT  TestGetClusterMappingInfo/site2-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site1-storage:site2-storage] [map[1:3]] [map[11:5]]} {map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    === CONT  TestGetClusterMappingInfo/site3-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    --- FAIL: TestGetClusterMappingInfo (0.01s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_not_found (0.00s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_found_with_empty_data (0.00s)
        --- PASS: TestGetClusterMappingInfo/cluster-id_mapping_not_found (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site2-storage_cluster-id_mapping (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site3-storage_cluster-id_mapping (0.00s)
        --- PASS: TestGetClusterMappingInfo/site1-storage_cluster-id_mapping (0.00s)

By splitting the public GetClusterMappingInfo() function into an
internal getClusterMappingInfo() that takes a filename, unit-testing can
use different files for each go-routine, and testing becomes more
predictable.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-25 16:08:48 +00:00
Rakshith R
f05ac2b25d rbd: extract kmsID from volumeAttributes in RegenerateJournal()
This commit adds functionality of extracting encryption kmsID,
owner from volumeAttributes in RegenerateJournal() and adds utility
functions ParseEncryptionOpts and FetchEncryptionKMSID.

Signed-off-by: Rakshith R <rar@redhat.com>
2021-08-10 09:17:59 +00:00
Madhu Rajanna
6cc37f0a17 cleanup: use different file name for testing
For clusterMappingConfigFile using different
file name so that multiple unit test cases can
work without any data race.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
2021-08-09 13:37:25 +00:00
Madhu Rajanna
916c97b4a8 rbd: copy creds when copying the connection
rbd flatten functions is a CLI call and it expects
the creds as the input and copying of creds is
required when we generate the temp clone image.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
2021-08-09 07:28:43 +00:00
Niels de Vos
b5d2321d57 cleanup: use vaultDefaultCAVerify to set default value
Golang-ci complains about the following:

    internal/util/vault_tokens.go:99:20: string `true` has 4 occurrences, but such constant `vaultDefaultDestroyKeys` already exists (goconst)
    	v.VaultCAVerify = "true"
    	                  ^

This occurence of "true" can be replaced by vaultDefaultCAVerify so
address the warning.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-06 12:19:18 +00:00
Niels de Vos
f584db41e6 util: add vaultDestroyKeys option to destroy Vault kv-v2 secrets
Hashicorp Vault does not completely remove the secrets in a kv-v2
backend when the keys are deleted. The metadata of the keys will be
kept, and it is possible to recover the contents of the keys afterwards.

With the new `vaultDestroyKeys` configuration parameter, this behaviour
can now be selected. By default the parameter will be set to `true`,
indicating that the keys and contents should completely be destroyed.
Setting it to any other value will make it possible to recover the
deleted keys.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-06 12:19:18 +00:00
Madhu Rajanna
ac11d71e19 util: add helper function to read clusterID mapping
added helper function to read the clusterID mapping
from the mounted file.

The clusterID mapping contains below mappings
* ClusterID mappings (to cluster to which we are failingover
and from which cluster failover happened)
* RBD PoolID mapping of between the clusters.
* CephFS FscID mapping between the clusters.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
2021-08-05 16:07:51 +00:00
Niels de Vos
4859f2dfdb util: allow configuring VAULT_AUTH_MOUNT_PATH for Vault Tenant SA KMS
The VAULT_AUTH_MOUNT_PATH is a Vault configuration parameter that allows
a user to set a non default path for the Kubernetes ServiceAccount
integration. This can already be configured for the Vault KMS, and is
now added to the Vault Tenant SA KMS as well.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-05 06:02:57 +00:00
Niels de Vos
f2d5c2e0df util: add vaultAuthNamespace option for Vault KMS
The new `vaultAuthNamespace` configuration parameter can be set to the
Vault Namespace where the authentication is setup in the service. Some
Hashicorp Vault deployments use sub-namespaces for their users/tenants,
with a 'root' namespace where the authentication is configured. This
requires passing of different Vault namespaces for different operations.

Example:
 - the Kubernetes Auth mechanism is configured for in the Vault
   Namespace called 'devops'
 - a user/tenant has a sub-namespace called 'devops/website' where the
   encryption passphrases can be placed in the key-value store

The configuration for this, then looks like:

    vaultAuthNamespace: devops
    vaultNamespace: devops/homepage

Note that Vault Namespaces are a feature of the Hashicorp Vault
Enterprise product, and not part of the Open Source version. This
prevents adding e2e tests that validate the Vault Namespace
configuration.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-04 18:20:45 +00:00
Niels de Vos
83167e2ac5 util: correct error message when connecting to Vault fails
Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-08-04 18:20:45 +00:00
Artur Troian
16ec97d8f7 util: getCgroupPidsFile produces striped path when extra : present
This commit uses `string.SplitN` instead of `string.Split`.
The path for pids.max has extra `:` symbols in it due to which
getCgroupPidsFile() splits the string into 5 tokens instead of
3 leading to loss of part of the path.
As a result, the below error is reported:
`Failed to get the PID limit, can not reconfigure: open
/sys/fs/cgroup/pids/system.slice/containerd.service/
kubepods-besteffort-pod183b9d14_aed1_4b66_a696_da0c738bc012.slice/pids.max:
no such file or directory`
SplitN takes an argument n and splits the string
accordingly which helps us to get the desired
file path.

Fixes: #2337

Co-authored-by: Yati Padia <ypadia@redhat.com>
Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-08-03 06:03:10 +00:00
Prasanna Kumar Kalever
526ff95f10 rbd: add support to expand encrypted volume
Previously in ControllerExpandVolume() we had a check for encrypted
volumes and we use to fail for all expand requests on an encrypted
volume. Also for Block VolumeMode PVCs NodeExpandVolume used to be
ignored/skipped.

With these changes, we add support for the expansion of encrypted volumes.
Also for raw Block VolumeMode PVCs with Encryption we call NodeExpandVolume.

That said,
With LUKS1, cryptsetup utility doesn't prompt for a passphrase on resizing
the crypto mapper device. This is because LUKS1 devices don't use kernel
keyring for volume keys.

Whereas, LUKS2 devices use kernel keyring for volume key by default, i.e.
cryptsetup utility asks for a passphrase if it detects volume key was
previously passed to dm-crypt via kernel keyring service, we are overriding
the default by --disable-keyring option during cryptsetup open command.
So that at the time of crypto mapper device resize we will not be
prompted for any passphrase.

Fixes: #1469

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
2021-07-23 10:00:23 +00:00
Prasanna Kumar Kalever
4fa05cb3a1 util: add helper functions for resize of encrypted volume
such as:
ResizeEncryptedVolume() and LuksResize()

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
2021-07-23 10:00:23 +00:00
Prasanna Kumar Kalever
572f39d656 util: fix log level in OpenEncryptedVolume()
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
2021-07-23 10:00:23 +00:00
Prasanna Kumar Kalever
812003eb45 util: fix bug in DeviceEncryptionStatus()
With Luks1 device:
$ cryptsetup status /dev/mapper/crypto-rbd0
/dev/mapper/crypto-rbd0 is active and is in use.
  type:    LUKS1
  cipher:  aes-xts-plain64
  keysize: 512 bits
  key location: dm-crypt
  device:  /dev/rbd0
  sector size:  512
  offset:  4096 sectors
  size:    4190208 sectors
  mode:    read/write

With Luks2 device:
$ cryptsetup status /dev/mapper/crypto-rbd0
/dev/mapper/crypto-rbd0 is active and is in use.
  type:    LUKS2
  cipher:  aes-xts-plain64
  keysize: 512 bits
  key location: dm-crypt
  device:  /dev/rbd0
  sector size:  512
  offset:  32768 sectors
  size:    4161536 sectors
  mode:    read/write

This could lead to failures with unmap in the NodeUnstageVolume path
for the encrypted volumes.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
2021-07-23 10:00:23 +00:00
Yati Padia
1ae2afe208 cleanup: modifies the error caused due to merged PRs
This commit modifies the error of godot, cyclop,
paralleltest linter caused due to merged PRs.

Updates: #1586

Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-07-22 18:15:48 +00:00
Yati Padia
172b66f73f cleanup: resolves cyclop linter issue
this commit adds `// nolint:cyclop` for the
fucntions whose complexity is above 20

Updates: #1586

Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-07-22 18:15:48 +00:00
Humble Chirammal
abe6a6e5ac util: remove deleteLock test as it is enforced by the controller
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
2021-07-22 15:07:49 +00:00
Humble Chirammal
c42d4768ca util: remove the deleteLock acquistion check for clone and snapshot
At present while acquiring the deleteLock on the volume, we check
for ongoing clone and snapshot creation operations on the same.
Considering snapshot and clone controllers does not allow parent
volume deletion on subjected operations, we can be free from this
extra check.

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
2021-07-22 15:07:49 +00:00
Niels de Vos
82557e3f34 util: allow configuring VAULT_BACKEND for Vault connection
It seems that the version of the key/value engine can not always be
detected for Hashicorp Vault. In certain cases, it is required to
configure the `VAULT_BACKEND` (or `vaultBackend`) option so that a
successful connection to the service can be made.

The `kv-v2` is the current default for development deployments of
Hashicorp Vault (what we use for automated testing). Production
deployments default to version 1 for now.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
2021-07-22 13:02:47 +00:00
Rakshith R
43f753760b cleanup: resolve nlreturn linter issues
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>
2021-07-22 06:05:01 +00:00
Yati Padia
3469dfc753 cleanup: resolve errorlint issues
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>
2021-07-19 13:31:29 +00:00
Humble Chirammal
bd947bbe31 util: remove deleteLock check while acquiring snapshot createLock
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>
2021-07-17 10:23:13 +00:00
Prasanna Kumar Kalever
b6a88dd728 rbd: add volume healer
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>
2021-07-16 16:30:58 +00:00
Humble Chirammal
94c5c5e119 util: remove deleteLock while we acquire clone operation lock
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>
2021-07-16 12:32:28 +00:00
Yati Padia
f36d611ef9 cleanup: resolves gofumpt issues of internal codes
This PR runs gofumpt for internal folder.

Updates: #1586

Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-07-14 19:50:56 +00:00
Niels de Vos
d941e5abac util: make parseTenantConfig() usable for modular KMSs
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>
2021-07-13 17:16:35 +00:00
Niels de Vos
3d7d48a4aa util: VaultTenantSA KMS implementation
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>
2021-07-13 17:16:35 +00:00
Niels de Vos
6dc5bf2b29 util: split vaultTenantConnection from VaultTokensKMS
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>
2021-07-13 17:16:35 +00:00
Yati Padia
69c9e5ffb1 cleanup: resolve parallel test issue
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>
2021-07-13 11:31:39 +00:00
Yati Padia
4a649fe17f cleanup: resolve godot linter
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>
2021-07-13 06:50:03 +00:00
Yati Padia
f35ce3d880 cleanup: Adds t.Helper() to test helper function
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>
2021-07-12 11:25:55 +00:00
Jonas Zeiger
680a7bf411 util: more generic kernel version parsing
* 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>
2021-07-09 07:36:27 +00:00
Rakshith R
3352d4aabd rbd: add user secret based metadata encryption
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>
2021-07-08 17:06:02 +00:00
Madhu Rajanna
b1710f4c53 util: add method to get rados connection
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>
2021-07-06 14:41:48 +00:00
Humble Chirammal
cc6d67a7d6 internal: reformat long lines in internal/util package to 120 chars
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>
2021-06-28 14:43:49 +00:00
Yati Padia
13667c013c cleanup: addresses paralleltest linter
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>
2021-06-25 11:55:12 +00:00
Yati Padia
6bfdf2feb0 cleanup: gocyclo being unused for linter
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>
2021-06-15 02:54:16 +00:00
Yati Padia
095a82f37d util: returns actual error instead of ErrPoolNotFound
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>
2021-06-14 14:41:32 +00:00
Humble Chirammal
17b0091cba cleanup: fix codespell error in internal/utils package
Codespell checker report below error:
```
Resulting CLI options  --check-filenames --check-hidden --skip .git,./vendor --ignore-words-list ExtraVersion,extraversion,ba
1
Error: ./internal/util/aws_metadata.go:96: Kubenetes ==> Kubernetes
```
This commit address the same.

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
2021-06-11 08:04:07 +00:00
Madhu Rajanna
b4dbffa316 util: return actual error from IsMountPoint
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>
2021-06-07 07:38:48 +00:00
Yati Padia
0f44c6acb7 cleanup: address wasted assign issues
At places variable is reassigned without
being used.

Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-06-03 09:51:14 +00:00
Rakshith R
b891e5585d cleanup: address ifshort linter issues
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>
2021-05-26 07:04:32 +00:00
Yati Padia
774e8e4042 util: enable golang profiling
Add support for golang profiling.
Standard tools like go tool pprof and curl
work. example:
$ go tool pprof http://localhost:8080/debug/pprof/profile
$ go tool pprof http://localhost:8080/debug/pprof/heap
$ curl http://localhost:8080/debug/pprof/heap?debug=1

https://golang.org/pkg/net/http/pprof/ contains
more details about the pprof interface.

Fixes: #1699

Signed-off-by: Yati Padia <ypadia@redhat.com>
2021-05-25 10:41:22 +00:00
Humble Chirammal
074c937a08 cleanup: correct typo in vault_tokens.go
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
2021-04-29 08:51:29 +00:00
Rakshith R
9f2cf498b6 cephfs: enable ceph-fuse big_writes by default
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>
2021-04-19 07:08:57 +00:00
Humble Chirammal
0fae0e53b6 cleanup: various source code comment corrections
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
2021-04-16 10:22:35 +00:00