code review modifications

This commit is contained in:
Vincent Desjardins 2018-06-08 21:30:03 +00:00 committed by test@test.com
parent 2995cc90a3
commit ca3b909cb7
7 changed files with 29 additions and 33 deletions

View File

@ -109,11 +109,12 @@ type VaultAuth struct {
TokenSecretRef SecretKeySelector `json:"tokenSecretRef,omitempty"`
// This Secret contains a AppRole and Secret
AppRole VaultAppRole `json:"appRole,omitempty"`
// Where the authentication path is mounted in Vault.
AuthPath string `json:"authPath,omitempty"`
}
type VaultAppRole struct {
// Where the authentication path is mounted in Vault.
Path string `json:"path,omitempty"`
RoleId string `json:"roleId"`
SecretRef SecretKeySelector `json:"secretRef"`
}

View File

@ -110,7 +110,7 @@ func (v *Vault) initVaultClient() (*vault.Client, error) {
if appRole.RoleId != "" {
token, err := v.requestTokenWithAppRoleRef(client, &appRole)
if err != nil {
return nil, fmt.Errorf("error reading Vault token from secret %s/%s: %s", v.issuerResourcesNamespace, appRole.SecretRef.Name, err.Error())
return nil, fmt.Errorf("error reading Vault token from AppRole: %s", err.Error())
}
client.SetToken(token)
@ -131,12 +131,12 @@ func (v *Vault) requestTokenWithAppRoleRef(client *vault.Client, appRole *v1alph
"secret_id": secretId,
}
authPath := v.issuer.GetSpec().Vault.Auth.AuthPath
authPath := appRole.Path
if authPath == "" {
authPath = "approle"
}
url := path.Join("/v1/auth", authPath, "login")
url := path.Join("/v1", "auth", authPath, "login")
request := client.NewRequest("POST", url)
@ -147,7 +147,7 @@ func (v *Vault) requestTokenWithAppRoleRef(client *vault.Client, appRole *v1alph
resp, err := client.RawRequest(request)
if err != nil {
return "", fmt.Errorf("error calling Vault server: %s", err.Error())
return "", fmt.Errorf("error logging in to Vault server: %s", err.Error())
}
defer resp.Body.Close()
@ -193,7 +193,7 @@ func (v *Vault) requestVaultCert(commonName string, altNames []string, csr []byt
resp, err := client.RawRequest(request)
if err != nil {
return nil, fmt.Errorf("error calling Vault server: %s", err.Error())
return nil, fmt.Errorf("error logging in to Vault server: %s", err.Error())
}
defer resp.Body.Close()

View File

@ -21,7 +21,6 @@ const (
messageServerAndPathRequired = "Vault server and path are required fields"
messsageAuthFieldsRequired = "Vault tokenSecretRef or appRole is required"
messageAuthFieldRequired = "Vault tokenSecretRef and appRole cannot be set on the same issuer"
messageAuthPathInvalid = "Vault authPath cannot be used with a tokenSecretRef"
)
func (v *Vault) Setup(ctx context.Context) error {
@ -48,19 +47,13 @@ func (v *Vault) Setup(ctx context.Context) error {
if v.issuer.GetSpec().Vault.Auth.TokenSecretRef.Name != "" &&
(v.issuer.GetSpec().Vault.Auth.AppRole.RoleId != "" ||
v.issuer.GetSpec().Vault.Auth.AppRole.SecretRef.Name != "") {
v.issuer.GetSpec().Vault.Auth.AppRole.SecretRef.Name != "" ||
v.issuer.GetSpec().Vault.Auth.AppRole.Path != "") {
glog.Infof("%s: %s", v.issuer.GetObjectMeta().Name, messageAuthFieldRequired)
v.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorVault, messageAuthFieldRequired)
return fmt.Errorf(messageAuthFieldRequired)
}
if v.issuer.GetSpec().Vault.Auth.TokenSecretRef.Name != "" &&
v.issuer.GetSpec().Vault.Auth.AuthPath != "" {
glog.Infof("%s: %s", v.issuer.GetObjectMeta().Name, messageAuthPathInvalid)
v.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorVault, messageAuthPathInvalid)
return fmt.Errorf(messageAuthPathInvalid)
}
client, err := v.initVaultClient()
if err != nil {
s := messageVaultClientInitFailed + err.Error()

View File

@ -1,7 +1,7 @@
package certificate
import (
"fmt"
"path"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
@ -23,7 +23,8 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole)", func() {
certificateName := "test-vault-certificate"
certificateSecretName := "test-vault-certificate"
vaultSecretAppRoleName := "vault-role"
vaultPath := fmt.Sprintf("%s/sign/%s", intermediateMount, role)
vaultPath := path.Join(intermediateMount, "sign", role)
authPath := "approle"
var vaultInit *vault.VaultInitializer
var roleId string
var secretId string
@ -33,7 +34,7 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole)", func() {
podList, err := f.KubeClientSet.CoreV1().Pods("vault").List(metav1.ListOptions{})
Expect(err).NotTo(HaveOccurred())
vaultPodName := podList.Items[0].Name
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, "")
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, authPath)
Expect(err).NotTo(HaveOccurred())
err = vaultInit.Setup()
Expect(err).NotTo(HaveOccurred())
@ -54,7 +55,7 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole)", func() {
vaultURL := "http://vault.vault:8200"
It("should generate a new valid certificate", func() {
By("Creating an Issuer")
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, ""))
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, authPath))
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")
@ -79,13 +80,13 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole with a custom
rootMount := "root-ca"
intermediateMount := "intermediate-ca"
roleMount := "custom/path"
authPath := "custom/path"
role := "kubernetes-vault"
issuerName := "test-vault-issuer"
certificateName := "test-vault-certificate"
certificateSecretName := "test-vault-certificate"
vaultSecretAppRoleName := "vault-role"
vaultPath := fmt.Sprintf("%s/sign/%s", intermediateMount, role)
vaultPath := path.Join(intermediateMount, "sign", role)
var vaultInit *vault.VaultInitializer
var roleId string
var secretId string
@ -95,7 +96,7 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole with a custom
podList, err := f.KubeClientSet.CoreV1().Pods("vault").List(metav1.ListOptions{})
Expect(err).NotTo(HaveOccurred())
vaultPodName := podList.Items[0].Name
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, roleMount)
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, authPath)
Expect(err).NotTo(HaveOccurred())
err = vaultInit.Setup()
Expect(err).NotTo(HaveOccurred())
@ -116,7 +117,7 @@ var _ = framework.CertManagerDescribe("Vault Certificate (AppRole with a custom
vaultURL := "http://vault.vault:8200"
It("should generate a new valid certificate", func() {
By("Creating an Issuer")
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, roleMount))
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, authPath))
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")

View File

@ -1,7 +1,7 @@
package issuer
import (
"fmt"
"path"
"time"
. "github.com/onsi/ginkgo"
@ -23,7 +23,8 @@ var _ = framework.CertManagerDescribe("Vault Issuer", func() {
role := "kubernetes-vault"
vaultSecretAppRoleName := "vault-role"
vaultSecretTokenName := "vault-token"
vaultPath := fmt.Sprintf("%s/sign/%s", intermediateMount, role)
vaultPath := path.Join(intermediateMount, "sign", role)
authPath := "approle"
var roleId, secretId string
var vaultInit *vault.VaultInitializer
@ -32,7 +33,7 @@ var _ = framework.CertManagerDescribe("Vault Issuer", func() {
podList, err := f.KubeClientSet.CoreV1().Pods("vault").List(metav1.ListOptions{})
Expect(err).NotTo(HaveOccurred())
vaultPodName := podList.Items[0].Name
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, "")
vaultInit, err = vault.NewVaultInitializer(vaultPodName, rootMount, intermediateMount, role, authPath)
Expect(err).NotTo(HaveOccurred())
err = vaultInit.Setup()
Expect(err).NotTo(HaveOccurred())
@ -56,7 +57,7 @@ var _ = framework.CertManagerDescribe("Vault Issuer", func() {
Expect(err).NotTo(HaveOccurred())
By("Creating an Issuer")
_, err = f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, ""))
_, err = f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, authPath))
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")
@ -71,7 +72,7 @@ var _ = framework.CertManagerDescribe("Vault Issuer", func() {
It("should fail to init with missing Vault AppRole", func() {
By("Creating an Issuer")
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, ""))
_, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(util.NewCertManagerVaultIssuerAppRole(issuerName, vaultURL, vaultPath, roleId, vaultSecretAppRoleName, authPath))
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")

View File

@ -424,8 +424,8 @@ func NewCertManagerVaultIssuerAppRole(name, vaultURL, vaultPath, roleId, vaultSe
Server: vaultURL,
Path: vaultPath,
Auth: v1alpha1.VaultAuth{
AuthPath: authPath,
AppRole: v1alpha1.VaultAppRole{
Path: authPath,
RoleId: roleId,
SecretRef: v1alpha1.SecretKeySelector{
Key: "secretkey",

View File

@ -65,7 +65,7 @@ func NewVaultInitializer(container, rootMount, intermediateMount, role, authPath
client.SetToken(vaultToken)
if authPath == "" {
authPath = "approle"
return nil, fmt.Errorf("Error authPath must be set")
}
return &VaultInitializer{
@ -151,7 +151,7 @@ func (v *VaultInitializer) CreateAppRole() (string, string, error) {
"policies": v.role,
}
baseUrl := path.Join("/v1/auth", v.authPath, "role", v.role)
baseUrl := path.Join("/v1", "auth", v.authPath, "role", v.role)
_, err = v.callVault("POST", baseUrl, "", params)
if err != nil {
return "", "", fmt.Errorf("Error creating approle: %s", err.Error())
@ -175,7 +175,7 @@ func (v *VaultInitializer) CreateAppRole() (string, string, error) {
}
func (v *VaultInitializer) CleanAppRole() error {
url := path.Join("/v1/auth", v.authPath, "role", v.role)
url := path.Join("/v1", "auth", v.authPath, "role", v.role)
_, err := v.callVault("DELETE", url, "", map[string]string{})
if err != nil {
return fmt.Errorf("Error deleting AppRole: %s", err.Error())