From 4b32da4eff5e4fa1270ed6861291067f53965f84 Mon Sep 17 00:00:00 2001 From: Bianca Moreira Date: Tue, 6 Jun 2023 14:57:07 +0200 Subject: [PATCH 1/3] Add new policy metrics --- vault/core_metrics.go | 58 ++++++++++++++++++++++++++ vault/core_metrics_test.go | 84 ++++++++++++++++++++++++++++++++++++++ vault/policy_store.go | 55 +++++++++++++++++++++++++ vault/policy_store_test.go | 34 +++++++++++++++ 4 files changed, 231 insertions(+) diff --git a/vault/core_metrics.go b/vault/core_metrics.go index 1b695c4f1d82..b6185be58fc3 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -301,6 +301,12 @@ func (c *Core) emitMetricsActiveNode(stopCh chan struct{}) { c.activeEntityGaugeCollector, "", }, + { + []string{"policy", "available", "count"}, + []metrics.Label{{"gauge", "number_policies_by_type"}}, + c.availablePoliciesGaugeCollector, + "", + }, } // Disable collection if configured, or if we're a performance standby @@ -565,3 +571,55 @@ func (c *Core) inFlightReqGaugeMetric() { // Adding a gauge metric to capture total number of inflight requests c.metricSink.SetGaugeWithLabels([]string{"core", "in_flight_requests"}, float32(totalInFlightReq), nil) } + +func (c *Core) availablePoliciesGaugeCollector(ctx context.Context) ([]metricsutil.GaugeLabelValues, error) { + if c.policyStore == nil { + return []metricsutil.GaugeLabelValues{}, nil + } + + c.stateLock.RLock() + policyStore := c.policyStore + c.stateLock.RUnlock() + + ctx = namespace.RootContext(ctx) + namespaces := c.collectNamespaces() + + policyTypes := []PolicyType{ + PolicyTypeACL, + PolicyTypeRGP, + PolicyTypeEGP, + } + var values []metricsutil.GaugeLabelValues + + filterDefaultPolicies := func(policies []string) []string { + var p []string + for _, s := range policies { + if s != "default" { + p = append(p, s) + } + } + return p + } + + for _, pt := range policyTypes { + policies, err := policyStore.ListPoliciesForNamespaces(ctx, pt, namespaces) + if err != nil { + return []metricsutil.GaugeLabelValues{}, err + } + + // Filter the "default" policies + policies = filterDefaultPolicies(policies) + + if len(policies) > 0 { + v := metricsutil.GaugeLabelValues{} + v.Labels = []metricsutil.Label{{ + "policy_type", + pt.String(), + }} + v.Value = float32(len(policies)) + values = append(values, v) + } + } + + return values, nil +} diff --git a/vault/core_metrics_test.go b/vault/core_metrics_test.go index 07147ad3f471..cab8f54941f4 100644 --- a/vault/core_metrics_test.go +++ b/vault/core_metrics_test.go @@ -4,12 +4,16 @@ package vault import ( + "context" + "encoding/base64" "errors" "sort" "strings" "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/armon/go-metrics" logicalKv "github.com/hashicorp/vault-plugin-secrets-kv" "github.com/hashicorp/vault/helper/namespace" @@ -350,3 +354,83 @@ func TestCoreMetrics_EntityGauges(t *testing.T) { "mount_point": "auth/userpass/", }) } + +func TestCoreMetrics_AvailablePolicies(t *testing.T) { + aclPolicy := map[string]interface{}{ + "policy": base64.StdEncoding.EncodeToString([]byte(`path "ns1/secret/foo/*" { + capabilities = ["create", "read", "update", "delete", "list"] +}`)), + "name": "secret", + } + + type pathPolicy struct { + Path string + Policy map[string]interface{} + } + + tests := map[string]struct { + Policies []pathPolicy + ExpectedValues map[string]float32 + }{ + "single acl": { + Policies: []pathPolicy{ + { + "sys/policy/secret", aclPolicy, + }, + }, + ExpectedValues: map[string]float32{ + "acl": 1, + }, + }, + "multiple acl": { + Policies: []pathPolicy{ + { + "sys/policy/secret", aclPolicy, + }, + { + "sys/policy/secret2", aclPolicy, + }, + }, + ExpectedValues: map[string]float32{ + "acl": 2, + }, + }, + } + + for name, tst := range tests { + t.Run(name, func(t *testing.T) { + core, _, root := TestCoreUnsealed(t) + + ctxRoot := namespace.RootContext(context.Background()) + + // Create policies + for _, p := range tst.Policies { + req := logical.TestRequest(t, logical.UpdateOperation, p.Path) + req.Data = p.Policy + req.ClientToken = root + + resp, err := core.HandleRequest(ctxRoot, req) + if err != nil { + t.Fatalf("err: %v", err) + } + if resp != nil { + logger.Info("expected nil response", resp) + t.Fatalf("expected nil response") + } + } + + gValues, err := core.availablePoliciesGaugeCollector(ctxRoot) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Check the metrics values match the expected values + mgValues := make(map[string]float32, len(gValues)) + for _, v := range gValues { + mgValues[v.Labels[0].Value] = v.Value + } + + assert.EqualValues(t, tst.ExpectedValues, mgValues) + }) + } +} diff --git a/vault/policy_store.go b/vault/policy_store.go index edde91dcc2bf..085a8daafc95 100644 --- a/vault/policy_store.go +++ b/vault/policy_store.go @@ -649,6 +649,61 @@ func (ps *PolicyStore) ListPolicies(ctx context.Context, policyType PolicyType) return keys, err } +// ListPoliciesForNamespace is used to list the available policies for the given namespace +func (ps *PolicyStore) listPoliciesForNamespace(ctx context.Context, policyType PolicyType, ns *namespace.Namespace) ([]string, error) { + var err error + var keys []string + + // Get the appropriate view based on policy type and namespace + ctx = namespace.ContextWithNamespace(ctx, ns) + + // Scan the view, since the policy names are the same as the + // key names. + switch policyType { + case PolicyTypeACL: + view := ps.getACLView(ns) + if view == nil { + return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) + } + keys, err = logical.CollectKeys(ctx, view) + case PolicyTypeRGP: + view := ps.getRGPView(ns) + if view == nil { + return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) + } + return logical.CollectKeys(ctx, view) + case PolicyTypeEGP: + view := ps.getEGPView(ns) + if view == nil { + return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) + } + return logical.CollectKeys(ctx, view) + default: + return nil, fmt.Errorf("unknown policy type %q", policyType) + } + + // We only have non-assignable ACL policies at the moment + keys = strutil.Difference(keys, nonAssignablePolicies, false) + + return keys, err +} + +// ListPoliciesForNamespaces is used to list the available policies for the given namespaces +func (ps *PolicyStore) ListPoliciesForNamespaces(ctx context.Context, policyType PolicyType, ns []*namespace.Namespace) ([]string, error) { + var err error + var keys []string + + for _, nspace := range ns { + ks, err := ps.listPoliciesForNamespace(ctx, policyType, nspace) + if err != nil { + return []string{}, err + } + keys = append(keys, ks...) + } + + return keys, err +} + // DeletePolicy is used to delete the named policy func (ps *PolicyStore) DeletePolicy(ctx context.Context, name string, policyType PolicyType) error { return ps.switchedDeletePolicy(ctx, name, policyType, true, false) diff --git a/vault/policy_store_test.go b/vault/policy_store_test.go index 624f2806783f..605d38472092 100644 --- a/vault/policy_store_test.go +++ b/vault/policy_store_test.go @@ -319,3 +319,37 @@ func TestDefaultPolicy(t *testing.T) { }) } } + +// TestPolicyStore_ListPoliciesForNamespaces tests the ListPoliciesForNamespaces function, which should return a list of policies for a given list of namespaces. +func TestPolicyStore_ListPoliciesForNamespaces(t *testing.T) { + _, ps := mockPolicyWithCore(t, false) + + ctxRoot := namespace.RootContext(context.Background()) + rootNs := namespace.RootNamespace + + parsedPolicy, _ := ParseACLPolicy(rootNs, aclPolicy) + + err := ps.SetPolicy(ctxRoot, parsedPolicy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Get should work + pResult, err := ps.GetPolicy(ctxRoot, "dev", PolicyTypeACL) + if err != nil { + t.Fatalf("err: %v", err) + } + if !reflect.DeepEqual(pResult, parsedPolicy) { + t.Fatalf("bad: %v", pResult) + } + + out, err := ps.ListPoliciesForNamespaces(ctxRoot, PolicyTypeACL, []*namespace.Namespace{rootNs}) + if err != nil { + t.Fatalf("err: %v", err) + } + + expectedResult := []string{"default", "dev"} + if !reflect.DeepEqual(expectedResult, out) { + t.Fatalf("expected: %v\ngot: %v", expectedResult, out) + } +} From 8365fa444b7f605f9855c8ffc77640a68c7f8928 Mon Sep 17 00:00:00 2001 From: Bianca Moreira Date: Tue, 6 Jun 2023 15:54:26 +0200 Subject: [PATCH 2/3] Add changelog entry --- changelog/21010.txt | 3 +++ vault/core_metrics.go | 29 +++++++---------------------- vault/core_metrics_test.go | 11 +++++++++-- 3 files changed, 19 insertions(+), 24 deletions(-) create mode 100644 changelog/21010.txt diff --git a/changelog/21010.txt b/changelog/21010.txt new file mode 100644 index 000000000000..405e6e269072 --- /dev/null +++ b/changelog/21010.txt @@ -0,0 +1,3 @@ +```release-note:improvement +core: Add a new periodic metric to track the number of available policies, `vault.policy.available.count`. +``` \ No newline at end of file diff --git a/vault/core_metrics.go b/vault/core_metrics.go index b6185be58fc3..1b83cd8c8f6a 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -591,34 +591,19 @@ func (c *Core) availablePoliciesGaugeCollector(ctx context.Context) ([]metricsut } var values []metricsutil.GaugeLabelValues - filterDefaultPolicies := func(policies []string) []string { - var p []string - for _, s := range policies { - if s != "default" { - p = append(p, s) - } - } - return p - } - for _, pt := range policyTypes { policies, err := policyStore.ListPoliciesForNamespaces(ctx, pt, namespaces) if err != nil { return []metricsutil.GaugeLabelValues{}, err } - // Filter the "default" policies - policies = filterDefaultPolicies(policies) - - if len(policies) > 0 { - v := metricsutil.GaugeLabelValues{} - v.Labels = []metricsutil.Label{{ - "policy_type", - pt.String(), - }} - v.Value = float32(len(policies)) - values = append(values, v) - } + v := metricsutil.GaugeLabelValues{} + v.Labels = []metricsutil.Label{{ + "policy_type", + pt.String(), + }} + v.Value = float32(len(policies)) + values = append(values, v) } return values, nil diff --git a/vault/core_metrics_test.go b/vault/core_metrics_test.go index cab8f54941f4..79ca7dac942a 100644 --- a/vault/core_metrics_test.go +++ b/vault/core_metrics_test.go @@ -355,6 +355,7 @@ func TestCoreMetrics_EntityGauges(t *testing.T) { }) } +// TestCoreMetrics_AvailablePolicies tests the that available metrics are getting correctly collected when the availablePoliciesGaugeCollector function is invoked func TestCoreMetrics_AvailablePolicies(t *testing.T) { aclPolicy := map[string]interface{}{ "policy": base64.StdEncoding.EncodeToString([]byte(`path "ns1/secret/foo/*" { @@ -379,7 +380,10 @@ func TestCoreMetrics_AvailablePolicies(t *testing.T) { }, }, ExpectedValues: map[string]float32{ - "acl": 1, + // The "default" policy will always be included + "acl": 2, + "egp": 0, + "rgp": 0, }, }, "multiple acl": { @@ -392,7 +396,10 @@ func TestCoreMetrics_AvailablePolicies(t *testing.T) { }, }, ExpectedValues: map[string]float32{ - "acl": 2, + // The "default" policy will always be included + "acl": 3, + "egp": 0, + "rgp": 0, }, }, } From c0b794830b1176f8c96a637701ad5f3e7b313603 Mon Sep 17 00:00:00 2001 From: Bianca Moreira Date: Wed, 7 Jun 2023 11:50:06 +0200 Subject: [PATCH 3/3] Change policy wording --- changelog/21010.txt | 2 +- vault/core_metrics.go | 9 ++++--- vault/core_metrics_test.go | 2 +- vault/policy_store.go | 51 +++++++++++++++++++------------------- vault/policy_store_test.go | 6 ++--- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/changelog/21010.txt b/changelog/21010.txt index 405e6e269072..bcd218794df9 100644 --- a/changelog/21010.txt +++ b/changelog/21010.txt @@ -1,3 +1,3 @@ ```release-note:improvement -core: Add a new periodic metric to track the number of available policies, `vault.policy.available.count`. +core: Add a new periodic metric to track the number of available policies, `vault.policy.configured.count`. ``` \ No newline at end of file diff --git a/vault/core_metrics.go b/vault/core_metrics.go index 1b83cd8c8f6a..18086bc3b254 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -302,9 +302,9 @@ func (c *Core) emitMetricsActiveNode(stopCh chan struct{}) { "", }, { - []string{"policy", "available", "count"}, + []string{"policy", "configured", "count"}, []metrics.Label{{"gauge", "number_policies_by_type"}}, - c.availablePoliciesGaugeCollector, + c.configuredPoliciesGaugeCollector, "", }, } @@ -572,7 +572,8 @@ func (c *Core) inFlightReqGaugeMetric() { c.metricSink.SetGaugeWithLabels([]string{"core", "in_flight_requests"}, float32(totalInFlightReq), nil) } -func (c *Core) availablePoliciesGaugeCollector(ctx context.Context) ([]metricsutil.GaugeLabelValues, error) { +// configuredPoliciesGaugeCollector is used to collect gauge label values for the `vault.policy.configured.count` metric +func (c *Core) configuredPoliciesGaugeCollector(ctx context.Context) ([]metricsutil.GaugeLabelValues, error) { if c.policyStore == nil { return []metricsutil.GaugeLabelValues{}, nil } @@ -592,7 +593,7 @@ func (c *Core) availablePoliciesGaugeCollector(ctx context.Context) ([]metricsut var values []metricsutil.GaugeLabelValues for _, pt := range policyTypes { - policies, err := policyStore.ListPoliciesForNamespaces(ctx, pt, namespaces) + policies, err := policyStore.policiesByNamespaces(ctx, pt, namespaces) if err != nil { return []metricsutil.GaugeLabelValues{}, err } diff --git a/vault/core_metrics_test.go b/vault/core_metrics_test.go index 79ca7dac942a..7e31ab9d0baf 100644 --- a/vault/core_metrics_test.go +++ b/vault/core_metrics_test.go @@ -426,7 +426,7 @@ func TestCoreMetrics_AvailablePolicies(t *testing.T) { } } - gValues, err := core.availablePoliciesGaugeCollector(ctxRoot) + gValues, err := core.configuredPoliciesGaugeCollector(ctxRoot) if err != nil { t.Fatalf("err: %v", err) } diff --git a/vault/policy_store.go b/vault/policy_store.go index 085a8daafc95..e4c12e7b0f36 100644 --- a/vault/policy_store.go +++ b/vault/policy_store.go @@ -649,54 +649,53 @@ func (ps *PolicyStore) ListPolicies(ctx context.Context, policyType PolicyType) return keys, err } -// ListPoliciesForNamespace is used to list the available policies for the given namespace -func (ps *PolicyStore) listPoliciesForNamespace(ctx context.Context, policyType PolicyType, ns *namespace.Namespace) ([]string, error) { +// policiesByNamespace is used to list the available policies for the given namespace +func (ps *PolicyStore) policiesByNamespace(ctx context.Context, policyType PolicyType, ns *namespace.Namespace) ([]string, error) { var err error var keys []string - - // Get the appropriate view based on policy type and namespace - ctx = namespace.ContextWithNamespace(ctx, ns) + var view *BarrierView // Scan the view, since the policy names are the same as the // key names. switch policyType { case PolicyTypeACL: - view := ps.getACLView(ns) - if view == nil { - return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) - } - keys, err = logical.CollectKeys(ctx, view) + view = ps.getACLView(ns) case PolicyTypeRGP: - view := ps.getRGPView(ns) - if view == nil { - return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) - } - return logical.CollectKeys(ctx, view) + view = ps.getRGPView(ns) case PolicyTypeEGP: - view := ps.getEGPView(ns) - if view == nil { - return []string{}, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) - } - return logical.CollectKeys(ctx, view) + view = ps.getEGPView(ns) default: return nil, fmt.Errorf("unknown policy type %q", policyType) } - // We only have non-assignable ACL policies at the moment - keys = strutil.Difference(keys, nonAssignablePolicies, false) + if view == nil { + return nil, fmt.Errorf("unable to get the barrier subview for policy type %q", policyType) + } + + // Get the appropriate view based on policy type and namespace + ctx = namespace.ContextWithNamespace(ctx, ns) + keys, err = logical.CollectKeys(ctx, view) + if err != nil { + return nil, err + } + + if policyType == PolicyTypeACL { + // We only have non-assignable ACL policies at the moment + keys = strutil.Difference(keys, nonAssignablePolicies, false) + } return keys, err } -// ListPoliciesForNamespaces is used to list the available policies for the given namespaces -func (ps *PolicyStore) ListPoliciesForNamespaces(ctx context.Context, policyType PolicyType, ns []*namespace.Namespace) ([]string, error) { +// policiesByNamespaces is used to list the available policies for the given namespaces +func (ps *PolicyStore) policiesByNamespaces(ctx context.Context, policyType PolicyType, ns []*namespace.Namespace) ([]string, error) { var err error var keys []string for _, nspace := range ns { - ks, err := ps.listPoliciesForNamespace(ctx, policyType, nspace) + ks, err := ps.policiesByNamespace(ctx, policyType, nspace) if err != nil { - return []string{}, err + return nil, err } keys = append(keys, ks...) } diff --git a/vault/policy_store_test.go b/vault/policy_store_test.go index 605d38472092..59e005d61270 100644 --- a/vault/policy_store_test.go +++ b/vault/policy_store_test.go @@ -320,8 +320,8 @@ func TestDefaultPolicy(t *testing.T) { } } -// TestPolicyStore_ListPoliciesForNamespaces tests the ListPoliciesForNamespaces function, which should return a list of policies for a given list of namespaces. -func TestPolicyStore_ListPoliciesForNamespaces(t *testing.T) { +// TestPolicyStore_PoliciesByNamespaces tests the policiesByNamespaces function, which should return a slice of policy names for a given slice of namespaces. +func TestPolicyStore_PoliciesByNamespaces(t *testing.T) { _, ps := mockPolicyWithCore(t, false) ctxRoot := namespace.RootContext(context.Background()) @@ -343,7 +343,7 @@ func TestPolicyStore_ListPoliciesForNamespaces(t *testing.T) { t.Fatalf("bad: %v", pResult) } - out, err := ps.ListPoliciesForNamespaces(ctxRoot, PolicyTypeACL, []*namespace.Namespace{rootNs}) + out, err := ps.policiesByNamespaces(ctxRoot, PolicyTypeACL, []*namespace.Namespace{rootNs}) if err != nil { t.Fatalf("err: %v", err) }