diff --git a/.circleci/config.yml b/.circleci/config.yml index 6eb0e8d3..40a6d724 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -13,22 +13,6 @@ jobs: executor: golang-executor steps: - checkout - - restore_cache: - keys: - - vendor-{{ .Branch }}-{{ .Revision }}-{{ checksum "go.sum" }} - - vendor-{{ .Branch }}-{{ .Revision }}- - - run: - name: Golang vendor directory - command: | - if [ ! -d $GOPATH/src/github.com/$ORG_NAME/$PROJECT_NAME/vendor ]; then - cd $GOPATH/src/github.com/$ORG_NAME/$PROJECT_NAME && \ - make vendor - fi - - save_cache: - name: Saving cache for project vendor directory - key: vendor-{{ .Branch }}-{{ .Revision }}-{{ checksum "go.sum" }} - paths: - - vendor - run: name: Operator-sdk installation command: | @@ -50,10 +34,6 @@ jobs: - attach_workspace: at: /go - checkout - - restore_cache: - keys: - - vendor-{{ .Branch }}-{{ .Revision }}-{{ checksum "go.sum" }} - - vendor-{{ .Branch }}-{{ .Revision }}- - restore_cache: keys: - release-{{ .Revision }}-{{ .Environment.CIRCLE_TAG }} diff --git a/Makefile b/Makefile index 3146ad10..068d227e 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ default: all CIRCLE_TAG ?= latest -IMAGE_NAME = nmaupu/vault-secret:$(CIRCLE_TAG) +DOCKER_ID ?= nmaupu +IMAGE_NAME = $(DOCKER_ID)/vault-secret:$(CIRCLE_TAG) .PHONY: all all: build push diff --git a/deploy/crds/maupu.org_vaultsecrets_crd.yaml b/deploy/crds/maupu.org_vaultsecrets_crd.yaml index 8b56f4b5..64faa30c 100644 --- a/deploy/crds/maupu.org_vaultsecrets_crd.yaml +++ b/deploy/crds/maupu.org_vaultsecrets_crd.yaml @@ -92,6 +92,9 @@ spec: kvPath: description: Path of the key-value storage type: string + kvVersion: + description: Version of the KV backend + type: integer path: description: Path of the vault secret type: string diff --git a/pkg/apis/maupu/v1beta1/vaultsecret_types.go b/pkg/apis/maupu/v1beta1/vaultsecret_types.go index cebd278e..3f2517b6 100644 --- a/pkg/apis/maupu/v1beta1/vaultsecret_types.go +++ b/pkg/apis/maupu/v1beta1/vaultsecret_types.go @@ -17,7 +17,7 @@ type VaultSecretSpec struct { SyncPeriod metav1.Duration `json:"syncPeriod,omitempty"` } -// Configuration part of a vault-secret object +// VaultSecretSpecConfig Configuration part of a vault-secret object type VaultSecretSpecConfig struct { Addr string `json:"addr,required"` Namespace string `json:"namespace,omitempty"` @@ -25,27 +25,27 @@ type VaultSecretSpecConfig struct { Auth VaultSecretSpecConfigAuth `json:"auth,required"` } -// Mean of authentication for Vault +// VaultSecretSpecConfigAuth Mean of authentication for Vault type VaultSecretSpecConfigAuth struct { Token string `json:"token,omitempty"` Kubernetes KubernetesAuthType `json:"kubernetes,omitempty"` AppRole AppRoleAuthType `json:"approle,omitempty"` } -// Kubernetes authentication type +// KubernetesAuthType Kubernetes authentication type type KubernetesAuthType struct { Role string `json:"role,required"` Cluster string `json:"cluster,required"` } -// AppRole authentication type +// AppRoleAuthType AppRole authentication type type AppRoleAuthType struct { Name string `json:"name,omitempty"` RoleID string `json:"roleId,required"` SecretID string `json:"secretId,required"` } -// Define secrets to create from Vault +// VaultSecretSpecSecret Defines secrets to create from Vault type VaultSecretSpecSecret struct { // Key name in the secret to create SecretKey string `json:"secretKey,required"` @@ -55,16 +55,18 @@ type VaultSecretSpecSecret struct { Path string `json:"path,required"` // Field to retrieve from the path Field string `json:"field,required"` + // KvVersion is the version of the KV backend, if unspecified, try to automatically determine it + KvVersion int `json:"kvVersion,omitempty"` } -// Status field regarding last custom resource process +// VaultSecretStatus Status field regarding last custom resource process // +k8s:openapi-gen=true type VaultSecretStatus struct { // +listType=set Entries []VaultSecretStatusEntry `json:"entries,omitempty"` } -// Entry for the status field +// VaultSecretStatusEntry Entry for the status field type VaultSecretStatusEntry struct { Secret VaultSecretSpecSecret `json:"secret,required"` Status bool `json:"status,required"` diff --git a/pkg/apis/maupu/v1beta1/zz_generated.deepcopy.go b/pkg/apis/maupu/v1beta1/zz_generated.deepcopy.go index 87531d23..c524d1b2 100644 --- a/pkg/apis/maupu/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/maupu/v1beta1/zz_generated.deepcopy.go @@ -24,6 +24,26 @@ func (in *AppRoleAuthType) DeepCopy() *AppRoleAuthType { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in BySecretKey) DeepCopyInto(out *BySecretKey) { + { + in := &in + *out = make(BySecretKey, len(*in)) + copy(*out, *in) + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BySecretKey. +func (in BySecretKey) DeepCopy() BySecretKey { + if in == nil { + return nil + } + out := new(BySecretKey) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubernetesAuthType) DeepCopyInto(out *KubernetesAuthType) { *out = *in @@ -117,6 +137,7 @@ func (in *VaultSecretSpec) DeepCopyInto(out *VaultSecretSpec) { (*out)[key] = val } } + out.SyncPeriod = in.SyncPeriod return } diff --git a/pkg/controller/vaultsecret/vaultsecret_controller.go b/pkg/controller/vaultsecret/vaultsecret_controller.go index cbf1657d..985322f6 100644 --- a/pkg/controller/vaultsecret/vaultsecret_controller.go +++ b/pkg/controller/vaultsecret/vaultsecret_controller.go @@ -2,10 +2,10 @@ package vaultsecret import ( "context" - goerrors "errors" "fmt" "os" "sort" + "sync" "time" maupuv1beta1 "github.com/nmaupu/vault-secret/pkg/apis/maupu/v1beta1" @@ -33,11 +33,20 @@ const ( OperatorAppName = "vaultsecret-operator" // TimeFormat is the time format to indicate last updated field TimeFormat = "2006-01-02_15-04-05" + // MinTimeMsBetweenSecretUpdate avoid a secret to be updated too often + MinTimeMsBetweenSecretUpdate = time.Millisecond * 500 ) -var log = logf.Log.WithName(OperatorAppName) +var ( + log = logf.Log.WithName(OperatorAppName) -// LabelsFilter Fitlers events on labels + // secretsLastUpdateTime store last updated time of a secret to avoid reconciling too often + // the same secret if it changes very fast (like with database KV backend or OTP) + secretsLastUpdateTime = make(map[string]time.Time) + secretsLastUpdateTimeMutex sync.Mutex +) + +// LabelsFilter filters events on labels var LabelsFilter map[string]string // AddLabelFilter adds a label for filtering events @@ -71,23 +80,29 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { // Generic function for create, update and delete events // which verifies labels' filtering predFunc := func(e interface{}) bool { + reqLogger := log.WithValues("func", "predFunc") var objectLabels map[string]string - // Trying to determine what sort of event it is + // Trying to determine what sort of event we have // https://tour.golang.org/methods/16 switch e.(type) { case event.CreateEvent: + reqLogger.Info("Create event") objectLabels = e.(event.CreateEvent).Meta.GetLabels() case event.UpdateEvent: + reqLogger.Info("Update event") objectLabels = e.(event.UpdateEvent).MetaNew.GetLabels() case event.DeleteEvent: + reqLogger.Info("Delete event") objectLabels = e.(event.DeleteEvent).Meta.GetLabels() case event.GenericEvent: + reqLogger.Info("Generic event") objectLabels = e.(event.GenericEvent).Meta.GetLabels() default: // should never happen except if a new Event type is created return false } + // If labels match, we process the event, otherwise, simply ignore it // Verifying that each labels configured are present in the target object for lfk, lfv := range LabelsFilter { if val, ok := objectLabels[lfk]; ok { @@ -127,6 +142,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { IsController: true, OwnerType: &maupuv1beta1.VaultSecret{}, }, pred) + if err != nil { return err } @@ -160,6 +176,7 @@ func (r *ReconcileVaultSecret) Reconcile(request reconcile.Request) (reconcile.R // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue + reqLogger.Info("IsNotFound error when retrieving the VaultSecret object") return reconcile.Result{}, nil } @@ -168,55 +185,68 @@ func (r *ReconcileVaultSecret) Reconcile(request reconcile.Request) (reconcile.R return reconcile.Result{}, err } - // Define a new Secret object from CR specs - secretFromCR, err := newSecretForCR(CRInstance) - if err != nil && secretFromCR == nil { - // An error occurred, requeue - reqLogger.Error(err, "An error occurred when creating secret from CR, requeuing.") - return reconcile.Result{}, err - } else if err != nil && secretFromCR != nil { - // Some vault path and/or fields are not found, update CR (status) and requeue - if updateErr := r.client.Status().Update(context.TODO(), CRInstance); updateErr != nil { - reqLogger.Error(updateErr, fmt.Sprintf("Some errors occurred but CR cannot be updated, requeuing, original error=%v", err)) - } else { - reqLogger.Error(err, "Some errors have been issued in the CR status information, please check, requeuing") + // Only updating stuff if two updates are not too close from each other + // See secretsLastUpdateTime and MinTimeMsBetweenSecretUpdate variables + updateTimeKey := fmt.Sprintf("%s/%s", CRInstance.GetNamespace(), CRInstance.Spec.SecretName) + secretsLastUpdateTimeMutex.Lock() + defer secretsLastUpdateTimeMutex.Unlock() + ti := secretsLastUpdateTime[updateTimeKey] // no problem if it does not exist: it returns a default time.Time object (set to zero) + now := time.Now() + if now.Sub(ti) > MinTimeMsBetweenSecretUpdate { + // Define a new Secret object from CR specs + secretFromCR, err := newSecretForCR(CRInstance) + if err != nil && secretFromCR == nil { + // An error occurred, requeue + reqLogger.Error(err, "An error occurred when creating secret from CR, requeuing.") + return reconcile.Result{}, err + } else if err != nil && secretFromCR != nil { + // Some vault path and/or fields are not found, update CR (status) and requeue + if updateErr := r.client.Status().Update(context.TODO(), CRInstance); updateErr != nil { + reqLogger.Error(updateErr, fmt.Sprintf("Some errors occurred but CR cannot be updated, requeuing, original error=%v", err)) + } else { + reqLogger.Error(err, "Some errors have been issued in the CR status information, please check, requeuing") + } + return reconcile.Result{}, err } - return reconcile.Result{}, err - } - // Everything's ok + // Everything's ok - // Set VaultSecret CRInstance as the owner and controller - if err = controllerutil.SetControllerReference(CRInstance, secretFromCR, r.scheme); err != nil { - reqLogger.Error(err, "An error occurred when setting controller reference, requeuing") - return reconcile.Result{}, err - } + // Set VaultSecret CRInstance as the owner and controller + if err = controllerutil.SetControllerReference(CRInstance, secretFromCR, r.scheme); err != nil { + reqLogger.Error(err, "An error occurred when setting controller reference, requeuing") + return reconcile.Result{}, err + } - // Creating or updating secret resource from CR - // Check if this Secret already exists - found := &corev1.Secret{} - err = r.client.Get(context.TODO(), types.NamespacedName{Name: secretFromCR.Name, Namespace: secretFromCR.Namespace}, found) - if err != nil && errors.IsNotFound(err) { - // Secret does not exist, creating it - reqLogger.Info(fmt.Sprintf("Creating new Secret %s/%s", secretFromCR.Namespace, secretFromCR.Name)) - err = r.client.Create(context.TODO(), secretFromCR) - } else { - // Secret already exists - updating - reqLogger.Info(fmt.Sprintf("Reconciling existing Secret %s/%s", found.Namespace, found.Name)) - err = r.client.Update(context.TODO(), secretFromCR) - } + // Creating or updating secret resource from CR + // Check if this Secret already exists + found := &corev1.Secret{} + err = r.client.Get(context.TODO(), types.NamespacedName{Name: secretFromCR.Name, Namespace: secretFromCR.Namespace}, found) + if err != nil && errors.IsNotFound(err) { + // Secret does not exist, creating it + reqLogger.Info(fmt.Sprintf("Creating new Secret %s/%s", secretFromCR.Namespace, secretFromCR.Name)) + err = r.client.Create(context.TODO(), secretFromCR) + } else { + // Secret already exists - updating + reqLogger.Info(fmt.Sprintf("Reconciling existing Secret %s/%s", found.Namespace, found.Name)) + err = r.client.Update(context.TODO(), secretFromCR) + } + + // No problem creating or updating secret, updating CR info + reqLogger.Info("Updating CR status information") + if updateErr := r.client.Status().Update(context.TODO(), CRInstance); updateErr != nil { + reqLogger.Error(updateErr, "Error occurred when updating CR status") + } - // No problem creating or updating secret, updating CR info - /*reqLogger.Info("Updating CR status information") - if updateErr := r.client.Status().Update(context.TODO(), CRInstance); updateErr != nil { - reqLogger.Error(updateErr, "An error occurred when updating CR status") - }*/ + // Updating "update time" at the very end to take into account all potential requeue above + secretsLastUpdateTime[updateTimeKey] = now + } - // finally return giving err (nil if not problem occurred, set to something otherwise) + // finally returning given err (nil if no problem occurred, set to something otherwise) return reconcile.Result{RequeueAfter: CRInstance.Spec.SyncPeriod.Duration}, err } func newSecretForCR(cr *maupuv1beta1.VaultSecret) (*corev1.Secret, error) { + reqLogger := log.WithValues("func", "newSecretForCR") operatorName := os.Getenv("OPERATOR_NAME") if operatorName == "" { operatorName = OperatorAppName @@ -271,18 +301,39 @@ func newSecretForCR(cr *maupuv1beta1.VaultSecret) (*corev1.Secret, error) { // Clear status slice cr.Status.Entries = nil + // Creating secret data from CR + // Each secret entry in the CR will need a vault read to get filled. + // If the KV/path remain the same, it's useless to call the vault again + // as all fields are returned in the original read. + // As a result, we are storing temporarily vault's data and use it as a "cache" to avoid + // overloading the vault server. + // This will be GC'ed at the end of the func + cache := make(map[string](map[string]interface{}), 0) + // Sort by secret keys to avoid updating the resource if order changes specSecrets := cr.Spec.Secrets sort.Sort(maupuv1beta1.BySecretKey(specSecrets)) // Creating secret data from CR for _, s := range specSecrets { + var err error errMessage := "" rootErrMessage := "" status := true // Vault read - sec, err := nmvault.Read(vclient, s.KvPath, s.Path) + var sec map[string]interface{} + cacheKey := fmt.Sprintf("%s/%s", s.KvPath, s.Path) + if cacheVal, ok := cache[cacheKey]; ok { + sec = cacheVal + err = nil + } else { + reqLogger.Info(fmt.Sprintf("Reading from vault with the following info, path=%s, kvVersion=%d", cacheKey, s.KvVersion)) + sec, err = nmvault.Read(vclient, s.KvPath, s.Path, s.KvVersion) + if err != nil || sec != nil { // only cache value if there is no error or a sec returned + cache[cacheKey] = sec + } + } if err != nil { hasError = true @@ -318,7 +369,7 @@ func newSecretForCR(cr *maupuv1beta1.VaultSecret) (*corev1.Secret, error) { var retErr error retErr = nil if hasError { - retErr = goerrors.New(fmt.Sprintf("Secret %s cannot be created, see CR Status field for details", cr.Spec.SecretName)) + retErr = fmt.Errorf("Secret %s cannot be created, see CR Status field for details", cr.Spec.SecretName) } return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/vault/utils.go b/pkg/vault/utils.go index 493c646c..b2ce32ee 100644 --- a/pkg/vault/utils.go +++ b/pkg/vault/utils.go @@ -1,8 +1,6 @@ package vault import ( - "errors" - "fmt" "path" "strings" @@ -13,8 +11,29 @@ const ( VaultKVWarning = "Invalid path for a versioned K/V secrets engine." ) -// Read a path from vault taking account KV version 1 and 2 automatically -func Read(vc *vapi.Client, kvPath string, secretPath string) (map[string]interface{}, error) { +// Read reads a path from vault taking account KV version 1 and 2 if specified or automatically if not +// to automatically discover the kvVersion of the backend, pass kvVersion = 0 +func Read(vc *vapi.Client, kvPath string, secretPath string, kvVersion int) (map[string]interface{}, error) { + switch kvVersion { + case 1: + sec, err := read(vc, path.Join(kvPath, secretPath)) + if err != nil { + return nil, err + } + return sec.Data, nil + case 2: + sec, err := read(vc, path.Join(kvPath, "data", secretPath)) + if err != nil { + return nil, err + } + return sec.Data["data"].(map[string]interface{}), nil + } + + // Defaulting to auto detecting KV version + return readWithAutoKvVersion(vc, kvPath, secretPath) +} + +func readWithAutoKvVersion(vc *vapi.Client, kvPath string, secretPath string) (map[string]interface{}, error) { p := path.Join(kvPath, secretPath) pathV1 := path.Join(kvPath, secretPath) @@ -23,50 +42,61 @@ func Read(vc *vapi.Client, kvPath string, secretPath string) (map[string]interfa var data map[string]interface{} // Trying V1 type URL - // Might fail (err!=nil with a 403) if policy is for a v2 backend (including data in the path) + // Might fail (err!=nil with a 403) if policy is for a v2 backend (including /data in the path) sec, err := read(vc, pathV1) - if err != nil || (sec != nil && contains(sec.Warnings, VaultKVWarning)) { - // Need a V2 KV type read - sec, err := read(vc, pathV2) - if err != nil { - return nil, err - } + if err != nil { + switch err.(type) { + case *WrongVersionError: + // Need a V2 KV type read + sec, err := read(vc, pathV2) + if err != nil { + return nil, err + } - if sec != nil && sec.Data != nil && sec.Data["data"] != nil { - data = sec.Data["data"].(map[string]interface{}) + if sec != nil && sec.Data != nil && sec.Data["data"] != nil { + // Get the inner data object (v2 KV) + data = sec.Data["data"].(map[string]interface{}) + } + default: + return nil, err } } else if sec != nil { + // Get the raw data object (v1 KV) data = sec.Data } else { - data = nil + return nil, &PathNotFound{p} } - if data == nil { - return nil, errors.New(fmt.Sprintf("No secret found for path=%v", p)) - } else { - return data, nil - } + return data, nil } func read(vc *vapi.Client, p string) (*vapi.Secret, error) { + //reqLogger := log.WithValues() + sec, err := vc.Logical().Read(p) + //reqLogger.Info(fmt.Sprintf("Reading from vault path=%s, sec=%+v", p, sec)) + if err != nil { // An unknown error occurred return nil, err + } else if err == nil && sec != nil && contains(sec.Warnings, VaultKVWarning) >= 0 { + // Calling with a v1 path but needs v2 path + idx := contains(sec.Warnings, VaultKVWarning) + return nil, &WrongVersionError{sec.Warnings[idx]} } else if err == nil && sec == nil { - return nil, errors.New(fmt.Sprintf("Secret path %s not found", p)) + return nil, &PathNotFound{p} } else { return sec, nil } } // Check wether s contains str or not -func contains(s []string, str string) bool { - for _, v := range s { +func contains(s []string, str string) int { + for k, v := range s { if strings.Contains(v, str) { - return true + return k } } - return false + return -1 } diff --git a/pkg/vault/vaultErrors.go b/pkg/vault/vaultErrors.go new file mode 100644 index 00000000..27cdd150 --- /dev/null +++ b/pkg/vault/vaultErrors.go @@ -0,0 +1,23 @@ +package vault + +import "fmt" + +// WrongVersionError represents an error raised when the KV version is not correct +type WrongVersionError struct { + Message string +} + +// Error +func (e *WrongVersionError) Error() string { + return e.Message +} + +// PathNotFound represents an error when a path is not found in vault +type PathNotFound struct { + Path string +} + +// Error +func (e *PathNotFound) Error() string { + return fmt.Sprintf("Path %s not found", e.Path) +}