From 193ccbe7cc975acd605422bd81af8c7993c39460 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 29 Jan 2019 15:48:47 +0100 Subject: [PATCH 1/7] bump version to v0.2.0-rc.2 Modifies makefile to reflect the new version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8cd97f3..07b742f 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ # Binary names BUILD_FOLDER=build BINARY_NAME=secrets-manager -SECRETS_MANAGER_VERSION=v0.2.0-rc.1 +SECRETS_MANAGER_VERSION=v0.2.0-rc.2 DOCKER_REGISTRY ?= "registry.hub.docker.com" DOCKER_IMAGE_NAME=${BINARY_NAME} DOCKER_IMAGE=${DOCKER_REGISTRY}/${DOCKER_IMAGE_NAME}:${SECRETS_MANAGER_VERSION} From a9fd806c6001edc6c8105aa2e5771ea6ab4acf8a Mon Sep 17 00:00:00 2001 From: Fernando Llaca Date: Sun, 27 Jan 2019 23:02:05 +0100 Subject: [PATCH 2/7] Use Golang as base image for building binaries. Avoid using deprecated third-party Docker images. --- .dockerignore | 1 + Dockerfile | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.dockerignore b/.dockerignore index 270dad8..5b6ea89 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,4 @@ vendor/ build/ Dockerfile +.git diff --git a/Dockerfile b/Dockerfile index 1ee6096..d56423f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,8 +1,14 @@ # Stage 0 # Build binary file -FROM instrumentisto/glide:0.13.1-go1.10 +FROM golang:1.10-alpine +ENV GLIDE_VERSION v0.13.2 -RUN apk add --update make +RUN wget "https://github.com/Masterminds/glide/releases/download/${GLIDE_VERSION}/glide-${GLIDE_VERSION}-linux-amd64.tar.gz" \ + && tar xf glide-${GLIDE_VERSION}-linux-amd64.tar.gz \ + && cp linux-amd64/glide $GOPATH/bin/ \ + && chmod +x $GOPATH/bin/glide + +RUN apk add --update git make ARG PROJECT_SLUG=github.com/tuenti/secrets-manager COPY glide.yaml /go/src/$PROJECT_SLUG/glide.yaml @@ -18,5 +24,6 @@ RUN make build-linux # Build actual docker image FROM alpine ARG PROJECT_SLUG=github.com/tuenti/secrets-manager +LABEL maintainer="sre@tuenti.com" COPY --from=0 /go/src/$PROJECT_SLUG/build/secrets-manager /secrets-manager ENTRYPOINT ["/secrets-manager"] From f259df01d25ace200e91c6b4e47c960d6f1a3046 Mon Sep 17 00:00:00 2001 From: Fernando Llaca Date: Tue, 29 Jan 2019 14:10:46 +0100 Subject: [PATCH 3/7] Use specific Alpine version --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index d56423f..199dfb4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,7 +22,7 @@ RUN make build-linux # Stage 1 # Build actual docker image -FROM alpine +FROM alpine:3.8 ARG PROJECT_SLUG=github.com/tuenti/secrets-manager LABEL maintainer="sre@tuenti.com" COPY --from=0 /go/src/$PROJECT_SLUG/build/secrets-manager /secrets-manager From 57f9a157aa5722337768c146845c12cf5cb47b89 Mon Sep 17 00:00:00 2001 From: Fernando Llaca Date: Tue, 29 Jan 2019 15:04:14 +0100 Subject: [PATCH 4/7] Use Golang 1.11.5 --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 199dfb4..2a8eb17 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # Stage 0 # Build binary file -FROM golang:1.10-alpine +FROM golang:1.11.5-alpine as builder ENV GLIDE_VERSION v0.13.2 RUN wget "https://github.com/Masterminds/glide/releases/download/${GLIDE_VERSION}/glide-${GLIDE_VERSION}-linux-amd64.tar.gz" \ @@ -25,5 +25,5 @@ RUN make build-linux FROM alpine:3.8 ARG PROJECT_SLUG=github.com/tuenti/secrets-manager LABEL maintainer="sre@tuenti.com" -COPY --from=0 /go/src/$PROJECT_SLUG/build/secrets-manager /secrets-manager +COPY --from=builder /go/src/$PROJECT_SLUG/build/secrets-manager /secrets-manager ENTRYPOINT ["/secrets-manager"] From 57d891a2ef006469928e78b65b68375e2b6a9c8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Crespo=20Gr=C3=A1valos?= Date: Tue, 29 Jan 2019 16:12:36 +0100 Subject: [PATCH 5/7] simplifies and makes clearer all vault prometheus metrics --- CHANGELOG.md | 10 +++ README.md | 5 +- backend/vault.go | 30 ++++----- backend/vault_metrics.go | 72 ++++++++------------- backend/vault_metrics_test.go | 56 ++++++++--------- backend/vault_test.go | 114 +++++++++++++++++----------------- 6 files changed, 134 insertions(+), 153 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95687a6..6745c5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## v0.2.0-rc.2 - 2019-01-29 + +### Added +- New `secrets_manager_vault_max_token_ttl` metric, so a user could alert based on this and `secrets_manager_token_ttl` + +### Fixed +- Deprecates `secrets_manager_vault_token_expired` metric as it was quite confusing since it's not really possible for `secrets-manager` to know when the token it's expired, just when it's "close to expire". +- Renames counter metrics to follow the Prometheus naming standard with the `_total` suffix instead of `_count`. +- Simplifies prometheus token renewal metrics by merging `secrets_manager_vault_token_lookup_errors_count` and `secrets_manager_vault_token_renew_errors_count` into one single metric `secrets_manager_vault_token_renewal_errors_total` with one more dimension called `vault_operation` which will be one of `lookup-self, renew-self, is-renewable`. + ## v0.2.0-rc.1 - 2019-01-21 ### Added diff --git a/README.md b/README.md index 491a0a2..07d5548 100644 --- a/README.md +++ b/README.md @@ -104,10 +104,9 @@ data: | Metric| Type| Description| Labels| | ------| ----|------------| ------| -|`secrets_manager_vault_token_expired` | Gauge | Whether or not token TTL is under `vault.max-token-ttl`: 1 = expired; 0 = still valid | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"` | +|`secrets_manager_vault_max_token_ttl` | Gauge | `secrets-manager` max Vault token TTL | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"` | |`secrets_manager_vault_token_ttl` | Gauge | Vault token TTL | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"` | -|`secrets_manager_vault_token_lookup_errors_count`| Counter | Vault token lookup-self errors counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "error"` | -|`secrets_manager_vault_token_renew_errors_count`| Counter | Vault token renew-self errors counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "error"` | +|`secrets_manager_vault_token_renewal_errors_total`| Counter | Vault token renewal errors counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "vault_operation", "error"` | |`secrets_manager_read_secret_errors_count`| Counter | Vault read operations counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "path", "key", "error"` | | `secrets_manager_secret_sync_errors_count`| Counter |Secrets sync error counter|`"name", "namespace"`| |`secrets_manager_secret_last_updated`| Gauge |The last update timestamp as a Unix time (the number of seconds elapsed since January 1, 1970 UTC)|`"name", "namespace"`| diff --git a/backend/vault.go b/backend/vault.go index a135e95..6fd4520 100644 --- a/backend/vault.go +++ b/backend/vault.go @@ -72,6 +72,9 @@ func vaultClient(l *log.Logger, cfg Config) (*client, error) { renewTTLIncrement: cfg.VaultRenewTTLIncrement, engine: engine, } + + metrics.updateVaultMaxTokenTTLMetric(cfg.VaultMaxTokenTTL) + return &client, err } @@ -80,7 +83,7 @@ func (c *client) getToken() (*api.Secret, error) { lookup, err := auth.Token().LookupSelf() if err != nil { logger.Errorf("error checking token with lookup self api: %v", err) - metrics.updateVaultTokenLookupErrorsCountMetric(errors.UnknownErrorType) + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultLookupSelfOperationName, errors.UnknownErrorType) return nil, err } return lookup, nil @@ -97,31 +100,22 @@ func (c *client) getTokenTTL(token *api.Secret) (int64, error) { return ttl, nil } -func (c *client) shouldRenewToken(ttl int64) bool { - if ttl < c.maxTokenTTL { - metrics.updateVaultTokenExpiredMetric(vaultTokenExpired) - return true - } - metrics.updateVaultTokenExpiredMetric(vaultTokenNotExpired) - return false -} - func (c *client) renewToken(token *api.Secret) error { isRenewable, err := token.TokenIsRenewable() if err != nil { logger.Errorf("could not check token renewability: %v", err) - metrics.updateVaultTokenRenewErrorsCountMetric(errors.UnknownErrorType) + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultIsRenewableOperationName, errors.UnknownErrorType) return err } if !isRenewable { - metrics.updateVaultTokenRenewErrorsCountMetric(errors.VaultTokenNotRenewableErrorType) + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultIsRenewableOperationName, errors.VaultTokenNotRenewableErrorType) err = &errors.VaultTokenNotRenewableError{ErrType: errors.VaultTokenNotRenewableErrorType} return err } auth := c.vclient.Auth() if _, err = auth.Token().RenewSelf(c.renewTTLIncrement); err != nil { log.Errorf("failed to renew token: %v", err) - metrics.updateVaultTokenRenewErrorsCountMetric(errors.UnknownErrorType) + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultRenewSelfOperationName, errors.UnknownErrorType) return err } return nil @@ -137,7 +131,7 @@ func (c *client) renewalLoop() { if err != nil { logger.Errorf("failed to read token TTL: %v", err) return - } else if c.shouldRenewToken(ttl) { + } else if ttl < c.maxTokenTTL { logger.Warnf("token is really close to expire, current ttl: %d", ttl) err := c.renewToken(token) if err != nil { @@ -174,7 +168,7 @@ func (c *client) ReadSecret(path string, key string) (string, error) { logical := c.logical secret, err := logical.Read(path) if err != nil { - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.UnknownErrorType) + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.UnknownErrorType) return data, err } @@ -185,18 +179,18 @@ func (c *client) ReadSecret(path string, key string) (string, error) { if secretData[key] != nil { data = secretData[key].(string) } else { - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.BackendSecretNotFoundErrorType) + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.BackendSecretNotFoundErrorType) err = &errors.BackendSecretNotFoundError{ErrType: errors.BackendSecretNotFoundErrorType, Path: path, Key: key} } } else { for _, w := range warnings { logger.Warningln(w) } - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.BackendSecretNotFoundErrorType) + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.BackendSecretNotFoundErrorType) err = &errors.BackendSecretNotFoundError{ErrType: errors.BackendSecretNotFoundErrorType, Path: path, Key: key} } } else { - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.BackendSecretNotFoundErrorType) + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.BackendSecretNotFoundErrorType) err = &errors.BackendSecretNotFoundError{ErrType: errors.BackendSecretNotFoundErrorType, Path: path, Key: key} } return data, err diff --git a/backend/vault_metrics.go b/backend/vault_metrics.go index b3c06b4..c1e3b91 100644 --- a/backend/vault_metrics.go +++ b/backend/vault_metrics.go @@ -3,44 +3,39 @@ package backend import "github.com/prometheus/client_golang/prometheus" const ( - vaultTokenExpired = 1 - vaultTokenNotExpired = 0 + vaultLookupSelfOperationName = "lookup-self" + vaultRenewSelfOperationName = "renew-self" + vaultIsRenewableOperationName = "is-renewable" ) var ( - vaultLabelNames = []string{"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"} - secretLabelNames = []string{"path", "key", "error"} - errorLabelNames = []string{"error"} + vaultLabelNames = []string{"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"} + secretLabelNames = []string{"path", "key", "error"} + vaultErrorLabelNames = []string{"vault_operation", "error"} // Prometeheus metrics: https://prometheus.io - tokenExpired = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: "secrets_manager", - Subsystem: "vault", - Name: "token_expired", - Help: "The state of the token: 1 = expired; 0 = still valid", - }, vaultLabelNames) tokenTTL = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "secrets_manager", Subsystem: "vault", Name: "token_ttl", Help: "Vault token TTL", }, vaultLabelNames) - tokenLookupErrorsCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + maxTokenTTL = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "secrets_manager", Subsystem: "vault", - Name: "token_lookup_errors_count", - Help: "Vault token lookup-self errors counter", - }, append(vaultLabelNames, errorLabelNames...)) - tokenRenewErrorsCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "max_token_ttl", + Help: "secrets-manager max Vault token TTL", + }, vaultLabelNames) + tokenRenewalErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "secrets_manager", Subsystem: "vault", - Name: "token_renew_errors_count", - Help: "Vault token renew-self errors counter", - }, append(vaultLabelNames, errorLabelNames...)) - secretReadErrorsCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "token_renewal_errors_total", + Help: "Vault token renewal errors counter", + }, append(vaultLabelNames, vaultErrorLabelNames...)) + secretReadErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "secrets_manager", Subsystem: "vault", - Name: "read_secret_errors_count", + Name: "read_secret_errors_total", Help: "Vault read operations counter", }, append(vaultLabelNames, secretLabelNames...)) ) @@ -50,11 +45,10 @@ type vaultMetrics struct { } func init() { - prometheus.MustRegister(tokenExpired) prometheus.MustRegister(tokenTTL) - prometheus.MustRegister(tokenLookupErrorsCount) - prometheus.MustRegister(tokenRenewErrorsCount) - prometheus.MustRegister(secretReadErrorsCount) + prometheus.MustRegister(maxTokenTTL) + prometheus.MustRegister(tokenRenewalErrorsTotal) + prometheus.MustRegister(secretReadErrorsTotal) } func newVaultMetrics(vaultAddr string, vaultVersion string, vaultEngine string, vaultClusterID string, vaultClusterName string) *vaultMetrics { @@ -68,13 +62,8 @@ func newVaultMetrics(vaultAddr string, vaultVersion string, vaultEngine string, return &vaultMetrics{vaultLabels: labels} } -func (vm *vaultMetrics) updateVaultTokenExpiredMetric(value int) { - if value != vaultTokenExpired && value != vaultTokenNotExpired { - logger.Errorf("refusing to update secrets_manager_vault_token_expired metric with value %d. Allowed values are %d and %d", value, vaultTokenExpired, vaultTokenNotExpired) - return - } - - tokenExpired.WithLabelValues( +func (vm *vaultMetrics) updateVaultMaxTokenTTLMetric(value int64) { + maxTokenTTL.WithLabelValues( vm.vaultLabels["vault_addr"], vm.vaultLabels["vault_engine"], vm.vaultLabels["vault_version"], @@ -91,8 +80,8 @@ func (vm *vaultMetrics) updateVaultTokenTTLMetric(value int64) { vm.vaultLabels["vault_cluster_name"]).Set(float64(value)) } -func (vm *vaultMetrics) updateVaultSecretReadErrorsCountMetric(path string, key string, errorType string) { - secretReadErrorsCount.WithLabelValues( +func (vm *vaultMetrics) updateVaultSecretReadErrorsTotalMetric(path string, key string, errorType string) { + secretReadErrorsTotal.WithLabelValues( vm.vaultLabels["vault_addr"], vm.vaultLabels["vault_engine"], vm.vaultLabels["vault_version"], @@ -103,22 +92,13 @@ func (vm *vaultMetrics) updateVaultSecretReadErrorsCountMetric(path string, key errorType).Inc() } -func (vm *vaultMetrics) updateVaultTokenLookupErrorsCountMetric(errorType string) { - tokenLookupErrorsCount.WithLabelValues( - vm.vaultLabels["vault_addr"], - vm.vaultLabels["vault_engine"], - vm.vaultLabels["vault_version"], - vm.vaultLabels["vault_cluster_id"], - vm.vaultLabels["vault_cluster_name"], - errorType).Inc() -} - -func (vm *vaultMetrics) updateVaultTokenRenewErrorsCountMetric(errorType string) { - tokenRenewErrorsCount.WithLabelValues( +func (vm *vaultMetrics) updateVaultTokenRenewalErrorsTotalMetric(vaultOperation string, errorType string) { + tokenRenewalErrorsTotal.WithLabelValues( vm.vaultLabels["vault_addr"], vm.vaultLabels["vault_engine"], vm.vaultLabels["vault_version"], vm.vaultLabels["vault_cluster_id"], vm.vaultLabels["vault_cluster_name"], + vaultOperation, errorType).Inc() } diff --git a/backend/vault_metrics_test.go b/backend/vault_metrics_test.go index a83d2fa..3e14826 100644 --- a/backend/vault_metrics_test.go +++ b/backend/vault_metrics_test.go @@ -16,13 +16,13 @@ const ( fakeVaultClusterName = "vault-fake" ) -func TestUpdateTokenExpired(t *testing.T) { +func TestUpdateMaxTokenTTL(t *testing.T) { metrics := newVaultMetrics(fakeVaultAddress, fakeVaultVersion, fakeVaultEngine, fakeVaultClusterID, fakeVaultClusterName) - tokenExpired.Reset() - metrics.updateVaultTokenExpiredMetric(1) - metricTokenExpired, _ := tokenExpired.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName) + maxTokenTTL.Reset() + metrics.updateVaultMaxTokenTTLMetric(600) + metricMaxTokenTTL, _ := maxTokenTTL.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenExpired)) + assert.Equal(t, 600.0, testutil.ToFloat64(metricMaxTokenTTL)) } func TestUpdateTokenTTL(t *testing.T) { @@ -34,44 +34,44 @@ func TestUpdateTokenTTL(t *testing.T) { assert.Equal(t, 300.0, testutil.ToFloat64(metricTokenTTL)) } -func TestUpdateTokenLookupErrorsCount(t *testing.T) { +func TestUpdateTokenLookupErrorsTotal(t *testing.T) { metrics := newVaultMetrics(fakeVaultAddress, fakeVaultVersion, fakeVaultEngine, fakeVaultClusterID, fakeVaultClusterName) - tokenLookupErrorsCount.Reset() - metrics.updateVaultTokenLookupErrorsCountMetric(errors.UnknownErrorType) - metricTokenLookupErrorsCount, _ := tokenLookupErrorsCount.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, errors.UnknownErrorType) + tokenRenewalErrorsTotal.Reset() + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultLookupSelfOperationName, errors.UnknownErrorType) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, vaultLookupSelfOperationName, errors.UnknownErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenLookupErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) } -func TestUpdateTokenRenewErrorsCount(t *testing.T) { +func TestUpdateTokenRenewErrorsTotal(t *testing.T) { metrics := newVaultMetrics(fakeVaultAddress, fakeVaultVersion, fakeVaultEngine, fakeVaultClusterID, fakeVaultClusterName) - tokenRenewErrorsCount.Reset() - metrics.updateVaultTokenRenewErrorsCountMetric(errors.UnknownErrorType) - metricTokenRenewErrorsCount, _ := tokenRenewErrorsCount.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, errors.UnknownErrorType) + tokenRenewalErrorsTotal.Reset() + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultRenewSelfOperationName, errors.UnknownErrorType) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, vaultRenewSelfOperationName, errors.UnknownErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) - tokenRenewErrorsCount.Reset() - metrics.updateVaultTokenRenewErrorsCountMetric(errors.VaultTokenNotRenewableErrorType) - metricTokenRenewErrorsCount, _ = tokenRenewErrorsCount.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, errors.VaultTokenNotRenewableErrorType) + tokenRenewalErrorsTotal.Reset() + metrics.updateVaultTokenRenewalErrorsTotalMetric(vaultIsRenewableOperationName, errors.VaultTokenNotRenewableErrorType) + metricTokenRenewalErrorsTotal, _ = tokenRenewalErrorsTotal.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, vaultIsRenewableOperationName, errors.VaultTokenNotRenewableErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) } -func TestUpdateReadSecretErrorsCount(t *testing.T) { +func TestUpdateReadSecretErrorsTotal(t *testing.T) { path := "/path/to/secret" key := "key" metrics := newVaultMetrics(fakeVaultAddress, fakeVaultVersion, fakeVaultEngine, fakeVaultClusterID, fakeVaultClusterName) - secretReadErrorsCount.Reset() - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.UnknownErrorType) - metricSecretReadErrorsCount, _ := secretReadErrorsCount.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, path, key, errors.UnknownErrorType) + secretReadErrorsTotal.Reset() + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.UnknownErrorType) + metricSecretReadErrorsTotal, _ := secretReadErrorsTotal.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, path, key, errors.UnknownErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) - secretReadErrorsCount.Reset() - metrics.updateVaultSecretReadErrorsCountMetric(path, key, errors.BackendSecretNotFoundErrorType) - metricSecretReadErrorsCount, _ = secretReadErrorsCount.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, path, key, errors.BackendSecretNotFoundErrorType) + secretReadErrorsTotal.Reset() + metrics.updateVaultSecretReadErrorsTotalMetric(path, key, errors.BackendSecretNotFoundErrorType) + metricSecretReadErrorsTotal, _ = secretReadErrorsTotal.GetMetricWithLabelValues(fakeVaultAddress, fakeVaultEngine, fakeVaultVersion, fakeVaultClusterID, fakeVaultClusterName, path, key, errors.BackendSecretNotFoundErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) } diff --git a/backend/vault_test.go b/backend/vault_test.go index 6179c7f..2bb238c 100644 --- a/backend/vault_test.go +++ b/backend/vault_test.go @@ -115,30 +115,36 @@ func v1AuthTokenLookupSelf(w http.ResponseWriter, r *http.Request) { func v1AuthTokenRenewSelf(w http.ResponseWriter, r *http.Request) { var response interface{} - - jsonData := fmt.Sprintf(`{ - "request_id": "d8ae3e67-91a0-2f7a-528b-522048f9dad3", - "lease_id": "", - "renewable": false, - "lease_duration": 0, - "data": null, - "wrap_info": null, - "warnings": null, - "auth": { - "client_token": "%s", - "accessor": "dc6aa861-3020-322c-8df5-4b08afa43a34", - "policies": [ - "fake-policy" - ], - "token_policies": [ - "fake-policy" - ], - "metadata": null, - "lease_duration": 1000, - "renewable": true, - "entity_id": "" - } - }`, fakeToken) + jsonData := "" + if !testCfg.tokenRevoked { + jsonData = fmt.Sprintf(` + { + "request_id": "d8ae3e67-91a0-2f7a-528b-522048f9dad3", + "lease_id": "", + "renewable": false, + "lease_duration": 0, + "data": null, + "wrap_info": null, + "warnings": null, + "auth": { + "client_token": "%s", + "accessor": "dc6aa861-3020-322c-8df5-4b08afa43a34", + "policies": [ + "fake-policy" + ], + "token_policies": [ + "fake-policy" + ], + "metadata": null, + "lease_duration": 1000, + "renewable": true, + "entity_id": "" + } + }`, fakeToken) + } else { + jsonData = `{"errors":["permission denied"]}` + w.WriteHeader(http.StatusForbidden) + } if err := json.Unmarshal([]byte(jsonData), &response); err != nil { fmt.Printf("unable to unmarshal json %v", err) w.WriteHeader(http.StatusInternalServerError) @@ -218,9 +224,12 @@ func TestVaultBackend(t *testing.T) { assert.NotNil(t, client) } func TestVaultClient(t *testing.T) { + maxTokenTTL.Reset() client, err := vaultClient(nil, vaultCfg) + metricMaxTokenTTL, _ := maxTokenTTL.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName) assert.Nil(t, err) assert.NotNil(t, client) + assert.Equal(t, float64(client.maxTokenTTL), testutil.ToFloat64(metricMaxTokenTTL)) } func TestVaultClientInvalidCfg(t *testing.T) { @@ -250,35 +259,21 @@ func TestGetTokenTTL(t *testing.T) { assert.Nil(t, err) } -func TestShouldRenewToken(t *testing.T) { +func TestRenewToken(t *testing.T) { client, _ := vaultClient(nil, vaultCfg) mutex.Lock() defer mutex.Unlock() + testCfg.tokenRenewable = true testCfg.tokenTTL = 600 client.maxTokenTTL = 6000 - tokenExpired.Reset() - metricTokenExp, _ := tokenExpired.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName) - assert.True(t, client.shouldRenewToken(int64(testCfg.tokenTTL))) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenExp)) -} - -func TestShouldNotRenewToken(t *testing.T) { - client, _ := vaultClient(nil, vaultCfg) - mutex.Lock() - defer mutex.Unlock() - testCfg.tokenTTL = 600 - client.maxTokenTTL = 60 - - assert.False(t, client.shouldRenewToken(int64(testCfg.tokenTTL))) - - tokenExpired.Reset() - metricTokenExp, _ := tokenExpired.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName) + token, err := client.getToken() + err = client.renewToken(token) - assert.Equal(t, 0.0, testutil.ToFloat64(metricTokenExp)) + assert.Nil(t, err) } -func TestRenewToken(t *testing.T) { +func TestRenewTokenRevokedToken(t *testing.T) { client, _ := vaultClient(nil, vaultCfg) mutex.Lock() defer mutex.Unlock() @@ -287,9 +282,12 @@ func TestRenewToken(t *testing.T) { client.maxTokenTTL = 6000 token, err := client.getToken() + testCfg.tokenRevoked = true + tokenRenewalErrorsTotal.Reset() err = client.renewToken(token) - - assert.Nil(t, err) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, vaultRenewSelfOperationName, errors.UnknownErrorType) + assert.NotNil(t, err) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) } func TestTokenNotRenewableError(t *testing.T) { @@ -302,12 +300,12 @@ func TestTokenNotRenewableError(t *testing.T) { token, err := client.getToken() - tokenRenewErrorsCount.Reset() + tokenRenewalErrorsTotal.Reset() err = client.renewToken(token) - metricTokenRenewErrorsCount, _ := tokenRenewErrorsCount.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, errors.VaultTokenNotRenewableErrorType) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, vaultIsRenewableOperationName, errors.VaultTokenNotRenewableErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) assert.EqualError(t, err, fmt.Sprintf("[%s] vault token not renewable", errors.VaultTokenNotRenewableErrorType)) } @@ -316,11 +314,11 @@ func TestRenewalLoopRevokedToken(t *testing.T) { mutex.Lock() defer mutex.Unlock() testCfg.tokenRevoked = true - tokenLookupErrorsCount.Reset() + tokenRenewalErrorsTotal.Reset() client.renewalLoop() - metricTokenLookupErrorsCount, _ := tokenLookupErrorsCount.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, errors.UnknownErrorType) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, vaultLookupSelfOperationName, errors.UnknownErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenLookupErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) } func TestRenewalLoopNotRenewableToken(t *testing.T) { @@ -332,11 +330,11 @@ func TestRenewalLoopNotRenewableToken(t *testing.T) { testCfg.tokenTTL = 600 client.maxTokenTTL = 6000 - tokenRenewErrorsCount.Reset() + tokenRenewalErrorsTotal.Reset() client.renewalLoop() - metricTokenRenewErrorsCount, _ := tokenRenewErrorsCount.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, errors.VaultTokenNotRenewableErrorType) + metricTokenRenewalErrorsTotal, _ := tokenRenewalErrorsTotal.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, vaultIsRenewableOperationName, errors.VaultTokenNotRenewableErrorType) - assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricTokenRenewalErrorsTotal)) } func TestReadSecretKv2(t *testing.T) { @@ -360,13 +358,13 @@ func TestSecretNotFound(t *testing.T) { client, _ := vaultClient(nil, vaultCfg) path := "/secret/data/test" key := "foo2" - secretReadErrorsCount.Reset() + secretReadErrorsTotal.Reset() secretValue, err := client.ReadSecret(path, key) - metricSecretReadErrorsCount, _ := secretReadErrorsCount.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, path, key, errors.BackendSecretNotFoundErrorType) + metricSecretReadErrorsTotal, _ := secretReadErrorsTotal.GetMetricWithLabelValues(vaultCfg.VaultURL, vaultCfg.VaultEngine, vaultFakeVersion, vaultFakeClusterID, vaultFakeClusterName, path, key, errors.BackendSecretNotFoundErrorType) assert.Empty(t, secretValue) assert.EqualError(t, err, fmt.Sprintf("[%s] secret key %s not found at %s", errors.BackendSecretNotFoundErrorType, key, path)) - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsCount)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) } func TestMain(m *testing.M) { r := mux.NewRouter() From 57ff94f4eb0983fbdc6aada4c36dcf7e7d66ab1b Mon Sep 17 00:00:00 2001 From: Fernando Llaca Date: Tue, 29 Jan 2019 17:10:53 +0100 Subject: [PATCH 6/7] Rename "*_count" metrics to "*_total". As following Prometheus recommended practices --- README.md | 5 ++-- kubernetes/kubernetes.go | 6 ++--- kubernetes/kubernetes_test.go | 30 +++++++++++------------ kubernetes/metrics.go | 16 ++++++------- secrets-manager/metrics.go | 8 +++---- secrets-manager/secrets_manager.go | 6 ++--- secrets-manager/secrets_manager_test.go | 32 ++++++++++++------------- 7 files changed, 52 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 07d5548..7d6ace5 100644 --- a/README.md +++ b/README.md @@ -107,8 +107,9 @@ data: |`secrets_manager_vault_max_token_ttl` | Gauge | `secrets-manager` max Vault token TTL | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"` | |`secrets_manager_vault_token_ttl` | Gauge | Vault token TTL | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name"` | |`secrets_manager_vault_token_renewal_errors_total`| Counter | Vault token renewal errors counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "vault_operation", "error"` | -|`secrets_manager_read_secret_errors_count`| Counter | Vault read operations counter | `"vault_address", "vault_engine", "vault_version", "vault_cluster_id", "vault_cluster_name", "path", "key", "error"` | -| `secrets_manager_secret_sync_errors_count`| Counter |Secrets sync error counter|`"name", "namespace"`| +|`secrets_manager_k8s_secret_read_errors_total`| Counter | Errors total count when reading a secret from Kubernetes | `"name", "namespace"` | +|`secrets_manager_k8s_secret_update_errors_total`| Counter | Error total count when updating (and also creating) a secret in Kubernetes | `"name", "namespace"` | +| `secrets_manager_secret_sync_errors_total`| Counter |Secrets synchronization total errors.|`"name", "namespace"`| |`secrets_manager_secret_last_updated`| Gauge |The last update timestamp as a Unix time (the number of seconds elapsed since January 1, 1970 UTC)|`"name", "namespace"`| ## Getting Started with Vault diff --git a/kubernetes/kubernetes.go b/kubernetes/kubernetes.go index 7713500..099b246 100644 --- a/kubernetes/kubernetes.go +++ b/kubernetes/kubernetes.go @@ -61,7 +61,7 @@ func (k *client) UpsertSecret(secret *Secret) error { _, err = k.client.CoreV1().Secrets(secret.Namespace).Update(k8sSecret) } if err != nil { - secretUpdateErrorCount.WithLabelValues(secret.Name, secret.Namespace).Inc() + secretUpdateErrorsTotal.WithLabelValues(secret.Name, secret.Namespace).Inc() } return err } @@ -72,10 +72,10 @@ func (k *client) ReadSecret(namespace string, name string) (map[string][]byte, e secret, err := k.client.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { - secretReadErrorCount.WithLabelValues(name, namespace).Inc() + secretReadErrorsTotal.WithLabelValues(name, namespace).Inc() return data, &smerrors.K8sSecretNotFoundError{ErrType: smerrors.K8sSecretNotFoundErrorType, Name: name, Namespace: namespace} } - secretReadErrorCount.WithLabelValues(name, namespace).Inc() + secretReadErrorsTotal.WithLabelValues(name, namespace).Inc() return data, err } diff --git a/kubernetes/kubernetes_test.go b/kubernetes/kubernetes_test.go index df1d0ac..9109605 100644 --- a/kubernetes/kubernetes_test.go +++ b/kubernetes/kubernetes_test.go @@ -23,7 +23,7 @@ import ( ) func TestUpsertSecretDoesNotExist(t *testing.T) { - secretUpdateErrorCount.Reset() + secretUpdateErrorsTotal.Reset() client := fake.NewSimpleClientset() k8s := New(client, log.New()) @@ -35,12 +35,12 @@ func TestUpsertSecretDoesNotExist(t *testing.T) { secret, _ := client.CoreV1().Secrets("ns").Get("secret-test", metav1.GetOptions{}) assert.NotNil(t, secret) - metricSecretUpdateErrorCount, _ := secretUpdateErrorCount.GetMetricWithLabelValues("secret-test", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretUpdateErrorCount)) + metricSecretUpdateErrorsTotal, _ := secretUpdateErrorsTotal.GetMetricWithLabelValues("secret-test", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretUpdateErrorsTotal)) } func TestUpsertSecretAlreadyExists(t *testing.T) { - secretUpdateErrorCount.Reset() + secretUpdateErrorsTotal.Reset() // Create the fake client. client := fake.NewSimpleClientset() @@ -60,8 +60,8 @@ func TestUpsertSecretAlreadyExists(t *testing.T) { actions := client.Actions() lastAction := actions[len(actions)-1] assert.Implements(t, (*clientgotesting.UpdateAction)(nil), lastAction, "Last action must be UpdateAction") - metricSecretUpdateErrorCount, _ := secretUpdateErrorCount.GetMetricWithLabelValues("secret-test", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretUpdateErrorCount)) + metricSecretUpdateErrorsTotal, _ := secretUpdateErrorsTotal.GetMetricWithLabelValues("secret-test", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretUpdateErrorsTotal)) } func TestReadConfigMap(t *testing.T) { @@ -91,7 +91,7 @@ func TestReadConfigMap(t *testing.T) { } func TestReadSecret(t *testing.T) { - secretReadErrorCount.Reset() + secretReadErrorsTotal.Reset() client := fake.NewSimpleClientset() k8s := New(client, log.New()) @@ -117,12 +117,12 @@ func TestReadSecret(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, secret) assert.Equal(t, "some-value", string(secret["some-key"])) - metricSecretReadErrorCount, _ := secretUpdateErrorCount.GetMetricWithLabelValues("secret-test", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretReadErrorCount)) + metricSecretReadErrorsTotal, _ := secretUpdateErrorsTotal.GetMetricWithLabelValues("secret-test", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) } func TestReadSecretNotFound(t *testing.T) { - secretReadErrorCount.Reset() + secretReadErrorsTotal.Reset() client := fake.NewSimpleClientset() k8s := New(client, log.New()) @@ -130,12 +130,12 @@ func TestReadSecretNotFound(t *testing.T) { secret, err := k8s.ReadSecret("ns", "secret-test") assert.EqualError(t, err, fmt.Sprintf("[%s] secret '%s/%s' not found", secretsManagerErrors.K8sSecretNotFoundErrorType, "ns", "secret-test")) assert.Empty(t, secret) - metricSecretReadErrorCount, _ := secretUpdateErrorCount.GetMetricWithLabelValues("secret-test", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretReadErrorCount)) + metricSecretReadErrorsTotal, _ := secretUpdateErrorsTotal.GetMetricWithLabelValues("secret-test", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) } func TestReadSecretError(t *testing.T) { - secretReadErrorCount.Reset() + secretReadErrorsTotal.Reset() client := fake.NewSimpleClientset() client.CoreV1().(*fakecorev1.FakeCoreV1).PrependReactor("*", "*", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { @@ -146,6 +146,6 @@ func TestReadSecretError(t *testing.T) { secret, err := k8s.ReadSecret("ns", "secret-test") assert.Empty(t, secret) assert.NotNil(t, err) - metricSecretReadErrorCount, _ := secretReadErrorCount.GetMetricWithLabelValues("secret-test", "ns") - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorCount)) + metricSecretReadErrorsTotal, _ := secretReadErrorsTotal.GetMetricWithLabelValues("secret-test", "ns") + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretReadErrorsTotal)) } diff --git a/kubernetes/metrics.go b/kubernetes/metrics.go index 02e924c..727affe 100644 --- a/kubernetes/metrics.go +++ b/kubernetes/metrics.go @@ -6,21 +6,21 @@ import ( var ( // Prometeheus metrics: https://prometheus.io - secretReadErrorCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + secretReadErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "secrets_manager", Subsystem: "k8s", - Name: "secret_read_error_count", - Help: "Errors count when reading a secret from Kubernetes", + Name: "secret_read_errors_total", + Help: "Errors total count when reading a secret from Kubernetes", }, []string{"name", "namespace"}) - secretUpdateErrorCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + secretUpdateErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "secrets_manager", Subsystem: "k8s", - Name: "secret_update_error_count", - Help: "Error count when updating (and also creating) a secret in Kubernetes", + Name: "secret_update_errors_total", + Help: "Error total count when updating (and also creating) a secret in Kubernetes", }, []string{"name", "namespace"}) ) func init() { - prometheus.MustRegister(secretReadErrorCount) - prometheus.MustRegister(secretUpdateErrorCount) + prometheus.MustRegister(secretReadErrorsTotal) + prometheus.MustRegister(secretUpdateErrorsTotal) } diff --git a/secrets-manager/metrics.go b/secrets-manager/metrics.go index cea1201..2937988 100644 --- a/secrets-manager/metrics.go +++ b/secrets-manager/metrics.go @@ -6,11 +6,11 @@ import ( var ( // Prometeheus metrics: https://prometheus.io - secretSyncErrorsCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + secretSyncErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "secrets_manager", Subsystem: "secret", - Name: "sync_errors_count", - Help: "Secrets sync error counter", + Name: "sync_errors_total", + Help: "Secrets synchronization total errors.", }, []string{"name", "namespace"}) secretLastUpdated = prometheus.NewGaugeVec(prometheus.GaugeOpts{ @@ -22,6 +22,6 @@ var ( ) func init() { - prometheus.MustRegister(secretSyncErrorsCount) + prometheus.MustRegister(secretSyncErrorsTotal) prometheus.MustRegister(secretLastUpdated) } diff --git a/secrets-manager/secrets_manager.go b/secrets-manager/secrets_manager.go index 10bd9fc..c8f69bf 100644 --- a/secrets-manager/secrets_manager.go +++ b/secrets-manager/secrets_manager.go @@ -128,7 +128,7 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { if err != nil { logger.Errorf("unable to get desired state for secret '%s' : %v", secret.Name, err) for _, namespace := range secret.Namespaces { - secretSyncErrorsCount.WithLabelValues(secret.Name, namespace).Inc() + secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() } return err } @@ -136,7 +136,7 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { currentState, err := s.getCurrentState(namespace, secret.Name) if err != nil && !errors.IsK8sSecretNotFound(err) { logger.Errorf("unable to get current state of secret '%s/%s' : %v", namespace, secret.Name, err) - secretSyncErrorsCount.WithLabelValues(secret.Name, namespace).Inc() + secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() // If we fail to read from Kubernetes, we keep trying with another namespace continue } @@ -145,7 +145,7 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { logger.Infof("secret '%s/%s' must be updated", namespace, secret.Name) if err := s.upsertSecret(secret.Type, namespace, secret.Name, desiredState); err != nil { log.Errorf("unable to upsert secret %s/%s: %v", namespace, secret.Name, err) - secretSyncErrorsCount.WithLabelValues(secret.Name, namespace).Inc() + secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() continue } logger.Infof("secret '%s/%s' updated", namespace, secret.Name) diff --git a/secrets-manager/secrets_manager_test.go b/secrets-manager/secrets_manager_test.go index 522d390..849f5f4 100644 --- a/secrets-manager/secrets_manager_test.go +++ b/secrets-manager/secrets_manager_test.go @@ -279,7 +279,7 @@ func TestUpsertSecretError(t *testing.T) { } func TestSyncState(t *testing.T) { - secretSyncErrorsCount.Reset() + secretSyncErrorsTotal.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -321,12 +321,12 @@ func TestSyncState(t *testing.T) { assert.Nil(t, err) // Test Prometheus metric - metricSecretSyncErrorsCount, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsCount)) + metricSecretSyncErrorsTotal, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal)) } func TestSyncStateErrorGetDesired(t *testing.T) { - secretSyncErrorsCount.Reset() + secretSyncErrorsTotal.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -351,12 +351,12 @@ func TestSyncStateErrorGetDesired(t *testing.T) { assert.NotNil(t, err) // Test Prometheus metric - metricSecretSyncErrorsCount, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns") - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsCount)) + metricSecretSyncErrorsTotal, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns") + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsTotal)) } func TestSyncStateErrorGetCurrentInOneSecret(t *testing.T) { - secretSyncErrorsCount.Reset() + secretSyncErrorsTotal.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -409,18 +409,18 @@ func TestSyncStateErrorGetCurrentInOneSecret(t *testing.T) { assert.Nil(t, err) // Test Prometheus metric - metricSecretSyncErrorsCount1, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns1") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsCount1)) + metricSecretSyncErrorsTotal1, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns1") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal1)) // Test Prometheus metric - metricSecretSyncErrorsCount2, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns2") - assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsCount2)) + metricSecretSyncErrorsTotal2, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns2") + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsTotal2)) // Test Prometheus metric - metricSecretSyncErrorsCount3, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns3") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsCount3)) + metricSecretSyncErrorsTotal3, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns3") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal3)) } func TestSyncStateErrorUpsertSecret(t *testing.T) { - secretSyncErrorsCount.Reset() + secretSyncErrorsTotal.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -462,8 +462,8 @@ func TestSyncStateErrorUpsertSecret(t *testing.T) { assert.Nil(t, err) // Test Prometheus metric - metricSecretSyncErrorsCount, _ := secretSyncErrorsCount.GetMetricWithLabelValues("secret-name", "ns") - assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsCount)) + metricSecretSyncErrorsTotal, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns") + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal)) } func TestLoadConfig(t *testing.T) { From 815f28a07219feca9cd9d4832b1551fbeafc695f Mon Sep 17 00:00:00 2001 From: Fernando Llaca Date: Thu, 31 Jan 2019 13:15:55 +0100 Subject: [PATCH 7/7] Add "secrets_manager_secret_last_sync_status" metric. --- CHANGELOG.md | 1 + README.md | 1 + secrets-manager/metrics.go | 8 ++++++++ secrets-manager/secrets_manager.go | 4 ++++ secrets-manager/secrets_manager_test.go | 13 +++++++++++++ 5 files changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6745c5d..16a5d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Added - New `secrets_manager_vault_max_token_ttl` metric, so a user could alert based on this and `secrets_manager_token_ttl` +- New `secrets_manager_secret_last_sync_status` metric, that shows wether the secret succeded or not in last synchronization iteration ### Fixed - Deprecates `secrets_manager_vault_token_expired` metric as it was quite confusing since it's not really possible for `secrets-manager` to know when the token it's expired, just when it's "close to expire". diff --git a/README.md b/README.md index 7d6ace5..482e97a 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,7 @@ data: |`secrets_manager_k8s_secret_update_errors_total`| Counter | Error total count when updating (and also creating) a secret in Kubernetes | `"name", "namespace"` | | `secrets_manager_secret_sync_errors_total`| Counter |Secrets synchronization total errors.|`"name", "namespace"`| |`secrets_manager_secret_last_updated`| Gauge |The last update timestamp as a Unix time (the number of seconds elapsed since January 1, 1970 UTC)|`"name", "namespace"`| +|`secrets_manager_secret_last_sync_status`| Gauge |The result of the last sync of a secret. 1 = OK, 0 = Error|`"name", "namespace"`| ## Getting Started with Vault diff --git a/secrets-manager/metrics.go b/secrets-manager/metrics.go index 2937988..33ee317 100644 --- a/secrets-manager/metrics.go +++ b/secrets-manager/metrics.go @@ -19,9 +19,17 @@ var ( Name: "last_updated", Help: "The last update timestamp as a Unix time (the number of seconds elapsed since January 1, 1970 UTC)", }, []string{"name", "namespace"}) + + secretLastSyncStatus = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: "secrets_manager", + Subsystem: "secret", + Name: "last_sync_status", + Help: "The result of the last sync of a secret. 1 = OK, 0 = Error", + }, []string{"name", "namespace"}) ) func init() { prometheus.MustRegister(secretSyncErrorsTotal) prometheus.MustRegister(secretLastUpdated) + prometheus.MustRegister(secretLastSyncStatus) } diff --git a/secrets-manager/secrets_manager.go b/secrets-manager/secrets_manager.go index c8f69bf..60f27d3 100644 --- a/secrets-manager/secrets_manager.go +++ b/secrets-manager/secrets_manager.go @@ -129,6 +129,7 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { logger.Errorf("unable to get desired state for secret '%s' : %v", secret.Name, err) for _, namespace := range secret.Namespaces { secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() + secretLastSyncStatus.WithLabelValues(secret.Name, namespace).Set(0.0) } return err } @@ -137,6 +138,7 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { if err != nil && !errors.IsK8sSecretNotFound(err) { logger.Errorf("unable to get current state of secret '%s/%s' : %v", namespace, secret.Name, err) secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() + secretLastSyncStatus.WithLabelValues(secret.Name, namespace).Set(0.0) // If we fail to read from Kubernetes, we keep trying with another namespace continue } @@ -146,9 +148,11 @@ func (s *SecretManager) syncState(secret SecretDefinition) error { if err := s.upsertSecret(secret.Type, namespace, secret.Name, desiredState); err != nil { log.Errorf("unable to upsert secret %s/%s: %v", namespace, secret.Name, err) secretSyncErrorsTotal.WithLabelValues(secret.Name, namespace).Inc() + secretLastSyncStatus.WithLabelValues(secret.Name, namespace).Set(0.0) continue } logger.Infof("secret '%s/%s' updated", namespace, secret.Name) + secretLastSyncStatus.WithLabelValues(secret.Name, namespace).Set(1.0) } } return nil diff --git a/secrets-manager/secrets_manager_test.go b/secrets-manager/secrets_manager_test.go index 849f5f4..762bf13 100644 --- a/secrets-manager/secrets_manager_test.go +++ b/secrets-manager/secrets_manager_test.go @@ -280,6 +280,7 @@ func TestUpsertSecretError(t *testing.T) { func TestSyncState(t *testing.T) { secretSyncErrorsTotal.Reset() + secretLastSyncStatus.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -322,11 +323,14 @@ func TestSyncState(t *testing.T) { assert.Nil(t, err) // Test Prometheus metric metricSecretSyncErrorsTotal, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns") + metricSecretLastSyncStatus, _ := secretLastSyncStatus.GetMetricWithLabelValues("secret-name", "ns") assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretLastSyncStatus)) } func TestSyncStateErrorGetDesired(t *testing.T) { secretSyncErrorsTotal.Reset() + secretLastSyncStatus.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -352,11 +356,14 @@ func TestSyncStateErrorGetDesired(t *testing.T) { assert.NotNil(t, err) // Test Prometheus metric metricSecretSyncErrorsTotal, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns") + metricSecretLastSyncStatus, _ := secretLastSyncStatus.GetMetricWithLabelValues("secret-name", "ns") assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsTotal)) + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretLastSyncStatus)) } func TestSyncStateErrorGetCurrentInOneSecret(t *testing.T) { secretSyncErrorsTotal.Reset() + secretLastSyncStatus.Reset() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() k8s := mocks.NewMockKubernetesClient(mockCtrl) @@ -410,13 +417,19 @@ func TestSyncStateErrorGetCurrentInOneSecret(t *testing.T) { assert.Nil(t, err) // Test Prometheus metric metricSecretSyncErrorsTotal1, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns1") + metricSecretLastSyncStatus1, _ := secretLastSyncStatus.GetMetricWithLabelValues("secret-name", "ns1") assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal1)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretLastSyncStatus1)) // Test Prometheus metric metricSecretSyncErrorsTotal2, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns2") + metricSecretLastSyncStatus2, _ := secretLastSyncStatus.GetMetricWithLabelValues("secret-name", "ns2") assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretSyncErrorsTotal2)) + assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretLastSyncStatus2)) // Test Prometheus metric metricSecretSyncErrorsTotal3, _ := secretSyncErrorsTotal.GetMetricWithLabelValues("secret-name", "ns3") + metricSecretLastSyncStatus3, _ := secretLastSyncStatus.GetMetricWithLabelValues("secret-name", "ns3") assert.Equal(t, 0.0, testutil.ToFloat64(metricSecretSyncErrorsTotal3)) + assert.Equal(t, 1.0, testutil.ToFloat64(metricSecretLastSyncStatus3)) } func TestSyncStateErrorUpsertSecret(t *testing.T) {