weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #5591 from wallrj/fix-vault-namespace-rjw

Browse Source 
Set the Vault namespace using vault SDK client methods instead of using raw request object
This commit is contained in:
jetstack-bot 2022-11-23 11:34:54 +00:00 committed by GitHub
commit d85e424cd0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 221 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
internal/vault/fake/client.go
View File

@ -64,7 +64,3 @@ func (c *Client) Token() string {
func (c *Client) RawRequest(r *vault.Request) (*vault.Response, error) {
return c.RawRequestFn(r)
}
func (c *Client) Sys() *vault.Sys {
return nil
}

6
internal/vault/fake/vault.go
View File

@ -20,7 +20,6 @@ package fake
import (
"time"
vault "github.com/hashicorp/vault/api"
corelisters "k8s.io/client-go/listers/core/v1"
v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
@ -80,11 +79,6 @@ func (v *Vault) New(ns string, sl corelisters.SecretLister, iss v1.GenericIssuer
return v, nil
}
// Sys returns an empty `vault.Sys`.
func (v *Vault) Sys() *vault.Sys {
return new(vault.Sys)
}
// IsVaultInitializedAndUnsealed always returns nil
func (v *Vault) IsVaultInitializedAndUnsealed() error {
return nil

65
internal/vault/vault.go
View File

@ -45,10 +45,8 @@ type ClientBuilder func(namespace string, secretsLister corelisters.SecretLister
// Interface implements various high level functionality related to connecting
// with a Vault server, verifying its status and signing certificate request for
// Vault's certificate.
// TODO: Sys() is duplicated here and in Client interface
type Interface interface {
Sign(csrPEM []byte, duration time.Duration) (certPEM []byte, caPEM []byte, err error)
Sys() *vault.Sys
IsVaultInitializedAndUnsealed() error
}
@ -57,8 +55,6 @@ type Client interface {
NewRequest(method, requestPath string) *vault.Request
RawRequest(r *vault.Request) (*vault.Response, error)
SetToken(v string)
Token() string
Sys() *vault.Sys
}
// Vault implements Interface and holds a Vault issuer, secrets lister and a
@ -68,7 +64,22 @@ type Vault struct {
issuer v1.GenericIssuer
namespace string
// The pattern below, of namespaced and non-namespaced Vault clients, is copied from Hashicorp Nomad:
// https://github.com/hashicorp/nomad/blob/6e4410a9b13ce167bc7ef53da97c621b5c9dcd12/nomad/vault.go#L180-L190
// client is the Vault API client used for Namespace-relative integrations
// with the Vault API (anything except `/v1/sys`).
// The namespace feature is only available in Vault Enterprise.
// The namespace HTTP header (X-Vault-Namespace) is ignored by the open source version of Vault.
// See https://www.vaultproject.io/docs/enterprise/namespaces
client Client
// clientSys is the Vault API client used for non-Namespace-relative integrations
// with the Vault API (anything involving `/v1/sys`). This client is never configured
// with a Vault namespace, because these endpoints may return errors if a namespace
// header is provided
// See https://developer.hashicorp.com/vault/docs/enterprise/namespaces#root-only-api-paths
clientSys Client
}
// New returns a new Vault instance with the given namespace, issuer and
@ -92,11 +103,26 @@ func New(namespace string, secretsLister corelisters.SecretLister, issuer v1.Gen
return nil, fmt.Errorf("error initializing Vault client: %s", err.Error())
}
if err := v.setToken(client); err != nil {
// Set the Vault namespace.
// An empty namespace string will cause the client to not send the namespace related HTTP headers to Vault.
clientNS := client.WithNamespace(issuer.GetSpec().Vault.Namespace)
// Use the (maybe) namespaced client to authenticate.
// If a Vault namespace is configured, then the authentication endpoints are
// expected to be in that namespace.
if err := v.setToken(clientNS); err != nil {
return nil, err
}
v.client = client
// A client for use with namespaced API paths
v.client = clientNS
// Create duplicate Vault client without a namespace, for interacting with root-only API paths.
// For backwards compatibility, this client will use the token from the namespaced client,
// although this is probably unnecessary / bad practice, since we only
// interact with the sys/health endpoint which is an unauthenticated endpoint:
// https://github.com/hashicorp/vault/issues/209#issuecomment-102485565.
v.clientSys = clientNS.WithNamespace("")
return v, nil
}
@ -124,8 +150,6 @@ func (v *Vault) Sign(csrPEM []byte, duration time.Duration) (cert []byte, ca []b
request := v.client.NewRequest("POST", url)
v.addVaultNamespaceToRequest(request)
if err := request.SetJSONBody(parameters); err != nil {
return nil, nil, fmt.Errorf("failed to build vault request: %s", err)
}
@ -312,8 +336,6 @@ func (v *Vault) requestTokenWithAppRoleRef(client Client, appRole *v1.VaultAppRo
return "", fmt.Errorf("error encoding Vault parameters: %s", err.Error())
}
v.addVaultNamespaceToRequest(request)
resp, err := client.RawRequest(request)
if err != nil {
return "", fmt.Errorf("error logging in to Vault server: %s", err.Error())
@ -373,8 +395,6 @@ func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1
return "", fmt.Errorf("error encoding Vault parameters: %s", err.Error())
}
v.addVaultNamespaceToRequest(request)
resp, err := client.RawRequest(request)
if err != nil {
return "", fmt.Errorf("error calling Vault server: %s", err.Error())
@ -395,10 +415,6 @@ func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1
return token, nil
}
func (v *Vault) Sys() *vault.Sys {
return v.client.Sys()
}
func extractCertificatesFromVaultCertificateSecret(secret *certutil.Secret) ([]byte, []byte, error) {
parsedBundle, err := certutil.ParsePKIMap(secret.Data)
if err != nil {
@ -425,8 +441,8 @@ func extractCertificatesFromVaultCertificateSecret(secret *certutil.Secret) ([]b
func (v *Vault) IsVaultInitializedAndUnsealed() error {
healthURL := path.Join("/v1", "sys", "health")
healthRequest := v.client.NewRequest("GET", healthURL)
healthResp, err := v.client.RawRequest(healthRequest)
healthRequest := v.clientSys.NewRequest("GET", healthURL)
healthResp, err := v.clientSys.RawRequest(healthRequest)
if healthResp != nil {
defer healthResp.Body.Close()
@ -448,16 +464,3 @@ func (v *Vault) IsVaultInitializedAndUnsealed() error {
return nil
}
func (v *Vault) addVaultNamespaceToRequest(request *vault.Request) {
vaultIssuer := v.issuer.GetSpec().Vault
if vaultIssuer != nil && vaultIssuer.Namespace != "" {
if request.Headers != nil {
request.Headers.Add("X-VAULT-NAMESPACE", vaultIssuer.Namespace)
} else {
vaultReqHeaders := http.Header{}
vaultReqHeaders.Add("X-VAULT-NAMESPACE", vaultIssuer.Namespace)
request.Headers = vaultReqHeaders
}
}
}

187
internal/vault/vault_test.go
View File

@ -25,6 +25,7 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
@ -32,11 +33,15 @@ import (
vault "github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/helper/jsonutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientcorev1 "k8s.io/client-go/listers/core/v1"
vaultfake "github.com/cert-manager/cert-manager/internal/vault/fake"
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
"github.com/cert-manager/cert-manager/pkg/util/pki"
"github.com/cert-manager/cert-manager/test/unit/gen"
@ -1169,3 +1174,185 @@ func TestRequestTokenWithAppRoleRef(t *testing.T) {
})
}
}
// TestNewWithVaultNamespaces demonstrates that New initializes two Vault
// clients, one with a namespace and one without a namespace which is used for
// interacting with root-only APIs.
func TestNewWithVaultNamespaces(t *testing.T) {
type testCase struct {
name string
vaultNS string
}
tests := []testCase{
{
name: "without-namespace",
vaultNS: "",
},
{
name: "with-namespace",
vaultNS: "vault-ns-1",
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c, err := New(
"k8s-ns1",
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(
&corev1.Secret{
Data: map[string][]byte{
"key1": []byte("not-used"),
},
}, nil),
),
&cmapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Namespace: "k8s-ns1",
},
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
Vault: &v1.VaultIssuer{
Namespace: tc.vaultNS,
Auth: cmapi.VaultAuth{
TokenSecretRef: &cmmeta.SecretKeySelector{
LocalObjectReference: cmmeta.LocalObjectReference{
Name: "secret1",
},
Key: "key1",
},
},
},
},
},
})
require.NoError(t, err)
assert.Equal(t, tc.vaultNS, c.(*Vault).client.(*vault.Client).Namespace(),
"The vault client should have the namespace provided in the Issuer recource")
assert.Equal(t, "", c.(*Vault).clientSys.(*vault.Client).Namespace(),
"The vault sys client should never have a namespace")
})
}
}
// TestIsVaultInitiatedAndUnsealedIntegration demonstrates that it interacts only with the
// sys/health endpoint and that it supplies the Vault token but not a Vault namespace header.
func TestIsVaultInitiatedAndUnsealedIntegration(t *testing.T) {
const vaultToken = "token1"
mux := http.NewServeMux()
mux.HandleFunc("/v1/sys/health", func(response http.ResponseWriter, request *http.Request) {
assert.Empty(t, request.Header.Values("X-Vault-Namespace"), "Unexpected Vault namespace header for root-only API path")
assert.Equal(t, vaultToken, request.Header.Get("X-Vault-Token"), "Expected the Vault token for root-only API path")
})
server := httptest.NewServer(mux)
defer server.Close()
v, err := New(
"k8s-ns1",
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(
&corev1.Secret{
Data: map[string][]byte{
"key1": []byte(vaultToken),
},
}, nil),
),
&cmapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Namespace: "k8s-ns1",
},
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
Vault: &v1.VaultIssuer{
Server: server.URL,
Namespace: "ns1",
Auth: cmapi.VaultAuth{
TokenSecretRef: &cmmeta.SecretKeySelector{
LocalObjectReference: cmmeta.LocalObjectReference{
Name: "secret1",
},
Key: "key1",
},
},
},
},
},
})
require.NoError(t, err)
err = v.IsVaultInitializedAndUnsealed()
require.NoError(t, err)
}
// TestSignIntegration demonstrates that it interacts only with the API endpoint
// path supplied in the Issuer resource and that it supplies the Vault namespace
// and token to that endpoint.
func TestSignIntegration(t *testing.T) {
const (
vaultToken = "token1"
vaultNamespace = "vault-ns-1"
vaultPath = "my_pki_mount/sign/my-role-name"
)
privatekey := generateRSAPrivateKey(t)
csrPEM := generateCSR(t, privatekey)
rootBundleData, err := bundlePEM(testIntermediateCa, testRootCa)
require.NoError(t, err)
mux := http.NewServeMux()
mux.HandleFunc(fmt.Sprintf("/v1/%s", vaultPath), func(response http.ResponseWriter, request *http.Request) {
assert.Equal(t, vaultNamespace, request.Header.Get("X-Vault-Namespace"), "Expected Vault namespace header for namespaced API path")
assert.Equal(t, vaultToken, request.Header.Get("X-Vault-Token"), "Expected the Vault token for root-only API path")
_, err := response.Write(rootBundleData)
require.NoError(t, err)
})
server := httptest.NewServer(mux)
defer server.Close()
v, err := New(
"k8s-ns1",
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(
&corev1.Secret{
Data: map[string][]byte{
"key1": []byte(vaultToken),
},
}, nil),
),
&cmapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Namespace: "k8s-ns1",
},
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
Vault: &v1.VaultIssuer{
Server: server.URL,
Path: vaultPath,
Namespace: vaultNamespace,
Auth: cmapi.VaultAuth{
TokenSecretRef: &cmmeta.SecretKeySelector{
LocalObjectReference: cmmeta.LocalObjectReference{
Name: "secret1",
},
Key: "key1",
},
},
},
},
},
})
require.NoError(t, err)
certPEM, caPEM, err := v.Sign(csrPEM, time.Hour)
require.NoError(t, err)
require.NotEmpty(t, certPEM)
require.NotEmpty(t, caPEM)
}