Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@
From version `1.0.1`, k8s auth method switches from using the local *service account* configured on the operator side to using the one from the client's namespace defined in the *custom resource*.
This is improving security but as a result, you will probably have to check your vault configuration is in adequation with this change.

# Note for Kubernetes 1.24+

From Kubernetes 1.24, secrets are not created along a service account anymore. A secret needs to be manually created to make the controller happy.
See https://github.com/nmaupu/vault-secret/issues/40 for more info.

# Installation

## Kubernetes version requirements
Expand Down Expand Up @@ -212,6 +207,7 @@ If several configuration options are specified, there are used in the following

- Operator SDK installation (https://github.com/operator-framework/operator-sdk)
- Go Dep (https://golang.github.io/dep/docs/installation.html)
- Kubebuilder (https://github.com/kubernetes-sigs/kubebuilder)

## Building

Expand Down
10 changes: 2 additions & 8 deletions api/v1beta1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

"github.com/nmaupu/vault-secret/pkg/k8sutils"
nmvault "github.com/nmaupu/vault-secret/pkg/vault"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// BySecretKey allows sorting an array of VaultSecretSpecSecret by SecretKey
Expand All @@ -37,7 +36,7 @@ func (a BySecretKey) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
func (a BySecretKey) Less(i, j int) bool { return a[i].SecretKey < a[j].SecretKey }

// GetVaultAuthProvider implem from custom resource object
func (cr *VaultSecret) GetVaultAuthProvider(c client.Client) (nmvault.AuthProvider, error) {
func (cr *VaultSecret) GetVaultAuthProvider() (nmvault.AuthProvider, error) {
// Checking order:
// - Token
// - AppRole
Expand All @@ -55,13 +54,8 @@ func (cr *VaultSecret) GetVaultAuthProvider(c client.Client) (nmvault.AuthProvid
cr.Spec.Config.Auth.AppRole.SecretID,
), nil
} else if cr.Spec.Config.Auth.Kubernetes.Role != "" {
// Retrieving token from the serviceAccount configured
saName := cr.Spec.Config.Auth.Kubernetes.ServiceAccount
if saName == "" {
saName = "default"
}

tok, err := k8sutils.GetTokenFromSA(c, cr.Namespace, saName)
tok, err := k8sutils.GetTokenFromSA()
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/vaultsecret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func (r *VaultSecretReconciler) readSecretData(cr *maupuv1beta1.VaultSecret) (ma
reqLogger := log.WithValues("func", "readSecretData")

// Authentication provider
authProvider, err := cr.GetVaultAuthProvider(r.Client)
authProvider, err := cr.GetVaultAuthProvider()
if err != nil {
return nil, nil, err
}
Expand Down
37 changes: 6 additions & 31 deletions pkg/k8sutils/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,15 @@ limitations under the License.
package k8sutils

import (
"context"
"fmt"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"io/ioutil"
)

// GetTokenFromSA gets the token associated to the first secret located in a k8s' service account
func GetTokenFromSA(cli client.Client, ns, saName string) (string, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be completely misunderstanding this ... but with this change it seems to me that now we are just reading the secret for the SA the vault-secrets-manager pod is running with instead of the one defined in the CR

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right! I missed this part :(

We deploy the vault-secrets-manager to every namespace (with a single value for WATCH_NAMESPACE env) and then using the SA of the operator makes more sense... However, indeed, this kills the possibility of using single-operator installation and keeping more granular access to Vault. I guess, the PR will be declined

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes security wise, it's bad. The operator should connect on behalf of the client SA.

I currently have very little access to internet and only have my phone, so I can't really work on that before Sept. I am afraid... I can merge PR though ;)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this PR will break installations, which watch multiple namespaces. I'm converting the PR into a draft

if cli == nil {
return "", fmt.Errorf("Cannot get token from service account, k8s client is nil")
}

// Getting SA
saClient := &corev1.ServiceAccount{}
err := cli.Get(context.TODO(), types.NamespacedName{Name: saName, Namespace: ns}, saClient)
if err != nil && errors.IsNotFound(err) {
return "", fmt.Errorf("Unable to retrieve service account, err=%v", err)
}

if len(saClient.Secrets) == 0 {
return "", fmt.Errorf("No secret associated with the service account %s/%s", ns, saName)
}

// TODO See how to handle this slice of Secrets instead of taking the first one
saSecret := saClient.Secrets[0]
secret := &corev1.Secret{}
err = cli.Get(context.TODO(), types.NamespacedName{Name: saSecret.Name, Namespace: ns}, secret)
// GetTokenFromSA gets the SA token the projected volume mount
func GetTokenFromSA() (string, error) {
token, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/token")
if err != nil {
return "", fmt.Errorf("Unable to retrieve the secret from the service account, err=%v", err)
return "", fmt.Errorf("Unable to retrieve service account, err=%v", err)
}

// Finally, set the token
return string(secret.Data["token"]), nil
return string(token), nil
}