Skip to content

Commit

Permalink
core: add context parameter to functions
Browse files Browse the repository at this point in the history 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
  • Loading branch information
@weirdwiz
weirdwiz committed Mar 22, 2022
1 parent 833c458 commit 9008409
Show file tree
Hide file tree
Showing 58 changed files with 241 additions and 238 deletions.
4 changes: 3 additions & 1 deletion cmd/rook/ceph/cleanup.go
View file
Expand Up @@ -59,6 +59,8 @@ func startCleanUp(cmd *cobra.Command, args []string) error {
rook.SetLogLevel()
rook.LogStartupInfo(cleanUpCmd.Flags())

ctx := cmd.Context()

logger.Info("starting cluster clean up")
// Delete dataDirHostPath
if dataDirHostPath != "" {
Expand All @@ -67,7 +69,7 @@ func startCleanUp(cmd *cobra.Command, args []string) error {
}

namespace := os.Getenv(k8sutil.PodNamespaceEnvVar)
clusterInfo := client.AdminClusterInfo(namespace, "")
clusterInfo := client.AdminClusterInfo(ctx, namespace, "")
clusterInfo.FSID = clusterFSID

// Build Sanitizer
Expand Down
3 changes: 2 additions & 1 deletion cmd/rook/discover.go
View file
Expand Up @@ -51,8 +51,9 @@ func startDiscover(cmd *cobra.Command, args []string) error {
rook.LogStartupInfo(discoverCmd.Flags())

context := rook.NewContext()
ctx := cmd.Context()

err := discover.Run(context, discoverDevicesInterval, usesCVInventory)
err := discover.Run(ctx, context, discoverDevicesInterval, usesCVInventory)
if err != nil {
rook.TerminateFatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/daemon/ceph/client/command.go
View file
Expand Up @@ -165,7 +165,7 @@ func (c *CephToolCommand) run() ([]byte, error) {
// Still forcing the check for the command if the behavior changes in the future
if command == RBDTool {
if c.RemoteExecution {
output, stderr, err = c.context.RemoteExecutor.ExecCommandInContainerWithFullOutputWithTimeout(ProxyAppLabel, CommandProxyInitContainerName, c.clusterInfo.Namespace, append([]string{command}, args...)...)
output, stderr, err = c.context.RemoteExecutor.ExecCommandInContainerWithFullOutputWithTimeout(c.clusterInfo.Context, ProxyAppLabel, CommandProxyInitContainerName, c.clusterInfo.Namespace, append([]string{command}, args...)...)
if stderr != "" || err != nil {
err = errors.Errorf("%s. %s", err.Error(), stderr)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/daemon/ceph/client/info.go
View file
Expand Up @@ -89,7 +89,7 @@ func (c *ClusterInfo) NamespacedName() types.NamespacedName {

// AdminClusterInfo() creates a ClusterInfo with the basic info to access the cluster
// as an admin.
func AdminClusterInfo(namespace, name string) *ClusterInfo {
func AdminClusterInfo(ctx context.Context, namespace, name string) *ClusterInfo {
ownerInfo := k8sutil.NewOwnerInfoWithOwnerRef(&metav1.OwnerReference{}, "")
return &ClusterInfo{
Namespace: namespace,
Expand All @@ -98,14 +98,14 @@ func AdminClusterInfo(namespace, name string) *ClusterInfo {
},
name: name,
OwnerInfo: ownerInfo,
Context: context.TODO(),
Context: ctx,
}
}

// AdminTestClusterInfo() creates a ClusterInfo with the basic info to access the cluster
// as an admin. This cluster info should only be used by unit or integration tests.
func AdminTestClusterInfo(namespace string) *ClusterInfo {
return AdminClusterInfo(namespace, "testing")
return AdminClusterInfo(context.TODO(), namespace, "testing")
}

// IsInitialized returns true if the critical information in the ClusterInfo struct has been filled
Expand Down
10 changes: 5 additions & 5 deletions pkg/daemon/ceph/osd/kms/kms.go
View file
Expand Up @@ -86,7 +86,7 @@ func (c *Config) PutSecret(secretName, secretValue string) error {
}
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
v, err := InitVault(c.ClusterInfo.Context, c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
if err != nil {
return errors.Wrap(err, "failed to init vault kms")
}
Expand Down Expand Up @@ -123,7 +123,7 @@ func (c *Config) GetSecret(secretName string) (string, error) {
var value string
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
v, err := InitVault(c.ClusterInfo.Context, c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
if err != nil {
return "", errors.Wrap(err, "failed to init vault")
}
Expand Down Expand Up @@ -153,7 +153,7 @@ func (c *Config) GetSecret(secretName string) (string, error) {
func (c *Config) DeleteSecret(secretName string) error {
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
v, err := InitVault(c.ClusterInfo.Context, c.context, c.ClusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
if err != nil {
return errors.Wrap(err, "failed to delete secret in vault")
}
Expand Down Expand Up @@ -263,7 +263,7 @@ func ValidateConnectionDetails(ctx context.Context, clusterdContext *clusterd.Co
// Validate KMS provider connection details for each provider
switch provider {
case secrets.TypeVault:
err := validateVaultConnectionDetails(clusterdContext, ns, securitySpec.KeyManagementService.ConnectionDetails)
err := validateVaultConnectionDetails(ctx, clusterdContext, ns, securitySpec.KeyManagementService.ConnectionDetails)
if err != nil {
return errors.Wrap(err, "failed to validate vault connection details")
}
Expand All @@ -273,7 +273,7 @@ func ValidateConnectionDetails(ctx context.Context, clusterdContext *clusterd.Co
case VaultKVSecretEngineKey:
// Append Backend Version if not already present
if GetParam(securitySpec.KeyManagementService.ConnectionDetails, vault.VaultBackendKey) == "" {
backendVersion, err := BackendVersion(clusterdContext, ns, securitySpec.KeyManagementService.ConnectionDetails)
backendVersion, err := BackendVersion(ctx, clusterdContext, ns, securitySpec.KeyManagementService.ConnectionDetails)
if err != nil {
return errors.Wrap(err, "failed to get backend version")
}
Expand Down
44 changes: 22 additions & 22 deletions pkg/daemon/ceph/osd/kms/kms_test.go
View file
Expand Up @@ -34,7 +34,7 @@ import (
func TestValidateConnectionDetails(t *testing.T) {
ctx := context.TODO()
// Placeholder
context := &clusterd.Context{Clientset: test.New(t, 3)}
clusterdContext := &clusterd.Context{Clientset: test.New(t, 3)}
securitySpec := &cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{ConnectionDetails: map[string]string{}}}
ns := "rook-ceph"
vaultSecret := &v1.Secret{
Expand Down Expand Up @@ -63,67 +63,67 @@ func TestValidateConnectionDetails(t *testing.T) {
}}

t.Run("no kms provider given", func(t *testing.T) {
err := ValidateConnectionDetails(ctx, context, securitySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate kms config \"KMS_PROVIDER\". cannot be empty")
securitySpec.KeyManagementService.ConnectionDetails["KMS_PROVIDER"] = "vault"
})

t.Run("vault - no token object", func(t *testing.T) {
securitySpec.KeyManagementService.TokenSecretName = "vault-token"
err := ValidateConnectionDetails(ctx, context, securitySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to fetch kms token secret \"vault-token\": secrets \"vault-token\" not found")
})

t.Run("vault - token secret present but empty content", func(t *testing.T) {
_, err := context.Clientset.CoreV1().Secrets(ns).Create(ctx, vaultSecret, metav1.CreateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Create(ctx, vaultSecret, metav1.CreateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, securitySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to read k8s kms secret \"token\" key \"vault-token\" (not found or empty)")
})

t.Run("vault - token key does not exist", func(t *testing.T) {
vaultSecret.Data = map[string][]byte{"foo": []byte("bar")}
_, err := context.Clientset.CoreV1().Secrets(ns).Update(ctx, vaultSecret, metav1.UpdateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Update(ctx, vaultSecret, metav1.UpdateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, securitySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to read k8s kms secret \"token\" key \"vault-token\" (not found or empty)")
})

// Success: token content is ok
t.Run("vault - token content is ok", func(t *testing.T) {
vaultSecret.Data["token"] = []byte("token")
_, err := context.Clientset.CoreV1().Secrets(ns).Update(ctx, vaultSecret, metav1.UpdateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Update(ctx, vaultSecret, metav1.UpdateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, securitySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate vault connection details: failed to find connection details \"VAULT_ADDR\"")
securitySpec.KeyManagementService.ConnectionDetails["VAULT_ADDR"] = "https://1.1.1.1:8200"
})

t.Run("vault - TLS is configured but secrets do not exist", func(t *testing.T) {
securitySpec.KeyManagementService.ConnectionDetails["VAULT_CACERT"] = "vault-ca-secret"
err := ValidateConnectionDetails(ctx, context, securitySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate vault connection details: failed to find TLS connection details k8s secret \"vault-ca-secret\"")
})

t.Run("vault - TLS secret exists but empty key", func(t *testing.T) {
_, err := context.Clientset.CoreV1().Secrets(ns).Create(ctx, tlsSecret, metav1.CreateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Create(ctx, tlsSecret, metav1.CreateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, securitySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate vault connection details: failed to find TLS connection key \"cert\" for \"VAULT_CACERT\" in k8s secret \"vault-ca-secret\"")
})

t.Run("vault - success TLS config is correct", func(t *testing.T) {
tlsSecret.Data = map[string][]byte{"cert": []byte("envnrevbnbvsbjkrtn")}
_, err := context.Clientset.CoreV1().Secrets(ns).Update(ctx, tlsSecret, metav1.UpdateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Update(ctx, tlsSecret, metav1.UpdateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, securitySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.NoError(t, err, "")
})

Expand All @@ -136,7 +136,7 @@ func TestValidateConnectionDetails(t *testing.T) {
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client
// Mock the client here
vaultClient = func(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
vaultClient = func(ctx context.Context, clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
return client, nil
}
if err := client.Sys().Mount("rook/", &api.MountInput{
Expand All @@ -156,39 +156,39 @@ func TestValidateConnectionDetails(t *testing.T) {
TokenSecretName: "vault-token",
},
}
err := ValidateConnectionDetails(ctx, context, securitySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, securitySpec, ns)
assert.NoError(t, err, "")
assert.Equal(t, securitySpec.KeyManagementService.ConnectionDetails["VAULT_BACKEND"], "v2")
})

t.Run("ibm kp - fail no token specified, only token is supported", func(t *testing.T) {
err := ValidateConnectionDetails(ctx, context, ibmSecuritySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, ibmSecuritySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate kms configuration (missing token in spec)")
ibmSecuritySpec.KeyManagementService.TokenSecretName = "ibm-token"

})

t.Run("ibm kp - token present but no key for service key", func(t *testing.T) {
_, err := context.Clientset.CoreV1().Secrets(ns).Create(ctx, ibmSecret, metav1.CreateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Create(ctx, ibmSecret, metav1.CreateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, ibmSecuritySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, ibmSecuritySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to read k8s kms secret \"IBM_KP_SERVICE_API_KEY\" key \"ibm-token\" (not found or empty)")
})

t.Run("ibm kp - token ok but no instance id", func(t *testing.T) {
ibmSecret.Data["IBM_KP_SERVICE_API_KEY"] = []byte("foo")
_, err := context.Clientset.CoreV1().Secrets(ns).Update(ctx, ibmSecret, metav1.UpdateOptions{})
_, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Update(ctx, ibmSecret, metav1.UpdateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(ctx, context, ibmSecuritySpec, ns)
err = ValidateConnectionDetails(ctx, clusterdContext, ibmSecuritySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate kms config \"IBM_KP_SERVICE_INSTANCE_ID\". cannot be empty")
ibmSecuritySpec.KeyManagementService.ConnectionDetails["IBM_KP_SERVICE_INSTANCE_ID"] = "foo"
})

t.Run("ibm kp - success", func(t *testing.T) {
err := ValidateConnectionDetails(ctx, context, ibmSecuritySpec, ns)
err := ValidateConnectionDetails(ctx, clusterdContext, ibmSecuritySpec, ns)
assert.NoError(t, err, "")
// IBM_KP_SERVICE_API_KEY must be appended to the details so that the client can be built with
// all the details
Expand Down
10 changes: 4 additions & 6 deletions pkg/daemon/ceph/osd/kms/vault.go
View file
Expand Up @@ -75,7 +75,7 @@ type removeCertFilesFunction func()
*/

// InitVault inits the secret store
func InitVault(context *clusterd.Context, namespace string, config map[string]string) (secrets.Secrets, error) {
func InitVault(ctx context.Context, context *clusterd.Context, namespace string, config map[string]string) (secrets.Secrets, error) {
c := make(map[string]interface{})

// So that we don't alter the content of c.config for later iterations
Expand All @@ -86,7 +86,7 @@ func InitVault(context *clusterd.Context, namespace string, config map[string]st
}

// Populate TLS config
newConfigWithTLS, removeCertFiles, err := configTLS(context, namespace, oriConfig)
newConfigWithTLS, removeCertFiles, err := configTLS(ctx, context, namespace, oriConfig)
if err != nil {
return nil, errors.Wrap(err, "failed to initialize vault tls configuration")
}
Expand All @@ -111,8 +111,7 @@ func InitVault(context *clusterd.Context, namespace string, config map[string]st
// The signature has named result parameters to help building 'defer' statements especially for the
// content of removeCertFiles which needs to be populated by the files to remove if no errors and be
// nil on errors
func configTLS(clusterdContext *clusterd.Context, namespace string, config map[string]string) (newConfig map[string]string, removeCertFiles removeCertFilesFunction, retErr error) {
ctx := context.TODO()
func configTLS(ctx context.Context, clusterdContext *clusterd.Context, namespace string, config map[string]string) (newConfig map[string]string, removeCertFiles removeCertFilesFunction, retErr error) {
var filesToRemove []*os.File

defer func() {
Expand Down Expand Up @@ -248,8 +247,7 @@ func (c *Config) IsVault() bool {
return c.Provider == secrets.TypeVault
}

func validateVaultConnectionDetails(clusterdContext *clusterd.Context, ns string, kmsConfig map[string]string) error {
ctx := context.TODO()
func validateVaultConnectionDetails(ctx context.Context, clusterdContext *clusterd.Context, ns string, kmsConfig map[string]string) error {
for _, option := range vaultMandatoryConnectionDetails {
if GetParam(kmsConfig, option) == "" {
return errors.Errorf("failed to find connection details %q", option)
Expand Down
9 changes: 5 additions & 4 deletions pkg/daemon/ceph/osd/kms/vault_api.go
View file
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package kms

import (
"context"
"strings"

"github.com/libopenstorage/secrets/vault"
Expand All @@ -38,7 +39,7 @@ var vaultClient = newVaultClient

// newVaultClient returns a vault client, there is no need for any secretConfig validation
// Since this is called after an already validated call InitVault()
func newVaultClient(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
func newVaultClient(ctx context.Context, clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
// DefaultConfig uses the environment variables if present.
config := api.DefaultConfig()

Expand All @@ -56,7 +57,7 @@ func newVaultClient(clusterdContext *clusterd.Context, namespace string, secretC
}

// Populate TLS config
newConfigWithTLS, removeCertFiles, err := configTLS(clusterdContext, namespace, localSecretConfig)
newConfigWithTLS, removeCertFiles, err := configTLS(ctx, clusterdContext, namespace, localSecretConfig)
if err != nil {
return nil, errors.Wrap(err, "failed to initialize vault tls configuration")
}
Expand Down Expand Up @@ -106,7 +107,7 @@ func newVaultClient(clusterdContext *clusterd.Context, namespace string, secretC
return client, nil
}

func BackendVersion(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (string, error) {
func BackendVersion(ctx context.Context, clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (string, error) {
v1 := "v1"
v2 := "v2"

Expand All @@ -125,7 +126,7 @@ func BackendVersion(clusterdContext *clusterd.Context, namespace string, secretC
return v2, nil
default:
// Initialize Vault client
vaultClient, err := vaultClient(clusterdContext, namespace, secretConfig)
vaultClient, err := vaultClient(ctx, clusterdContext, namespace, secretConfig)
if err != nil {
return "", errors.Wrap(err, "failed to initialize vault client")
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/daemon/ceph/osd/kms/vault_api_test.go
View file
Expand Up @@ -40,9 +40,10 @@ func TestBackendVersion(t *testing.T) {
core := cluster.Cores[0].Core
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client
ctx := context.TODO()

// Mock the client here
vaultClient = func(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
vaultClient = func(ctx context.Context, clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (*api.Client, error) {
return client, nil
}

Expand Down Expand Up @@ -76,7 +77,7 @@ func TestBackendVersion(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := BackendVersion(&clusterd.Context{}, "ns", tt.args.secretConfig)
got, err := BackendVersion(ctx, &clusterd.Context{}, "ns", tt.args.secretConfig)
if (err != nil) != tt.wantErr {
t.Errorf("BackendVersion() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -186,7 +187,7 @@ cjLxGL8tcZjHKg==
}

// Populate TLS config
newConfigWithTLS, removeCertFiles, err := configTLS(context, ns, secretConfig)
newConfigWithTLS, removeCertFiles, err := configTLS(ctx, context, ns, secretConfig)
assert.NoError(t, err)
defer removeCertFiles()

Expand Down

0 comments on commit 9008409

Please sign in to comment.