Add ability to perform automatic PKI tidy operations #16900
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cipherboy
force-pushed
the
cipherboy-tidy-associate-revoked-certs
branch
from
7d50d0c
to
78d9b3d
Compare
cipherboy
force-pushed
the
cipherboy-auto-tidy-take-two
branch
from
2c1018e
to
062743f
Compare
cipherboy
changed the base branch from
cipherboy-tidy-associate-revoked-certs
to
main
cipherboy
force-pushed
the
cipherboy-auto-tidy-take-two
branch
2 times, most recently
from
0ecc396
to
1a27bb6
Compare
Closed
cipherboy
commented
Aug 29, 2022
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestAutoTidy(t *testing.T) { |
There was a problem hiding this comment.
Test takes 17s as it is fwiw.
cipherboy
force-pushed
the
cipherboy-auto-tidy-take-two
branch
from
1a27bb6
to
604a73c
Compare
|
@stevendpclark updated and bumped the timeout on this test... We'll see if that helps? :-) |
This enables the PKI secrets engine to allow tidy to be started periodically by the engine itself, avoiding the need for interaction. This operation is disabled by default (to avoid load on clusters which don't need tidy to be run) but can be enabled. In particular, a default tidy configuration is written (via /config/auto-tidy) which mirrors the options passed to /tidy. Two additional parameters, enabled and interval, are accepted, allowing auto-tidy to be enabled or disabled and controlling the interval (between successful tidy runs) to attempt auto-tidy. Notably, a manual execution of tidy will delay additional auto-tidy operations. Status is reported via the existing /tidy-status endpoint. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-auto-tidy-take-two
branch
from
604a73c
to
5edf6bb
Compare
We modified the RollbackManager's execution window to allow more faithful testing of the periodicFunc. However, the TestAutoRebuild and the new TestAutoTidy would then race against each other for modifying the period and creating their clusters (before resetting to the old value). This changeset adds a lock around this, preventing the races. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
stevendpclark
approved these changes
Aug 30, 2022
This prevents a data race between the periodic func and the execution of the running tidy. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When reading from tidyStatus for computing gauges, since the underlying values aren't atomics, we really should be gating these with a read lock around the status access. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
stevendpclark
approved these changes
Aug 30, 2022
|
Thanks! Finally got all the data races figured out! ty! |
Closed
beornf
reviewed
Aug 30, 2022
| "interval_duration": { | ||
| Type: framework.TypeDurationSecond, | ||
| 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: 43200, // 32h, but TypeDurationSecond currently requires the default to be an int. |
There was a problem hiding this comment.
Ah, good eyes. Yes, a typo. Note though that auto-tidy isn't enabled by default.
|
Very nice; I am looking forward to trying it out! |
cipherboy
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables the PKI secrets engine to allow tidy to be started
periodically by the engine itself, avoiding the need for interaction.
This operation is disabled by default (to avoid load on clusters which
don't need tidy to be run) but can be enabled.
In particular, a default tidy configuration is written (via
/config/auto-tidy) which mirrors the options passed to/tidy. Twoadditional parameters, enabled and interval, are accepted, allowing
auto-tidy to be enabled or disabled and controlling the interval
(between successful tidy runs) to attempt auto-tidy.
Notably, a manual execution of tidy will delay additional auto-tidy
operations. Status is reported via the existing
/tidy-statusendpoint.Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>