From 457c02deaa8212e7121e3ef958d4e767b32029b3 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Thu, 18 May 2023 14:29:01 -0400 Subject: [PATCH 1/3] Fix tidy with maintain_stored_certificate_counts == publish_stored_certificate_count_metrics == false The logic around the check to set both to false was wrong, and should be validated independently. Additionally, these fields should only exist on auto-tidy and not on the manual tidy endpoint. Signed-off-by: Alexander Scheel --- builtin/logical/pki/fields.go | 17 ----------------- builtin/logical/pki/path_tidy.go | 22 +++++++++++++++++++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 47df98ac9cdd..c7e85f00e8b8 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -547,23 +547,6 @@ greater period of time. By default this is zero seconds.`, Default: "0s", } - fields["maintain_stored_certificate_counts"] = &framework.FieldSchema{ - Type: framework.TypeBool, - Description: `This configures whether stored certificates -are counted upon initialization of the backend, and whether during -normal operation, a running count of certificates stored is maintained.`, - Default: false, - } - - fields["publish_stored_certificate_count_metrics"] = &framework.FieldSchema{ - Type: framework.TypeBool, - Description: `This configures whether the stored certificate -count is published to the metrics consumer. It does not affect if the -stored certificate count is maintained, and if maintained, it will be -available on the tidy-status endpoint.`, - Default: false, - } - fields["tidy_revocation_queue"] = &framework.FieldSchema{ Type: framework.TypeBool, Description: `Set to true to remove stale revocation queue entries diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index e7ed2ef46612..c8a19da166f0 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -508,6 +508,21 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Description: `Interval at which to run an auto-tidy operation. This is the time between tidy invocations (after one finishes to the start of the next). Running a manual tidy will reset this duration.`, Default: int(defaultTidyConfig.Interval / time.Second), // TypeDurationSecond currently requires the default to be an int. }, + "maintain_stored_certificate_counts": { + Type: framework.TypeBool, + Description: `This configures whether stored certificates +are counted upon initialization of the backend, and whether during +normal operation, a running count of certificates stored is maintained.`, + Default: false, + }, + "publish_stored_certificate_count_metrics": { + Type: framework.TypeBool, + Description: `This configures whether the stored certificate +count is published to the metrics consumer. It does not affect if the +stored certificate count is maintained, and if maintained, it will be +available on the tidy-status endpoint.`, + Default: false, + }, }), Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ @@ -1774,12 +1789,13 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ } if runningStorageMetricsEnabledRaw, ok := d.GetOk("publish_stored_certificate_count_metrics"); ok { - if config.MaintainCount == false { - return logical.ErrorResponse("Can not publish a running storage metrics count to metrics without first maintaining that count. Enable `maintain_stored_certificate_counts` to enable `publish_stored_certificate_count_metrics."), nil - } config.PublishMetrics = runningStorageMetricsEnabledRaw.(bool) } + if config.PublishMetrics && !config.MaintainCount { + return logical.ErrorResponse("Can not publish a running storage metrics count to metrics without first maintaining that count. Enable `maintain_stored_certificate_counts` to enable `publish_stored_certificate_count_metrics."), nil + } + if err := sc.writeAutoTidyConfig(config); err != nil { return nil, err } From 01e5ba1d6ed1c52d00f48b3301c71519cfedf89f Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Fri, 19 May 2023 07:38:27 -0400 Subject: [PATCH 2/3] Update builtin/logical/pki/path_tidy.go Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> --- builtin/logical/pki/path_tidy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index c8a19da166f0..11662d554699 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -1793,7 +1793,7 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ } if config.PublishMetrics && !config.MaintainCount { - return logical.ErrorResponse("Can not publish a running storage metrics count to metrics without first maintaining that count. Enable `maintain_stored_certificate_counts` to enable `publish_stored_certificate_count_metrics."), nil + return logical.ErrorResponse("Can not publish a running storage metrics count to metrics without first maintaining that count. Enable `maintain_stored_certificate_counts` to enable `publish_stored_certificate_count_metrics`."), nil } if err := sc.writeAutoTidyConfig(config); err != nil { From 2aefd9f3a0f5d1053cdd06ef011dbce25d146e59 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Fri, 19 May 2023 07:40:23 -0400 Subject: [PATCH 3/3] Add changelog entry Signed-off-by: Alexander Scheel --- changelog/20664.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/20664.txt diff --git a/changelog/20664.txt b/changelog/20664.txt new file mode 100644 index 000000000000..6f2b4abe61ae --- /dev/null +++ b/changelog/20664.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: Support setting both maintain_stored_certificate_counts=false and publish_stored_certificate_count_metrics=false explicitly in tidy config. +```