diff --git a/pkg/controller/certificaterequests/BUILD.bazel b/pkg/controller/certificaterequests/BUILD.bazel index 720d80b70..812a0c236 100644 --- a/pkg/controller/certificaterequests/BUILD.bazel +++ b/pkg/controller/certificaterequests/BUILD.bazel @@ -69,6 +69,7 @@ filegroup( "//pkg/controller/certificaterequests/ca:all-srcs", "//pkg/controller/certificaterequests/fake:all-srcs", "//pkg/controller/certificaterequests/selfsigned:all-srcs", + "//pkg/controller/certificaterequests/test:all-srcs", "//pkg/controller/certificaterequests/util:all-srcs", "//pkg/controller/certificaterequests/vault:all-srcs", ], diff --git a/pkg/controller/certificaterequests/ca/BUILD.bazel b/pkg/controller/certificaterequests/ca/BUILD.bazel index 312bc6ca7..52241ceae 100644 --- a/pkg/controller/certificaterequests/ca/BUILD.bazel +++ b/pkg/controller/certificaterequests/ca/BUILD.bazel @@ -21,20 +21,6 @@ go_library( ], ) -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) - go_test( name = "go_default_test", srcs = ["ca_test.go"], @@ -53,3 +39,17 @@ go_test( "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], ) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/controller/certificaterequests/test/BUILD.bazel b/pkg/controller/certificaterequests/test/BUILD.bazel new file mode 100644 index 000000000..c3512f25b --- /dev/null +++ b/pkg/controller/certificaterequests/test/BUILD.bazel @@ -0,0 +1,28 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["util.go"], + importpath = "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/test", + visibility = ["//visibility:public"], + deps = [ + "//pkg/apis/certmanager/v1alpha1:go_default_library", + "//pkg/controller/test:go_default_library", + "//pkg/issuer:go_default_library", + "//pkg/util/pki:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/controller/certificaterequests/test/util.go b/pkg/controller/certificaterequests/test/util.go new file mode 100644 index 000000000..cecae3a6c --- /dev/null +++ b/pkg/controller/certificaterequests/test/util.go @@ -0,0 +1,126 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +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 test + +import ( + "bytes" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/pem" + "reflect" + "testing" + "time" + + "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + testpkg "github.com/jetstack/cert-manager/pkg/controller/test" + "github.com/jetstack/cert-manager/pkg/issuer" + "github.com/jetstack/cert-manager/pkg/util/pki" +) + +func GenerateRSAPrivateKey(t *testing.T) *rsa.PrivateKey { + pk, err := pki.GenerateRSAPrivateKey(2048) + if err != nil { + t.Errorf("failed to generate private key: %v", err) + t.FailNow() + } + return pk +} + +func GenerateCSR(t *testing.T, secretKey crypto.Signer) []byte { + asn1Subj, _ := asn1.Marshal(pkix.Name{ + CommonName: "test", + }.ToRDNSequence()) + template := x509.CertificateRequest{ + RawSubject: asn1Subj, + SignatureAlgorithm: x509.SHA256WithRSA, + } + + csrBytes, err := x509.CreateCertificateRequest(rand.Reader, &template, secretKey) + if err != nil { + t.Error(err) + t.FailNow() + } + + csr := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes}) + + return csr +} + +func MustNoResponse(builder *testpkg.Builder, args ...interface{}) { + resp := args[0].(*issuer.IssueResponse) + if resp != nil { + builder.T.Errorf("unexpected response, exp='nil' got='%+v'", resp) + } +} + +func NoPrivateKeyFieldsSetCheck(expectedCA []byte) func(builder *testpkg.Builder, args ...interface{}) { + return func(builder *testpkg.Builder, args ...interface{}) { + resp := args[0].(*issuer.IssueResponse) + + if resp == nil { + builder.T.Errorf("no response given, got=%s", resp) + return + } + + if len(resp.PrivateKey) > 0 { + builder.T.Errorf("expected no new private key to be generated but got: %s", + resp.PrivateKey) + } + + CertificatesFieldsSetCheck(expectedCA)(builder, args...) + } +} + +func CertificatesFieldsSetCheck(expectedCA []byte) func(builder *testpkg.Builder, args ...interface{}) { + return func(builder *testpkg.Builder, args ...interface{}) { + resp := args[0].(*issuer.IssueResponse) + + if resp.Certificate == nil { + builder.T.Errorf("expected new certificate to be issued") + } + if resp.CA == nil || !reflect.DeepEqual(expectedCA, resp.CA) { + builder.T.Errorf("expected CA certificate to be returned") + } + } +} + +func GenerateSelfSignedCertFromCR(t *testing.T, cr *v1alpha1.CertificateRequest, key crypto.Signer, + duration time.Duration) (derBytes, pemBytes []byte) { + template, err := pki.GenerateTemplateFromCertificateRequest(cr) + if err != nil { + t.Errorf("error generating template: %v", err) + } + + derBytes, err = x509.CreateCertificate(rand.Reader, template, template, key.Public(), key) + if err != nil { + t.Errorf("error signing cert: %v", err) + t.FailNow() + } + + pemByteBuffer := bytes.NewBuffer([]byte{}) + err = pem.Encode(pemByteBuffer, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) + if err != nil { + t.Errorf("failed to encode cert: %v", err) + t.FailNow() + } + + return derBytes, pemByteBuffer.Bytes() +} diff --git a/pkg/controller/certificaterequests/vault/BUILD.bazel b/pkg/controller/certificaterequests/vault/BUILD.bazel index 6c9088ded..a23a3aa04 100644 --- a/pkg/controller/certificaterequests/vault/BUILD.bazel +++ b/pkg/controller/certificaterequests/vault/BUILD.bazel @@ -11,11 +11,11 @@ go_library( "//pkg/controller:go_default_library", "//pkg/controller/certificaterequests:go_default_library", "//pkg/controller/certificaterequests/util:go_default_library", + "//pkg/internal:go_default_library", + "//pkg/internal/vault:go_default_library", "//pkg/issuer:go_default_library", "//pkg/logs:go_default_library", "//pkg/util/pki:go_default_library", - "//vendor/github.com/hashicorp/vault/api:go_default_library", - "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", @@ -41,10 +41,14 @@ go_test( srcs = ["vault_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/apis/certmanager:go_default_library", "//pkg/apis/certmanager/v1alpha1:go_default_library", - "//pkg/controller/test/fake:go_default_library", + "//pkg/controller/certificaterequests/test:go_default_library", + "//pkg/controller/test:go_default_library", + "//pkg/internal/vault/fake:go_default_library", "//test/unit/gen:go_default_library", - "//vendor/github.com/hashicorp/vault/api:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", ], ) diff --git a/pkg/controller/certificaterequests/vault/vault.go b/pkg/controller/certificaterequests/vault/vault.go index 9162ef2c0..878248538 100644 --- a/pkg/controller/certificaterequests/vault/vault.go +++ b/pkg/controller/certificaterequests/vault/vault.go @@ -29,6 +29,7 @@ import ( controllerpkg "github.com/jetstack/cert-manager/pkg/controller" "github.com/jetstack/cert-manager/pkg/controller/certificaterequests" crutil "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util" + "github.com/jetstack/cert-manager/pkg/internal" vaultinternal "github.com/jetstack/cert-manager/pkg/internal/vault" "github.com/jetstack/cert-manager/pkg/issuer" logf "github.com/jetstack/cert-manager/pkg/logs" @@ -44,6 +45,8 @@ type Vault struct { recorder record.EventRecorder secretsLister corelisters.SecretLister helper issuer.Helper + + vaultFactory internal.VaultFactory } func init() { @@ -70,6 +73,7 @@ func NewVault(ctx *controllerpkg.Context) *Vault { ctx.SharedInformerFactory.Certmanager().V1alpha1().Issuers().Lister(), ctx.SharedInformerFactory.Certmanager().V1alpha1().ClusterIssuers().Lister(), ), + vaultFactory: vaultinternal.New, } } @@ -86,13 +90,14 @@ func (v *Vault) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest) (*iss ) if k8sErrors.IsNotFound(err) { - reporter.WithLog(log).Pending(err, v1alpha1.CertificateRequestReasonPending, - fmt.Sprintf("Referenced %s not found", apiutil.IssuerKind(cr.Spec.IssuerRef))) + reporter.WithLog(log).Pending(err, "ErrorMissingIssuer", + fmt.Sprintf("Referenced %q not found", apiutil.IssuerKind(cr.Spec.IssuerRef))) return nil, nil } - reporter.WithLog(log).Pending(err, v1alpha1.CertificateRequestReasonPending, - fmt.Sprintf("Referenced %s not found", apiutil.IssuerKind(cr.Spec.IssuerRef))) + // We are probably in a network error here so we should backoff and retry + reporter.Pending(err, "ErrorGettingIssuer", + fmt.Sprintf("Failed to get referenced Issuer %q", cr.Spec.IssuerRef.Name)) return nil, err } @@ -103,9 +108,20 @@ func (v *Vault) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest) (*iss return nil, nil } - client, err := vaultinternal.New(cr.Namespace, v.secretsLister, issuerObj) + client, err := v.vaultFactory(cr.Namespace, v.secretsLister, issuerObj) if err != nil { - reporter.Pending(err, "ErrorVaultInit", + log = log.WithValues( + logf.RelatedResourceNameKey, cr.Spec.IssuerRef.Name, + logf.RelatedResourceKindKey, cr.Spec.IssuerRef.Kind, + ) + + if k8sErrors.IsNotFound(err) { + reporter.WithLog(log).Pending(err, "MissingSecret", + fmt.Sprintf("Required resource not found: %s", err)) + return nil, nil + } + + reporter.WithLog(log).Pending(err, "ErrorVaultInit", fmt.Sprintf("Failed to initialise vault client for signing: %s", err)) return nil, nil } diff --git a/pkg/controller/certificaterequests/vault/vault_test.go b/pkg/controller/certificaterequests/vault/vault_test.go index 7feb4b589..4be2dfdcb 100644 --- a/pkg/controller/certificaterequests/vault/vault_test.go +++ b/pkg/controller/certificaterequests/vault/vault_test.go @@ -17,477 +17,285 @@ limitations under the License. package vault import ( - "bytes" - "crypto/tls" - "crypto/x509" + "context" "errors" - "fmt" - "net/http" - "reflect" - "strings" "testing" + "time" - vault "github.com/hashicorp/vault/api" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "github.com/jetstack/cert-manager/pkg/apis/certmanager" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" - testfake "github.com/jetstack/cert-manager/pkg/controller/test/fake" + testcr "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/test" + testpkg "github.com/jetstack/cert-manager/pkg/controller/test" + fakevault "github.com/jetstack/cert-manager/pkg/internal/vault/fake" "github.com/jetstack/cert-manager/test/unit/gen" ) -const ( - testCertBundle = `-----BEGIN CERTIFICATE----- -MIIDBTCCAe2gAwIBAgIUAR4qkcRVjl3D2ZNXyAjGrmJJafUwDQYJKoZIhvcNAQEL -BQAwEjEQMA4GA1UEAwwHZm9vLmJhcjAeFw0xOTA2MTExMTQ4MjZaFw0yOTA2MDgx -MTQ4MjZaMBIxEDAOBgNVBAMMB2Zvby5iYXIwggEiMA0GCSqGSIb3DQEBAQUAA4IB -DwAwggEKAoIBAQC6nycZ01dEcR1xaMdhP7HWeHEZVTCMBkvk9NJ7CjHBjEcRFPbo -koMfIeuQ2lO+mFXpLo9iJOE+fh+Pl8/vNihS9Xan23EFNYGNukmpup4zcZ5sBueA -sE9A1LwuHxCIhwutvSOatfzbw5i4LrXNncIRabNjHmJgd4j7hhRJF0PR3x5uTV0t -lMsPVtBUX2FehR3ZvJBaYRFk4ITa7wX8a9p2JQeavoeoSxX2UWGxdE9v2oMUU0Sn -+LjzoNHVWzkTZv5yn8X3GKS1Co4bWaeDmZywL8HSkK//ST/rk7UDItWgeetMRvTt -UO1xLjEYU4HO4aPEdmwVha58nzS87pdJm+LfAgMBAAGjUzBRMB0GA1UdDgQWBBR0 -B9MwNgun4l7JAyd2tqL24oRmGDAfBgNVHSMEGDAWgBR0B9MwNgun4l7JAyd2tqL2 -4oRmGDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAOofYo23Xv -I5fh1sg4cmayLU5TSZ1hv9/qLzqYDu/9MSJtY0ww8RotkZOL5E3sphh8JQfKnj0G -0NvJrq6RP3Bd9FfizF2k1y2Z6D/dorztd5uum6ctdylfBPgeZEemv9aCfdigAwd+ -nh5C+XrIPnsN7Xeq3N4gzyLVzdkFHbuMWTqmqJo5XaMEWP3/dzPl447z/QlSXVqe -nCSne2t3DgvoiqS+A1hVLzHeEiwwd9kmQdPUrybwXZ/i6B1sfcxf8eklbiuhtunQ -jy1M5ZaOOfj2WFwmydx1ycGdJbJiKppN3oehi7EJ2lAxwbGoKy4VD4Ks/nMu4TEY -2lUQ3SmEzoFL ------END CERTIFICATE-----` -) +func TestSign(t *testing.T) { + rsaPK := testcr.GenerateRSAPrivateKey(t) + caCSR := testcr.GenerateCSR(t, rsaPK) -type testAppRoleRefT struct { - expectedRoleID string - expectedSecretID string - expectedErr error + testCR := gen.CertificateRequest("test-cr", + gen.SetCertificateRequestCSR(caCSR), + gen.SetCertificateRequestIsCA(true), + gen.SetCertificateRequestDuration(&metav1.Duration{Duration: time.Hour * 24 * 60}), + gen.SetCertificateRequestIssuer(v1alpha1.ObjectReference{ + Name: "vault-issuer", + Group: certmanager.GroupName, + Kind: "Issuer", + }), + ) - appRole *v1alpha1.VaultAppRole - certificaterequest *v1alpha1.CertificateRequest + _, rsaPEMCert := testcr.GenerateSelfSignedCertFromCR(t, testCR, rsaPK, time.Hour*24*60) - fakeLister *testfake.FakeSecretLister -} - -func TestAppRoleRef(t *testing.T) { - errSecretGet := errors.New("no secret found") - - basicAppRoleRef := &v1alpha1.VaultAppRole{ - RoleId: "my-role-id", + tokenSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: gen.DefaultTestNamespace, + Name: "token-secret", + }, + Data: map[string][]byte{ + "my-token-key": []byte("my-secret-token"), + }, } - tests := map[string]testAppRoleRefT{ - "failing to get secret should error": { - appRole: basicAppRoleRef, - certificaterequest: gen.CertificateRequest("test", - gen.SetCertificateRequestNamespace("test-namespace"), - ), - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet(nil, errSecretGet), - ), - expectedRoleID: "", - expectedSecretID: "", - expectedErr: errSecretGet, + roleSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: gen.DefaultTestNamespace, + Name: "role-secret", }, + Data: map[string][]byte{ + "my-role-key": []byte("my-secret-role"), + }, + } - "no data in key should fail": { - appRole: &v1alpha1.VaultAppRole{ - RoleId: "", - SecretRef: v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "secret-name", - }, - Key: "my-key", + tests := map[string]testT{ + "if the issuer does not exist then report pending": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{}, + ExpectedEvents: []string{ + `Normal ErrorMissingIssuer Referenced "Issuer" not found: issuer.certmanager.k8s.io "vault-issuer" not found`, }, + CheckFn: testcr.MustNoResponse, }, - certificaterequest: gen.CertificateRequest("test", - gen.SetCertificateRequestNamespace("test-namespace"), - ), - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "foo": []byte("bar"), - }, - }, nil), - ), - expectedRoleID: "", - expectedSecretID: "", - expectedErr: errors.New(`no data for "my-key" in secret 'test-namespace/secret-name'`), + expectedErr: false, }, - - "should return roleID and secretID with trimmed space": { - appRole: &v1alpha1.VaultAppRole{ - RoleId: " my-role-id ", - SecretRef: v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "secret-name", - }, - Key: "my-key", + "a badly formed CSR should report failure": { + certificateRequest: gen.CertificateRequestFrom(testCR, + gen.SetCertificateRequestCSR([]byte("a bad csr")), + ), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer")}, + ExpectedEvents: []string{ + `Warning ErrorParsingCSR Failed to decode CSR in spec: error decoding certificate request PEM block: error decoding certificate request PEM block`, }, + CheckFn: testcr.MustNoResponse, }, - certificaterequest: gen.CertificateRequest("test", - gen.SetCertificateRequestNamespace("test-namespace"), - ), - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "foo": []byte("bar"), - "my-key": []byte(" my-key-data "), - }, - }, nil), - ), - expectedRoleID: "my-role-id", - expectedSecretID: "my-key-data", - expectedErr: nil, + expectedErr: false, }, - } - - for name, test := range tests { - v := &Vault{ - secretsLister: test.fakeLister, - } - - roleID, secretID, err := v.appRoleRef(test.certificaterequest, test.appRole) - if !reflect.DeepEqual(test.expectedErr, err) { - t.Errorf("%s: unexpected error, exp=%v got=%v", - name, test.expectedErr, err) - } - - if test.expectedRoleID != roleID { - t.Errorf("%s: got unexpected roleID, exp=%s got=%s", - name, test.expectedRoleID, roleID) - } - - if test.expectedSecretID != secretID { - t.Errorf("%s: got unexpected secretID, exp=%s got=%s", - name, test.expectedSecretID, secretID) - } - } -} - -type testTokenRefT struct { - expectedToken string - expectedErr error - - key string - - fakeLister *testfake.FakeSecretLister -} - -func TestTokenRef(t *testing.T) { - errSecretGet := errors.New("no secret found") - - testName, testNamespace := "test-name", "test-namespace" - - tests := map[string]testTokenRefT{ - "failing to get secret should error": { - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet(nil, errSecretGet), - ), - key: "a-key", - expectedToken: "", - expectedErr: errSecretGet, - }, - - "if no vault at key exists then error": { - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "foo": []byte("bar"), - }, - }, nil), - ), - - key: "a-key", - expectedToken: "", - expectedErr: fmt.Errorf(`no data for "a-key" in secret '%s/%s'`, - testName, testNamespace), - }, - "if value exists at key then return with whitespace trimmed": { - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "foo": []byte("bar"), - "a-key": []byte(" my-token "), - }, - }, nil), - ), - - key: "a-key", - expectedToken: "my-token", - }, - "if no key is given then it should default to 'token'": { - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "foo": []byte("bar"), - "token": []byte(" my-token "), - }, - }, nil), - ), - - key: "", - expectedToken: "my-token", - }, - } - - for name, test := range tests { - v := &Vault{ - secretsLister: test.fakeLister, - } - - token, err := v.tokenRef("test-name", "test-namespace", test.key) - if !reflect.DeepEqual(test.expectedErr, err) { - t.Errorf("%s: unexpected error, exp=%v got=%v", - name, test.expectedErr, err) - } - - if test.expectedToken != token { - t.Errorf("%s: got unexpected token, exp=%s got=%s", - name, test.expectedToken, token) - } - } -} - -type testConfigureCertPoolT struct { - expectedErr error - issuer *v1alpha1.Issuer - checkFunc func(cfg *vault.Config) error -} - -func TestConfigureCertPool(t *testing.T) { - tests := map[string]testConfigureCertPoolT{ - "no CA bundle set in issuer should return nil": { - issuer: gen.Issuer("vault-issuer", - gen.SetIssuerVault(v1alpha1.VaultIssuer{ - CABundle: nil, - }), - ), - expectedErr: nil, - }, - - "a bad cert bundle should error": { - issuer: gen.Issuer("vault-issuer", - gen.SetIssuerVault(v1alpha1.VaultIssuer{ - CABundle: []byte("a bad cert bundle"), - }), - ), - expectedErr: errors.New("error loading Vault CA bundle"), - }, - - "a good cert bundle should be added to the config": { - issuer: gen.Issuer("vault-issuer", - gen.SetIssuerVault(v1alpha1.VaultIssuer{ - CABundle: []byte(testCertBundle), - }), - ), - expectedErr: nil, - checkFunc: func(cfg *vault.Config) error { - testCA := x509.NewCertPool() - testCA.AppendCertsFromPEM([]byte(testCertBundle)) - subs := cfg.HttpClient.Transport.(*http.Transport).TLSClientConfig.RootCAs.Subjects() - - err := fmt.Errorf("got unexpected root CAs in config, exp=%s got=%s", - testCA.Subjects(), subs) - if len(subs) != len(testCA.Subjects()) { - return err - } - for i := range subs { - if !bytes.Equal(subs[i], testCA.Subjects()[i]) { - return err - } - } - - return nil + "no token or app role secret reference should report pending": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{}), + )}, + ExpectedEvents: []string{ + `Normal ErrorVaultInit Failed to initialise vault client for signing: error initializing Vault client tokenSecretRef or appRoleSecretRef not set: error initializing Vault client tokenSecretRef or appRoleSecretRef not set`, + }, + CheckFn: testcr.MustNoResponse, }, + expectedErr: false, }, - } - - for name, test := range tests { - v := new(Vault) - httpClient := http.DefaultClient - httpClient.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{}, - } - - cfg := vault.Config{ - HttpClient: http.DefaultClient, - } - - err := v.configureCertPool(&cfg, test.issuer) - if !reflect.DeepEqual(test.expectedErr, err) { - t.Errorf("%s: unexpected error, exp=%v got=%v", - name, test.expectedErr, err) - } - - if test.checkFunc != nil { - if err := test.checkFunc(&cfg); err != nil { - t.Errorf("%s: check function failed: %s", - name, err) - } - } - } -} - -type testInitVaultClientT struct { - expectedErr error - certificaterequest *v1alpha1.CertificateRequest - issuer v1alpha1.GenericIssuer -} - -func TestInitVaultClient(t *testing.T) { - tests := map[string]testInitVaultClientT{ - "a issuer with a bad CA cert should error": { - issuer: gen.Issuer("vault-issuer", - gen.SetIssuerVault(v1alpha1.VaultIssuer{ - CABundle: []byte("bad cert"), - }), - ), - expectedErr: errors.New("error loading Vault CA bundle"), - }, - "a vault issuer with no token or role secret reference should error": { - issuer: gen.Issuer("vault-issuer", - gen.SetIssuerVault(v1alpha1.VaultIssuer{ - CABundle: []byte(testCertBundle), - }), - ), - expectedErr: errors.New("error initializing Vault client. tokenSecretRef or appRoleSecretRef not set"), - }, - - //TODO: - // - test for when tokenRef.Name != "" - // - test for when appRole.RoleId != "" - // - test for when appRole.RoleId != "" and tokenRef.Name != "" - - //"foo": { - // issuer: gen.Issuer("vault-issuer", - // gen.SetIssuerVault(v1alpha1.VaultIssuer{ - // CABundle: []byte(testCertBundle), - // Auth: v1alpha1.VaultAuth{ - // AppRole: v1alpha1.VaultAppRole{ - // RoleId: "my-role-id", - // }, - // }, - // }), - // ), - // expectedErr: nil, - //}, - } - - for name, test := range tests { - v := new(Vault) - - _, err := v.initVaultClient(test.certificaterequest, test.issuer) - if !reflect.DeepEqual(test.expectedErr, err) { - t.Errorf("%s: unexpected error, exp=%v got=%v", - name, test.expectedErr, err) - } - } -} - -type requestTokenWithAppRoleRefT struct { - client *vault.Client - appRole *v1alpha1.VaultAppRole - - certificaterequest *v1alpha1.CertificateRequest - fakeLister *testfake.FakeSecretLister - - expectedToken string - expectedErr error -} - -func TestRequestTokenWithAppRoleRef(t *testing.T) { - basicAppRoleRef := &v1alpha1.VaultAppRole{ - RoleId: "test-role-id", - SecretRef: v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-secret", - }, - Key: "my-key", - }, - } - - tests := map[string]requestTokenWithAppRoleRefT{ - "a secret reference that does not exist should error": { - client: nil, - appRole: basicAppRoleRef, - certificaterequest: gen.CertificateRequest("test", - gen.SetCertificateRequestNamespace("test-namespace"), - ), - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet(nil, errors.New("secret not found")), - ), - - expectedToken: "", - expectedErr: errors.New("error reading Vault AppRole from secret: test-namespace/test-secret: secret not found"), - }, - "foo": { - //client: &vault.NewClient(&vault.Config{ - // HttpClient: http.DefaultClient, - //}), - client: nil, - appRole: basicAppRoleRef, - certificaterequest: gen.CertificateRequest("test", - gen.SetCertificateRequestNamespace("test-namespace"), - ), - fakeLister: gen.FakeSecretListerFrom(testfake.NewFakeSecretLister(), - gen.SetFakeSecretNamespaceListerGet( - &corev1.Secret{ - Data: map[string][]byte{ - "my-key": []byte("my-key-data"), + "a client with a token secret referenced that doesn't exist should report pending": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + TokenSecretRef: v1alpha1.SecretKeySelector{ + Key: "secret-key", + LocalObjectReference: v1alpha1.LocalObjectReference{ + "non-existing-secret", + }, + }, }, - }, nil), - ), - - expectedToken: "", - expectedErr: errors.New("error reading Vault AppRole from secret: test-namespace/test-secret: secret not found"), + }), + )}, + ExpectedEvents: []string{ + `Normal MissingSecret Required resource not found: secret "non-existing-secret" not found: secret "non-existing-secret" not found`, + }, + CheckFn: testcr.MustNoResponse, + }, + expectedErr: false, + }, + "a client with a app role secret referenced that doesn't exist should report pending": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + AppRole: v1alpha1.VaultAppRole{ + RoleId: "my-role-id", + SecretRef: v1alpha1.SecretKeySelector{ + Key: "secret-key", + LocalObjectReference: v1alpha1.LocalObjectReference{ + "non-existing-secret", + }, + }, + }, + }, + }), + )}, + ExpectedEvents: []string{ + `Normal MissingSecret Required resource not found: secret "non-existing-secret" not found: secret "non-existing-secret" not found`, + }, + CheckFn: testcr.MustNoResponse, + }, + expectedErr: false, + }, + "a client with a token secret referenced with token but failed to sign should report fail": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{tokenSecret}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + TokenSecretRef: v1alpha1.SecretKeySelector{ + Key: "my-token-key", + LocalObjectReference: v1alpha1.LocalObjectReference{ + "token-secret", + }, + }, + }, + }), + )}, + ExpectedEvents: []string{ + `Warning ErrorSigning Vault failed to sign certificate: failed to sign: failed to sign`, + }, + CheckFn: testcr.MustNoResponse, + }, + fakeVault: fakevault.NewFakeVault().WithSign(nil, nil, errors.New("failed to sign")), + expectedErr: false, + }, + "a client with a app role secret referenced with role but failed to sign should report fail": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{roleSecret}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + AppRole: v1alpha1.VaultAppRole{ + RoleId: "my-role-id", + SecretRef: v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + "role-secret", + }, + Key: "my-role-key", + }, + }, + }, + }), + )}, + ExpectedEvents: []string{ + `Warning ErrorSigning Vault failed to sign certificate: failed to sign: failed to sign`, + }, + CheckFn: testcr.MustNoResponse, + }, + fakeVault: fakevault.NewFakeVault().WithNoOpNew().WithSign(nil, nil, errors.New("failed to sign")), + expectedErr: false, + }, + "a client with a token secret referenced with token and signs should return certificate": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{tokenSecret}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + TokenSecretRef: v1alpha1.SecretKeySelector{ + Key: "my-token-key", + LocalObjectReference: v1alpha1.LocalObjectReference{ + "token-secret", + }, + }, + }, + }), + )}, + ExpectedEvents: []string{}, + CheckFn: testcr.NoPrivateKeyFieldsSetCheck(rsaPEMCert), + }, + fakeVault: fakevault.NewFakeVault().WithSign(rsaPEMCert, rsaPEMCert, nil), + expectedErr: false, + }, + "a client with a app role secret referenced with role should return certificate": { + certificateRequest: testCR, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{roleSecret}, + CertManagerObjects: []runtime.Object{gen.Issuer("vault-issuer", + gen.SetIssuerVault(v1alpha1.VaultIssuer{ + Auth: v1alpha1.VaultAuth{ + AppRole: v1alpha1.VaultAppRole{ + RoleId: "my-role-id", + SecretRef: v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + "role-secret", + }, + Key: "my-role-key", + }, + }, + }, + }), + )}, + ExpectedEvents: []string{}, + CheckFn: testcr.NoPrivateKeyFieldsSetCheck(rsaPEMCert), + }, + fakeVault: fakevault.NewFakeVault().WithNoOpNew().WithSign(rsaPEMCert, rsaPEMCert, nil), + expectedErr: false, }, } - f := func(w http.ResponseWriter, r *http.Request) { - message := r.URL.Path - message = strings.TrimPrefix(message, "/") - message = "Hello " + message - w.Write([]byte(message)) - } - - http.HandleFunc("/", f) - go func() { - if err := http.ListenAndServeTLS("127.0.0.1:8443") - if err := http.ListenAndServe("127.0.0.1:8443", nil); err != nil { - t.Error(err) - t.FailNow() - } - }() - for name, test := range tests { - v := &Vault{ - secretsLister: test.fakeLister, - } - - client, err := vault.NewClient(&vault.Config{ - HttpClient: http.DefaultClient, - Address: "https://127.0.0.1:8443", + t.Run(name, func(t *testing.T) { + runTest(t, test) }) - - if err != nil { - t.Errorf("%s: failed to build vault client: %s", name, err) - continue - } - - token, err := v.requestTokenWithAppRoleRef(test.certificaterequest, client, test.appRole) - if !reflect.DeepEqual(test.expectedErr, err) { - t.Errorf("%s: unexpected error, exp=%v got=%v", - name, test.expectedErr, err) - } - - if test.expectedToken != token { - t.Errorf("%s: got unexpected token, exp=%s got=%s", - name, test.expectedToken, token) - } } } + +type testT struct { + builder *testpkg.Builder + certificateRequest *v1alpha1.CertificateRequest + + expectedErr bool + + fakeVault *fakevault.Vault +} + +func runTest(t *testing.T, test testT) { + test.builder.T = t + test.builder.Start() + defer test.builder.Stop() + + v := NewVault(test.builder.Context) + + if test.fakeVault != nil { + v.vaultFactory = test.fakeVault.New + } + + test.builder.Sync() + + resp, err := v.Sign(context.Background(), test.certificateRequest) + if err != nil && !test.expectedErr { + t.Errorf("expected to not get an error, but got: %v", err) + } + if err == nil && test.expectedErr { + t.Errorf("expected to get an error but did not get one") + } + test.builder.CheckAndFinish(resp, err) +} diff --git a/pkg/internal/BUILD.bazel b/pkg/internal/BUILD.bazel index 5c5a24968..3551c7e47 100644 --- a/pkg/internal/BUILD.bazel +++ b/pkg/internal/BUILD.bazel @@ -5,7 +5,11 @@ go_library( srcs = ["internal.go"], importpath = "github.com/jetstack/cert-manager/pkg/internal", visibility = ["//visibility:public"], - deps = ["//vendor/github.com/hashicorp/vault/api:go_default_library"], + deps = [ + "//pkg/apis/certmanager/v1alpha1:go_default_library", + "//vendor/github.com/hashicorp/vault/api:go_default_library", + "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", + ], ) filegroup( diff --git a/pkg/internal/internal.go b/pkg/internal/internal.go index 20801b19f..d647bf2a5 100644 --- a/pkg/internal/internal.go +++ b/pkg/internal/internal.go @@ -17,9 +17,20 @@ limitations under the License. package internal import ( + "time" + vault "github.com/hashicorp/vault/api" + corelisters "k8s.io/client-go/listers/core/v1" + + "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" ) +type VaultFactory func(string, corelisters.SecretLister, v1alpha1.GenericIssuer) (Vault, error) + +type Vault interface { + Sign(csrPEM []byte, duration time.Duration) (certPEM []byte, caPEM []byte, err error) +} + type VaultClient interface { NewRequest(method, requestPath string) *vault.Request RawRequest(r *vault.Request) (*vault.Response, error) diff --git a/pkg/internal/vault/BUILD.bazel b/pkg/internal/vault/BUILD.bazel index 5f3a174f5..c5f574be8 100644 --- a/pkg/internal/vault/BUILD.bazel +++ b/pkg/internal/vault/BUILD.bazel @@ -8,7 +8,9 @@ go_library( deps = [ "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/internal:go_default_library", + "//pkg/util/pki:go_default_library", "//vendor/github.com/hashicorp/vault/api:go_default_library", + "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], ) @@ -22,8 +24,11 @@ go_test( "//pkg/controller/test/fake:go_default_library", "//pkg/internal:go_default_library", "//pkg/internal/vault/fake:go_default_library", + "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", "//vendor/github.com/hashicorp/vault/api:go_default_library", + "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", + "//vendor/github.com/hashicorp/vault/helper/jsonutil:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", ], ) diff --git a/pkg/internal/vault/fake/BUILD.bazel b/pkg/internal/vault/fake/BUILD.bazel index 1b202967e..785a00426 100644 --- a/pkg/internal/vault/fake/BUILD.bazel +++ b/pkg/internal/vault/fake/BUILD.bazel @@ -2,12 +2,18 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["client.go"], + srcs = [ + "client.go", + "vault.go", + ], importpath = "github.com/jetstack/cert-manager/pkg/internal/vault/fake", visibility = ["//pkg:__subpackages__"], deps = [ + "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/internal:go_default_library", + "//pkg/internal/vault:go_default_library", "//vendor/github.com/hashicorp/vault/api:go_default_library", + "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], ) diff --git a/pkg/internal/vault/fake/vault.go b/pkg/internal/vault/fake/vault.go new file mode 100644 index 000000000..2a94e67f4 --- /dev/null +++ b/pkg/internal/vault/fake/vault.go @@ -0,0 +1,69 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +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 fake + +import ( + "time" + + corelisters "k8s.io/client-go/listers/core/v1" + + "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + "github.com/jetstack/cert-manager/pkg/internal" + "github.com/jetstack/cert-manager/pkg/internal/vault" +) + +var _ internal.Vault = &Vault{} + +type Vault struct { + NewFn internal.VaultFactory + SignFn func([]byte, time.Duration) ([]byte, []byte, error) +} + +func NewFakeVault() *Vault { + return &Vault{ + NewFn: vault.New, + SignFn: func([]byte, time.Duration) ([]byte, []byte, error) { + return nil, nil, nil + }, + } +} +func (v *Vault) Sign(csrPEM []byte, duration time.Duration) ([]byte, []byte, error) { + return v.SignFn(csrPEM, duration) +} + +func (v *Vault) WithSign(certPEM, caPEM []byte, err error) *Vault { + v.SignFn = func([]byte, time.Duration) ([]byte, []byte, error) { + return certPEM, caPEM, err + } + return v +} + +func (v *Vault) WithNoOpNew() *Vault { + v.NewFn = func(string, corelisters.SecretLister, v1alpha1.GenericIssuer) (internal.Vault, error) { + return v, nil + } + return v +} + +func (v *Vault) New(ns string, sl corelisters.SecretLister, iss v1alpha1.GenericIssuer) (internal.Vault, error) { + _, err := v.NewFn(ns, sl, iss) + if err != nil { + return nil, err + } + + return v, nil +} diff --git a/pkg/internal/vault/vault.go b/pkg/internal/vault/vault.go index 564e3f6f1..8e65c8148 100644 --- a/pkg/internal/vault/vault.go +++ b/pkg/internal/vault/vault.go @@ -34,6 +34,8 @@ import ( "github.com/jetstack/cert-manager/pkg/util/pki" ) +var _ internal.Vault = &Vault{} + type Vault struct { secretsLister corelisters.SecretLister issuer v1alpha1.GenericIssuer @@ -43,7 +45,7 @@ type Vault struct { } func New(namespace string, secretsLister corelisters.SecretLister, - issuer v1alpha1.GenericIssuer) (*Vault, error) { + issuer v1alpha1.GenericIssuer) (internal.Vault, error) { v := &Vault{ secretsLister: secretsLister, namespace: namespace, @@ -129,7 +131,7 @@ func (v *Vault) setToken(client internal.VaultClient) error { if tokenRef.Name != "" { token, err := v.tokenRef(tokenRef.Name, v.namespace, tokenRef.Key) if err != nil { - return fmt.Errorf("error reading Vault token from secret %s/%s: %s", v.namespace, tokenRef.Name, err.Error()) + return err } client.SetToken(token) @@ -140,7 +142,7 @@ func (v *Vault) setToken(client internal.VaultClient) error { if appRole.RoleId != "" { token, err := v.requestTokenWithAppRoleRef(client, &appRole) if err != nil { - return fmt.Errorf("error reading Vault token from AppRole: %s", err.Error()) + return err } client.SetToken(token) @@ -215,8 +217,7 @@ func (v *Vault) appRoleRef(appRole *v1alpha1.VaultAppRole) (roleId, secretId str func (v *Vault) requestTokenWithAppRoleRef(client internal.VaultClient, appRole *v1alpha1.VaultAppRole) (string, error) { roleId, secretId, err := v.appRoleRef(appRole) if err != nil { - return "", fmt.Errorf("error reading Vault AppRole from secret: %s/%s: %s", - v.namespace, appRole.SecretRef.Name, err.Error()) + return "", err } parameters := map[string]string{ diff --git a/pkg/internal/vault/vault_test.go b/pkg/internal/vault/vault_test.go index 68c544766..ed80767e5 100644 --- a/pkg/internal/vault/vault_test.go +++ b/pkg/internal/vault/vault_test.go @@ -235,8 +235,7 @@ func TestSetToken(t *testing.T) { ), fakeClient: vaultfake.NewFakeClient(), expectedToken: "", - expectedErr: errors.New( - "error reading Vault token from secret test-namespace/secret-ref-name: secret does not exists"), + expectedErr: errors.New("secret does not exists"), }, "if token secret ref set, return client using token stored": { @@ -284,7 +283,7 @@ func TestSetToken(t *testing.T) { fakeClient: vaultfake.NewFakeClient(), expectedToken: "", expectedErr: errors.New( - "error reading Vault token from AppRole: error reading Vault AppRole from secret: test-namespace/secret-ref-name: secret not found"), + "error reading Vault AppRole from secret: test-namespace/secret-ref-name: secret not found"), }, "if app role secret ref set, return client using token stored": { diff --git a/pkg/issuer/vault/BUILD.bazel b/pkg/issuer/vault/BUILD.bazel index e1fe3309e..021b18088 100644 --- a/pkg/issuer/vault/BUILD.bazel +++ b/pkg/issuer/vault/BUILD.bazel @@ -13,12 +13,11 @@ go_library( "//pkg/api/util:go_default_library", "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/controller:go_default_library", + "//pkg/internal/vault:go_default_library", "//pkg/issuer:go_default_library", "//pkg/util/errors:go_default_library", "//pkg/util/kube:go_default_library", "//pkg/util/pki:go_default_library", - "//vendor/github.com/hashicorp/vault/api:go_default_library", - "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library",