Skip to content

Commit

Permalink
Merge pull request #8618 from leseb/use-pointers
Browse files Browse the repository at this point in the history
ceph: fix vault kv secret engine auto-detection
  • Loading branch information
leseb committed Aug 31, 2021
2 parents 58a43a4 + d675969 commit 00c111b
Show file tree
Hide file tree
Showing 8 changed files with 794 additions and 53 deletions.
9 changes: 6 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/rook/rook
go 1.16

require (
github.com/aws/aws-sdk-go v1.35.24
github.com/aws/aws-sdk-go v1.37.19
github.com/banzaicloud/k8s-objectmatcher v1.1.0
github.com/ceph/go-ceph v0.10.1-0.20210729101705-11f319727ffb
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f
Expand All @@ -13,7 +13,10 @@ require (
github.com/go-ini/ini v1.51.1
github.com/google/go-cmp v0.5.5
github.com/google/uuid v1.1.2
github.com/hashicorp/vault/api v1.0.5-0.20200902155336-f9d5ce5a171a
github.com/hashicorp/vault v1.8.2
github.com/hashicorp/vault-plugin-secrets-kv v0.9.0
github.com/hashicorp/vault/api v1.1.2-0.20210713235431-1fc8af4c041f
github.com/hashicorp/vault/sdk v0.2.2-0.20210825150427-9b1f4d486f5d
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.1.0
github.com/kube-object-storage/lib-bucket-provisioner v0.0.0-20210818162813-3eee31c01875
github.com/libopenstorage/secrets v0.0.0-20210709082113-dde442ea20ec
Expand All @@ -27,7 +30,7 @@ require (
github.com/stretchr/testify v1.7.0
github.com/tevino/abool v1.2.0
github.com/yanniszark/go-nodetool v0.0.0-20191206125106-cd8f91fa16be
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
gopkg.in/ini.v1 v1.57.0
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.21.2
Expand Down
694 changes: 651 additions & 43 deletions go.sum

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/daemon/ceph/osd/kms/kms.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func GetParam(kmsConfig map[string]string, param string) string {
}

// ValidateConnectionDetails validates mandatory KMS connection details
func ValidateConnectionDetails(clusterdContext *clusterd.Context, securitySpec cephv1.SecuritySpec, ns string) error {
func ValidateConnectionDetails(clusterdContext *clusterd.Context, securitySpec *cephv1.SecuritySpec, ns string) error {
ctx := context.TODO()
// A token must be specified
if !securitySpec.KeyManagementService.IsTokenAuthEnabled() {
Expand Down
40 changes: 37 additions & 3 deletions pkg/daemon/ceph/osd/kms/kms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"os"
"testing"

"github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/vault"
cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
"github.com/rook/rook/pkg/clusterd"
"github.com/rook/rook/pkg/operator/test"
Expand All @@ -33,7 +35,7 @@ func TestValidateConnectionDetails(t *testing.T) {
ctx := context.TODO()
// Placeholder
context := &clusterd.Context{Clientset: test.New(t, 3)}
securitySpec := cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{ConnectionDetails: map[string]string{}}}
securitySpec := &cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{ConnectionDetails: map[string]string{}}}
ns := "rook-ceph"

// Error: no token in spec
Expand Down Expand Up @@ -69,7 +71,7 @@ func TestValidateConnectionDetails(t *testing.T) {
assert.EqualError(t, err, "failed to read k8s kms secret \"token\" key \"vault-token\" (not found or empty)")

// Success: token content is ok
s.Data["token"] = []byte("myt-otkenbenvqrev")
s.Data["token"] = []byte("token")
_, err = context.Clientset.CoreV1().Secrets(ns).Update(ctx, s, metav1.UpdateOptions{})
assert.NoError(t, err)
err = ValidateConnectionDetails(context, securitySpec, ns)
Expand All @@ -84,7 +86,6 @@ func TestValidateConnectionDetails(t *testing.T) {

// Error: connection details are correct but the token secret does not exist
securitySpec.KeyManagementService.ConnectionDetails["VAULT_ADDR"] = "https://1.1.1.1:8200"
securitySpec.KeyManagementService.ConnectionDetails["VAULT_BACKEND"] = "v1"

// Error: TLS is configured but secrets do not exist
securitySpec.KeyManagementService.ConnectionDetails["VAULT_CACERT"] = "vault-ca-secret"
Expand All @@ -111,6 +112,39 @@ func TestValidateConnectionDetails(t *testing.T) {
assert.NoError(t, err)
err = ValidateConnectionDetails(context, securitySpec, ns)
assert.NoError(t, err, "")

// test with vauult server
t.Run("success - auto detect kv version and set it", func(t *testing.T) {
cluster := fakeVaultServer(t)
cluster.Start()
defer cluster.Cleanup()
core := cluster.Cores[0].Core
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client
// Mock the client here
vaultClient = func(secretConfig map[string]string) (*api.Client, error) { return client, nil }
if err := client.Sys().Mount("rook/", &api.MountInput{
Type: "kv-v2",
Options: map[string]string{"version": "2"},
}); err != nil {
t.Fatal(err)
}
securitySpec := &cephv1.SecuritySpec{
KeyManagementService: cephv1.KeyManagementServiceSpec{
ConnectionDetails: map[string]string{
"VAULT_SECRET_ENGINE": "kv",
"KMS_PROVIDER": "vault",
"VAULT_ADDR": client.Address(),
"VAULT_BACKEND_PATH": "rook",
},
TokenSecretName: "vault-token",
},
}
err = ValidateConnectionDetails(context, securitySpec, ns)
assert.NoError(t, err, "")
assert.Equal(t, securitySpec.KeyManagementService.ConnectionDetails["VAULT_BACKEND"], "v2")
})

}

func TestSetTokenToEnvVar(t *testing.T) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/daemon/ceph/osd/kms/vault_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ const (
kvVersion2 = "kv-v2"
)

// vaultClient returns a vault client, also used in unit tests to mock the client
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(secretConfig map[string]string) (*api.Client, error) {
Expand Down Expand Up @@ -88,7 +91,7 @@ func BackendVersion(secretConfig map[string]string) (string, error) {
return v2, nil
default:
// Initialize Vault client
vaultClient, err := newVaultClient(secretConfig)
vaultClient, err := vaultClient(secretConfig)
if err != nil {
return "", errors.Wrap(err, "failed to initialize vault client")
}
Expand Down
93 changes: 93 additions & 0 deletions pkg/daemon/ceph/osd/kms/vault_api_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
Copyright 2021 The Rook Authors. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package kms

import (
"testing"

kv "github.com/hashicorp/vault-plugin-secrets-kv"
"github.com/hashicorp/vault/api"
vaulthttp "github.com/hashicorp/vault/http"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault"
)

func TestBackendVersion(t *testing.T) {
cluster := fakeVaultServer(t)
cluster.Start()
defer cluster.Cleanup()
core := cluster.Cores[0].Core
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client

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

// Set up the kv store
if err := client.Sys().Mount("rook/", &api.MountInput{
Type: "kv",
Options: map[string]string{"version": "1"},
}); err != nil {
t.Fatal(err)
}
if err := client.Sys().Mount("rookv2/", &api.MountInput{
Type: "kv-v2",
Options: map[string]string{"version": "2"},
}); err != nil {
t.Fatal(err)
}

type args struct {
secretConfig map[string]string
}
tests := []struct {
name string
args args
want string
wantErr bool
}{
{"v1 is set explicitly", args{map[string]string{"VAULT_BACKEND": "v1"}}, "v1", false},
{"v2 is set explicitly", args{map[string]string{"VAULT_BACKEND": "v2"}}, "v2", false},
{"v1 is set auto-discovered", args{map[string]string{"VAULT_ADDR": client.Address(), "VAULT_BACKEND_PATH": "rook"}}, "v1", false},
{"v2 is set auto-discovered", args{map[string]string{"VAULT_ADDR": client.Address(), "VAULT_BACKEND_PATH": "rookv2"}}, "v2", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := BackendVersion(tt.args.secretConfig)
if (err != nil) != tt.wantErr {
t.Errorf("BackendVersion() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("BackendVersion() = %v, want %v", got, tt.want)
}
})
}
}

func fakeVaultServer(t *testing.T) *vault.TestCluster {
cluster := vault.NewTestCluster(t, &vault.CoreConfig{
DevToken: "token",
LogicalBackends: map[string]logical.Factory{"kv": kv.Factory},
},
&vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
NumCores: 1,
})

return cluster
}
2 changes: 1 addition & 1 deletion pkg/operator/ceph/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ func preClusterStartValidation(cluster *cluster) error {
// Validate on-PVC cluster encryption KMS settings
if cluster.Spec.Storage.IsOnPVCEncrypted() && cluster.Spec.Security.KeyManagementService.IsEnabled() {
// Validate the KMS details
err := kms.ValidateConnectionDetails(cluster.context, cluster.Spec.Security, cluster.Namespace)
err := kms.ValidateConnectionDetails(cluster.context, &cluster.Spec.Security, cluster.Namespace)
if err != nil {
return errors.Wrap(err, "failed to validate kms connection details")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/ceph/object/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func (c *clusterConfig) vaultPrefixRGW() string {

func (c *clusterConfig) CheckRGWKMS() (bool, error) {
if c.store.Spec.Security != nil && c.store.Spec.Security.KeyManagementService.IsEnabled() {
err := kms.ValidateConnectionDetails(c.context, *c.store.Spec.Security, c.store.Namespace)
err := kms.ValidateConnectionDetails(c.context, c.store.Spec.Security, c.store.Namespace)
if err != nil {
return false, err
}
Expand Down

0 comments on commit 00c111b

Please sign in to comment.