Post review updates #2

api/v1alpha1/vaultauth_types.go

@@ -76,7 +76,7 @@ type VaultAuth struct {
 // If no value is specified an error is returned.
 func (v *VaultAuth) GetConnectionNamespacedName() (types.NamespacedName, error) {
 	connRef := v.Spec.VaultConnectionRef
-	if len(connRef) == 0 {
+	if connRef == "" {
 		return types.NamespacedName{}, fmt.Errorf(
 			"vaultConnectionRef is empty",
 		)

controllers/common.go

@@ -60,7 +60,7 @@ func getVaultConfig(ctx context.Context, c client.Client, obj client.Object) (*v
 
 	var err error
 	var va *secretsv1alpha1.VaultAuth
-	if len(authRef) == 0 {
+	if authRef == "" {
 		// if no authRef configured we try and grab the 'default' from the
 		// Operator's namespace.
 		va, err = getVaultAuth(ctx, c, types.NamespacedName{
@@ -97,7 +97,7 @@ func getVaultConfig(ctx context.Context, c client.Client, obj client.Object) (*v
 		return nil, err
 	}
 
-	config.SetAuthLogin(authLogin)
+	config.AuthLogin = authLogin
 
 	return config, nil
 }

controllers/vaultauth_controller.go

@@ -30,6 +30,7 @@ type VaultAuthReconciler struct {
 //+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultauths/finalizers,verbs=update
 //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch
 //+kubebuilder:rbac:groups="",resources=serviceaccounts/token,verbs=get;list;create;watch
+//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.

controllers/vaultconnection_controller.go

@@ -15,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/hashicorp/go-multierror"
+
 	secretsv1alpha1 "github.com/hashicorp/vault-secrets-operator/api/v1alpha1"
 	"github.com/hashicorp/vault-secrets-operator/internal/vault"
 )

integrationtest/vaultpkisecret/terraform/variables.tf

@@ -17,7 +17,9 @@ variable "k8s_config_path" {
   default = "~/.kube/config"
 }
 
-variable "k8s_host" {}
+variable "k8s_host" {
+  default = "https://kubernetes.default.svc"
+}
 
 variable "vault_pki_mount_path" {
   default = "pki"

integrationtest/vaultpkisecret_integration_test.go

@@ -34,8 +34,6 @@ func TestVaultPKISecret(t *testing.T) {
 		// Set the path to the Terraform code that will be tested.
 		TerraformDir: "vaultpkisecret/terraform",
 		Vars: map[string]interface{}{
-			// TODO: get this dynamically
-			"k8s_host":             "https://10.96.0.1",
 			"k8s_test_namespace":   testK8sNamespace,
 			"k8s_config_context":   "kind-" + clusterName,
 			"vault_pki_mount_path": testPKIMountPath,
@@ -67,8 +65,9 @@ func TestVaultPKISecret(t *testing.T) {
 		},
 	}
 
-	defer crdClient.Delete(context.Background(), testVaultConnection)
-	err := crdClient.Create(context.Background(), testVaultConnection)
+	ctx := context.Background()
+	defer crdClient.Delete(ctx, testVaultConnection)
+	err := crdClient.Create(ctx, testVaultConnection)
 	require.NoError(t, err)
 
 	// Create a VaultAuth CR
@@ -90,8 +89,8 @@ func TestVaultPKISecret(t *testing.T) {
 		},
 	}
 
-	defer crdClient.Delete(context.Background(), testVaultAuth)
-	err = crdClient.Create(context.Background(), testVaultAuth)
+	defer crdClient.Delete(ctx, testVaultAuth)
+	err = crdClient.Create(ctx, testVaultAuth)
 	require.NoError(t, err)
 
 	// Create a VaultPKI CR to trigger the sync
@@ -115,8 +114,8 @@ func TestVaultPKISecret(t *testing.T) {
 		},
 	}
 
-	defer crdClient.Delete(context.Background(), testVaultPKI)
-	err = crdClient.Create(context.Background(), testVaultPKI)
+	defer crdClient.Delete(ctx, testVaultPKI)
+	err = crdClient.Create(ctx, testVaultPKI)
 	require.NoError(t, err)
 
 	// Wait for the operator to sync Vault PKI --> k8s Secret, and return the

integrationtest/vaultstaticsecret/terraform/variables.tf

@@ -17,7 +17,9 @@ variable "k8s_config_path" {
   default = "~/.kube/config"
 }
 
-variable "k8s_host" {}
+variable "k8s_host" {
+  default = "https://kubernetes.default.svc"
+}
 
 variable "vault_kv_mount_path" {
   default = "kv"

integrationtest/vaultstaticsecret_integration_test.go

@@ -56,8 +56,8 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 
 	crdClient := getCRDClient(t)
 	var created []client.Object
+	ctx := context.Background()
 	t.Cleanup(func() {
-		ctx := context.Background()
 		for _, c := range created {
 			// test that the custom resources can be deleted before tf destroy
 			// removes the k8s namespace
@@ -73,10 +73,10 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 	// Set the secrets in vault to be synced to kubernetes
 	vClient := getVaultClient(t, testVaultNamespace)
 	putSecretV1 := map[string]interface{}{"password": "grapejuice", "username": "breakfast", "time": "now"}
-	err := vClient.KVv1(testKvMountPath).Put(context.Background(), "secret", putSecretV1)
+	err := vClient.KVv1(testKvMountPath).Put(ctx, "secret", putSecretV1)
 	require.NoError(t, err)
 	putSecretV2 := map[string]interface{}{"password": "applejuice", "username": "lunch", "time": "later"}
-	_, err = vClient.KVv2(testKvv2MountPath).Put(context.Background(), "secret", putSecretV2)
+	_, err = vClient.KVv2(testKvv2MountPath).Put(ctx, "secret", putSecretV2)
 	require.NoError(t, err)
 
 	// Create a VaultConnection CR
@@ -141,13 +141,11 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 	}
 
 	for _, c := range conns {
-		ctx := context.Background()
 		require.Nil(t, crdClient.Create(ctx, c))
 		created = append(created, c)
 	}
 
 	for _, a := range auths {
-		ctx := context.Background()
 		require.Nil(t, crdClient.Create(ctx, a))
 		created = append(created, a)
 	}
@@ -189,7 +187,6 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 	}
 
 	for _, a := range secrets {
-		ctx := context.Background()
 		require.Nil(t, crdClient.Create(ctx, a))
 		created = append(created, a)
 	}
@@ -204,10 +201,10 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 	// Change the secrets in Vault, wait for the VaultStaticSecret's to refresh,
 	// and check the result
 	updatedSecretV1 := map[string]interface{}{"password": "orangejuice", "time": "morning"}
-	err = vClient.KVv1(testKvMountPath).Put(context.Background(), "secret", updatedSecretV1)
+	err = vClient.KVv1(testKvMountPath).Put(ctx, "secret", updatedSecretV1)
 	require.NoError(t, err)
 	updatedSecretV2 := map[string]interface{}{"password": "cranberryjuice", "time": "evening"}
-	_, err = vClient.KVv2(testKvv2MountPath).Put(context.Background(), "secret", updatedSecretV2)
+	_, err = vClient.KVv2(testKvv2MountPath).Put(ctx, "secret", updatedSecretV2)
 	require.NoError(t, err)
 
 	expected = []map[string]interface{}{updatedSecretV1, updatedSecretV2}

internal/vault/auth.go

@@ -27,8 +27,8 @@ func NewAuthLogin(c crclient.Client, va *v1alpha1.VaultAuth, k8sNamespace string
 	switch method {
 	case "kubernetes":
 		a := &KubernetesAuth{
-			client: c,
-			va:     va,
+			client:    c,
+			vaultAuth: va,
 		}
 		a.SetK8SNamespace(k8sNamespace)
 		if err := a.Validate(); err != nil {

internal/vault/auth_kubernetes.go

@@ -19,10 +19,11 @@ import (
 
 // KubernetesAuth implements the AuthLogin interface to log in to Vault.
 type KubernetesAuth struct {
-	client ctrlclient.Client
-	va     *v1alpha1.VaultAuth
-	vc     *v1alpha1.VaultConnection
-	sans   string
+	client    ctrlclient.Client
+	vaultAuth *v1alpha1.VaultAuth
+	vaultConn *v1alpha1.VaultConnection
+	// serviceAccountNamespace to use when creating k8s tokens.
+	serviceAccountNamespace string
 }
 
 // Login to Vault with the related VaultAuth and VaultConnection.
@@ -31,7 +32,7 @@ func (l *KubernetesAuth) Login(ctx context.Context, client *api.Client) (*api.Se
 	logger := log.FromContext(ctx)
 	n := types.NamespacedName{
 		Namespace: l.GetK8SNamespace(),
-		Name:      l.va.Spec.Kubernetes.ServiceAccount,
+		Name:      l.vaultAuth.Spec.Kubernetes.ServiceAccount,
 	}
 
 	sa := &v1.ServiceAccount{}
@@ -50,7 +51,7 @@ func (l *KubernetesAuth) Login(ctx context.Context, client *api.Client) (*api.Se
 		ctx,
 		l.LoginPath(),
 		map[string]interface{}{
-			"role": l.va.Spec.Kubernetes.Role,
+			"role": l.vaultAuth.Spec.Kubernetes.Role,
 			"jwt":  tr.Status.Token,
 		})
 	if err != nil {
@@ -65,11 +66,11 @@ func (l *KubernetesAuth) Login(ctx context.Context, client *api.Client) (*api.Se
 func (l *KubernetesAuth) getSATokenRequest() (*authv1.TokenRequest, error) {
 	return &authv1.TokenRequest{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: l.va.Spec.Kubernetes.TokenGenerateName,
+			GenerateName: l.vaultAuth.Spec.Kubernetes.TokenGenerateName,
 		},
 		Spec: authv1.TokenRequestSpec{
-			ExpirationSeconds: pointer.Int64(l.va.Spec.Kubernetes.TokenExpirationSeconds),
-			Audiences:         l.va.Spec.Kubernetes.TokenAudiences,
+			ExpirationSeconds: pointer.Int64(l.vaultAuth.Spec.Kubernetes.TokenExpirationSeconds),
+			Audiences:         l.vaultAuth.Spec.Kubernetes.TokenAudiences,
 		},
 		Status: authv1.TokenRequestStatus{},
 	}, nil
@@ -95,31 +96,31 @@ func (l *KubernetesAuth) requestSAToken(ctx context.Context, sa *v1.ServiceAccou
 
 // MountPath to the Vault authentication backend.
 func (l *KubernetesAuth) MountPath() string {
-	return l.va.Spec.Mount
+	return l.vaultAuth.Spec.Mount
 }
 
-// LoginPath for authenticating to Vault
+// LoginPath for authenticating to Vault.
 func (l *KubernetesAuth) LoginPath() string {
 	return fmt.Sprintf("auth/%s/login", l.MountPath())
 }
 
 // SetK8SNamespace to use for the login request.
 func (l *KubernetesAuth) SetK8SNamespace(ns string) {
-	l.sans = ns
+	l.serviceAccountNamespace = ns
 }
 
-// GetK8SNamespace for the login request
+// GetK8SNamespace for the login request.
 func (l *KubernetesAuth) GetK8SNamespace() string {
-	return l.sans
+	return l.serviceAccountNamespace
 }
 
 // Validate that the AuthLogin was properly initialized.
 func (l *KubernetesAuth) Validate() error {
 	var err error
-	if l.va == nil {
+	if l.vaultAuth == nil {
 		err = multierror.Append(err, fmt.Errorf("VaultAuth is not set"))
 	} else {
-		if l.va.Spec.Kubernetes == nil {
+		if l.vaultAuth.Spec.Kubernetes == nil {
 			err = multierror.Append(err, fmt.Errorf("VaultAuth.Spec.Kubernetes is not set"))
 		}
 	}

internal/vault/auth_kubernetes_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	v12 "k8s.io/api/authentication/v1"
+	authv1 "k8s.io/api/authentication/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 
@@ -27,7 +27,7 @@ func TestKubernetesAuth_SetK8SNamespace(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			l := &KubernetesAuth{}
 			l.SetK8SNamespace(tt.want)
-			assert.Equalf(t, tt.want, l.sans, "SetK8SNamespace(%q)", tt.want)
+			assert.Equalf(t, tt.want, l.serviceAccountNamespace, "SetK8SNamespace(%q)", tt.want)
 		})
 	}
 }
@@ -56,7 +56,7 @@ func TestKubernetesAuth_getSATokenRequest(t *testing.T) {
 		name    string
 		va      *v1alpha1.VaultAuth
 		sans    string
-		want    *v12.TokenRequest
+		want    *authv1.TokenRequest
 		wantErr assert.ErrorAssertionFunc
 	}{
 		{
@@ -76,15 +76,15 @@ func TestKubernetesAuth_getSATokenRequest(t *testing.T) {
 					},
 				},
 			},
-			want: &v12.TokenRequest{
+			want: &authv1.TokenRequest{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "baz",
 				},
-				Spec: v12.TokenRequestSpec{
+				Spec: authv1.TokenRequestSpec{
 					ExpirationSeconds: pointer.Int64(1200),
 					Audiences:         []string{"buz", "qux"},
 				},
-				Status: v12.TokenRequestStatus{},
+				Status: authv1.TokenRequestStatus{},
 			},
 			wantErr: func(t assert.TestingT, err error, i ...interface{}) bool {
 				valid := err == nil
@@ -98,8 +98,8 @@ func TestKubernetesAuth_getSATokenRequest(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			l := &KubernetesAuth{
-				va:   tt.va,
-				sans: tt.sans,
+				vaultAuth:               tt.va,
+				serviceAccountNamespace: tt.sans,
 			}
 			got, err := l.getSATokenRequest()
 			if !tt.wantErr(t, err, fmt.Sprintf("getSATokenRequest()")) {

internal/vault/auth_test.go

@@ -36,15 +36,15 @@ func TestNewAuthLogin(t *testing.T) {
 			k8sNamespace: "baz",
 			want: &KubernetesAuth{
 				client: c,
-				va: &secretsv1alpha1.VaultAuth{
+				vaultAuth: &secretsv1alpha1.VaultAuth{
 					Spec: secretsv1alpha1.VaultAuthSpec{
 						VaultConnectionRef: "foo",
 						Namespace:          "baz",
 						Method:             "kubernetes",
 						Kubernetes:         &secretsv1alpha1.VaultAuthConfigKubernetes{},
 					},
 				},
-				sans: "baz",
+				serviceAccountNamespace: "baz",
 			},
 			wantErr: func(t assert.TestingT, err error, _ ...interface{}) bool {
 				valid := err == nil

internal/vault/vault.go

@@ -87,10 +87,6 @@ type VaultClientConfig struct {
 	AuthLogin AuthLogin
 }
 
-func (c *VaultClientConfig) SetAuthLogin(a AuthLogin) {
-	c.AuthLogin = a
-}
-
 // MakeVaultClient creates a Vault API client from a VaultClientConfig
 func MakeVaultClient(ctx context.Context, vaultConfig *VaultClientConfig, k8sClient client.Client) (*api.Client, error) {
 	l := log.FromContext(ctx)