:sparkles: allow vault roleId to come from k8s Secret (continued) (#2284)

* allow vault roleId to come from k8s Secret

Signed-off-by: intrand <intrand@users.noreply.github.com>

* mark RoleID as optional in kubebuilder

Co-authored-by: Gustavo Fernandes de Carvalho <gusfcarvalho@gmail.com>
Signed-off-by: intrand <intrand@users.noreply.github.com>

* mark RoleRef as optional in kubebuilder

Co-authored-by: Gustavo Fernandes de Carvalho <gusfcarvalho@gmail.com>
Signed-off-by: intrand <intrand@users.noreply.github.com>

* validate RoleRef through webhook

Signed-off-by: intrand <intrand@users.noreply.github.com>

* chore: make fmt/reviewable vault roleId addition

Signed-off-by: Brian Richardson <brianthemathguy@gmail.com>

---------

Signed-off-by: intrand <intrand@users.noreply.github.com>
Signed-off-by: Brian Richardson <brianthemathguy@gmail.com>
Co-authored-by: intrand <intrand@users.noreply.github.com>
Co-authored-by: Gustavo Fernandes de Carvalho <gusfcarvalho@gmail.com>

apis/externalsecrets/v1beta1/secretstore_vault_types.go

@@ -129,7 +129,15 @@ type VaultAppRole struct {
 
 	// RoleID configured in the App Role authentication backend when setting
 	// up the authentication backend in Vault.
-	RoleID string `json:"roleId"`
+	//+optional
+	RoleID string `json:"roleId,omitempty"`
+
+	// Reference to a key in a Secret that contains the App Role ID used
+	// to authenticate with Vault.
+	// The `key` field must be specified and denotes which entry within the Secret
+	// resource is used as the app role id.
+	//+optional
+	RoleRef *esmeta.SecretKeySelector `json:"roleRef,omitempty"`
 
 	// Reference to a key in a Secret that contains the App Role secret used
 	// to authenticate with Vault.

apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -2005,6 +2005,11 @@ func (in *TokenAuth) DeepCopy() *TokenAuth {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VaultAppRole) DeepCopyInto(out *VaultAppRole) {
 	*out = *in
+	if in.RoleRef != nil {
+		in, out := &in.RoleRef, &out.RoleRef
+		*out = new(metav1.SecretKeySelector)
+		(*in).DeepCopyInto(*out)
+	}
 	in.SecretRef.DeepCopyInto(&out.SecretRef)
 }
 

config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -2925,6 +2925,30 @@ spec:
                                   backend when setting up the authentication backend
                                   in Vault.
                                 type: string
+                              roleRef:
+                                description: Reference to a key in a Secret that contains
+                                  the App Role ID used to authenticate with Vault.
+                                  The `key` field must be specified and denotes which
+                                  entry within the Secret resource is used as the
+                                  app role id.
+                                properties:
+                                  key:
+                                    description: The key of the entry in the Secret
+                                      resource's `data` field to be used. Some instances
+                                      of this field may be defaulted, in others it
+                                      may be required.
+                                    type: string
+                                  name:
+                                    description: The name of the Secret resource being
+                                      referred to.
+                                    type: string
+                                  namespace:
+                                    description: Namespace of the resource being referred
+                                      to. Ignored if referent is not cluster-scoped.
+                                      cluster-scoped defaults to the namespace of
+                                      the referent.
+                                    type: string
+                                type: object
                               secretRef:
                                 description: Reference to a key in a Secret that contains
                                   the App Role secret used to authenticate with Vault.
@@ -2951,7 +2975,6 @@ spec:
                                 type: object
                             required:
                             - path
-                            - roleId
                             - secretRef
                             type: object
                           cert:

config/crds/bases/external-secrets.io_secretstores.yaml

@@ -2925,6 +2925,30 @@ spec:
                                   backend when setting up the authentication backend
                                   in Vault.
                                 type: string
+                              roleRef:
+                                description: Reference to a key in a Secret that contains
+                                  the App Role ID used to authenticate with Vault.
+                                  The `key` field must be specified and denotes which
+                                  entry within the Secret resource is used as the
+                                  app role id.
+                                properties:
+                                  key:
+                                    description: The key of the entry in the Secret
+                                      resource's `data` field to be used. Some instances
+                                      of this field may be defaulted, in others it
+                                      may be required.
+                                    type: string
+                                  name:
+                                    description: The name of the Secret resource being
+                                      referred to.
+                                    type: string
+                                  namespace:
+                                    description: Namespace of the resource being referred
+                                      to. Ignored if referent is not cluster-scoped.
+                                      cluster-scoped defaults to the namespace of
+                                      the referent.
+                                    type: string
+                                type: object
                               secretRef:
                                 description: Reference to a key in a Secret that contains
                                   the App Role secret used to authenticate with Vault.
@@ -2951,7 +2975,6 @@ spec:
                                 type: object
                             required:
                             - path
-                            - roleId
                             - secretRef
                             type: object
                           cert:

config/crds/bases/generators.external-secrets.io_vaultdynamicsecrets.yaml

@@ -67,6 +67,27 @@ spec:
                               backend when setting up the authentication backend in
                               Vault.
                             type: string
+                          roleRef:
+                            description: Reference to a key in a Secret that contains
+                              the App Role ID used to authenticate with Vault. The
+                              `key` field must be specified and denotes which entry
+                              within the Secret resource is used as the app role id.
+                            properties:
+                              key:
+                                description: The key of the entry in the Secret resource's
+                                  `data` field to be used. Some instances of this
+                                  field may be defaulted, in others it may be required.
+                                type: string
+                              name:
+                                description: The name of the Secret resource being
+                                  referred to.
+                                type: string
+                              namespace:
+                                description: Namespace of the resource being referred
+                                  to. Ignored if referent is not cluster-scoped. cluster-scoped
+                                  defaults to the namespace of the referent.
+                                type: string
+                            type: object
                           secretRef:
                             description: Reference to a key in a Secret that contains
                               the App Role secret used to authenticate with Vault.
@@ -91,7 +112,6 @@ spec:
                             type: object
                         required:
                         - path
-                        - roleId
                         - secretRef
                         type: object
                       cert:

deploy/crds/bundle.yaml

@@ -2608,6 +2608,19 @@ spec:
                                 roleId:
                                   description: RoleID configured in the App Role authentication backend when setting up the authentication backend in Vault.
                                   type: string
+                                roleRef:
+                                  description: Reference to a key in a Secret that contains the App Role ID used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role id.
+                                  properties:
+                                    key:
+                                      description: The key of the entry in the Secret resource's `data` field to be used. Some instances of this field may be defaulted, in others it may be required.
+                                      type: string
+                                    name:
+                                      description: The name of the Secret resource being referred to.
+                                      type: string
+                                    namespace:
+                                      description: Namespace of the resource being referred to. Ignored if referent is not cluster-scoped. cluster-scoped defaults to the namespace of the referent.
+                                      type: string
+                                  type: object
                                 secretRef:
                                   description: Reference to a key in a Secret that contains the App Role secret used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role secret.
                                   properties:
@@ -2623,7 +2636,6 @@ spec:
                                   type: object
                               required:
                                 - path
-                                - roleId
                                 - secretRef
                               type: object
                             cert:
@@ -6153,6 +6165,19 @@ spec:
                                 roleId:
                                   description: RoleID configured in the App Role authentication backend when setting up the authentication backend in Vault.
                                   type: string
+                                roleRef:
+                                  description: Reference to a key in a Secret that contains the App Role ID used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role id.
+                                  properties:
+                                    key:
+                                      description: The key of the entry in the Secret resource's `data` field to be used. Some instances of this field may be defaulted, in others it may be required.
+                                      type: string
+                                    name:
+                                      description: The name of the Secret resource being referred to.
+                                      type: string
+                                    namespace:
+                                      description: Namespace of the resource being referred to. Ignored if referent is not cluster-scoped. cluster-scoped defaults to the namespace of the referent.
+                                      type: string
+                                  type: object
                                 secretRef:
                                   description: Reference to a key in a Secret that contains the App Role secret used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role secret.
                                   properties:
@@ -6168,7 +6193,6 @@ spec:
                                   type: object
                               required:
                                 - path
-                                - roleId
                                 - secretRef
                               type: object
                             cert:
@@ -7282,6 +7306,19 @@ spec:
                             roleId:
                               description: RoleID configured in the App Role authentication backend when setting up the authentication backend in Vault.
                               type: string
+                            roleRef:
+                              description: Reference to a key in a Secret that contains the App Role ID used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role id.
+                              properties:
+                                key:
+                                  description: The key of the entry in the Secret resource's `data` field to be used. Some instances of this field may be defaulted, in others it may be required.
+                                  type: string
+                                name:
+                                  description: The name of the Secret resource being referred to.
+                                  type: string
+                                namespace:
+                                  description: Namespace of the resource being referred to. Ignored if referent is not cluster-scoped. cluster-scoped defaults to the namespace of the referent.
+                                  type: string
+                              type: object
                             secretRef:
                               description: Reference to a key in a Secret that contains the App Role secret used to authenticate with Vault. The `key` field must be specified and denotes which entry within the Secret resource is used as the app role secret.
                               properties:
@@ -7297,7 +7334,6 @@ spec:
                               type: object
                           required:
                             - path
-                            - roleId
                             - secretRef
                           type: object
                         cert:

docs/api/spec.md

@@ -5413,12 +5413,30 @@ string
 </em>
 </td>
 <td>
+<em>(Optional)</em>
 <p>RoleID configured in the App Role authentication backend when setting
 up the authentication backend in Vault.</p>
 </td>
 </tr>
 <tr>
 <td>
+<code>roleRef</code></br>
+<em>
+<a href="https://pkg.go.dev/github.com/external-secrets/external-secrets/apis/meta/v1#SecretKeySelector">
+External Secrets meta/v1.SecretKeySelector
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Reference to a key in a Secret that contains the App Role ID used
+to authenticate with Vault.
+The <code>key</code> field must be specified and denotes which entry within the Secret
+resource is used as the app role id.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>secretRef</code></br>
 <em>
 <a href="https://pkg.go.dev/github.com/external-secrets/external-secrets/apis/meta/v1#SecretKeySelector">

pkg/provider/vault/vault.go

@@ -121,6 +121,8 @@ const (
 	errInvalidStoreSpec  = "invalid store spec"
 	errInvalidStoreProv  = "invalid store provider"
 	errInvalidVaultProv  = "invalid vault provider"
+	errInvalidAppRoleID  = "invalid Auth.AppRole: neither `roleId` nor `roleRef` was supplied"
+	errInvalidAppRoleRef = "invalid Auth.AppRole.RoleRef: %w"
 	errInvalidAppRoleSec = "invalid Auth.AppRole.SecretRef: %w"
 	errInvalidClientCert = "invalid Auth.Cert.ClientCert: %w"
 	errInvalidCertSec    = "invalid Auth.Cert.SecretRef: %w"
@@ -318,9 +320,21 @@ func (c *Connector) ValidateStore(store esv1beta1.GenericStore) error {
 		return fmt.Errorf(errInvalidVaultProv)
 	}
 	if p.Auth.AppRole != nil {
+		// check SecretRef for valid configuration
 		if err := utils.ValidateReferentSecretSelector(store, p.Auth.AppRole.SecretRef); err != nil {
 			return fmt.Errorf(errInvalidAppRoleSec, err)
 		}
+
+		// prefer .auth.appRole.roleId, fallback to .auth.appRole.roleRef, give up after that.
+		if p.Auth.AppRole.RoleID == "" { // prevents further RoleID tests if .auth.appRole.roleId is given
+			if p.Auth.AppRole.RoleRef != nil { // check RoleRef for valid configuration
+				if err := utils.ValidateReferentSecretSelector(store, *p.Auth.AppRole.RoleRef); err != nil {
+					return fmt.Errorf(errInvalidAppRoleRef, err)
+				}
+			} else { // we ran out of ways to get RoleID. return an appropriate error
+				return fmt.Errorf(errInvalidAppRoleID)
+			}
+		}
 	}
 	if p.Auth.Cert != nil {
 		if err := utils.ValidateReferentSecretSelector(store, p.Auth.Cert.ClientCert); err != nil {
@@ -1293,7 +1307,20 @@ func revokeTokenIfValid(ctx context.Context, client util.Client) error {
 }
 
 func (v *client) requestTokenWithAppRoleRef(ctx context.Context, appRole *esv1beta1.VaultAppRole) error {
-	roleID := strings.TrimSpace(appRole.RoleID)
+	var err error
+	var roleID string // becomes the RoleID used to authenticate with HashiCorp Vault
+
+	// prefer .auth.appRole.roleId, fallback to .auth.appRole.roleRef, give up after that.
+	if appRole.RoleID != "" { // use roleId from CRD, if configured
+		roleID = strings.TrimSpace(appRole.RoleID)
+	} else if appRole.RoleRef != nil { // use RoleID from Secret, if configured
+		roleID, err = v.secretKeyRef(ctx, appRole.RoleRef)
+		if err != nil {
+			return err
+		}
+	} else { // we ran out of ways to get RoleID. return an appropriate error
+		return fmt.Errorf(errInvalidAppRoleID)
+	}
 
 	secretID, err := v.secretKeyRef(ctx, &appRole.SecretRef)
 	if err != nil {

pkg/provider/vault/vault_test.go

@@ -1423,6 +1423,42 @@ func TestValidateStore(t *testing.T) {
 			wantErr: true,
 		},
 		{
+			name: "invalid approle with roleId and no roleRef",
+			args: args{
+				auth: esv1beta1.VaultAuth{
+					AppRole: &esv1beta1.VaultAppRole{
+						RoleID:  "",
+						RoleRef: nil,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "valid approle with roleId and no roleRef",
+			args: args{
+				auth: esv1beta1.VaultAuth{
+					AppRole: &esv1beta1.VaultAppRole{
+						RoleID: "fake-value",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid approle with roleId and no roleRef",
+			args: args{
+				auth: esv1beta1.VaultAuth{
+					AppRole: &esv1beta1.VaultAppRole{
+						RoleRef: &esmeta.SecretKeySelector{
+							Name: "fake-value",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
 			name: "invalid clientcert",
 			args: args{
 				auth: esv1beta1.VaultAuth{