Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ability to Disable Replication Status Endpoints in Listener Configuration #23547

Merged
merged 21 commits into from
Oct 11, 2023

Conversation

marcboudreau
Copy link
Contributor

This PR adds the ability to parse a configuration setting in the TCP listener configuration section so that the value can be stored in the http.Request context. This will allow the enterprise version implementation of the replication status endpoints:

  • sys/replication/status
  • sys/replication/dr/status
  • sys/replication/performance/status
    to react accordingly.

@marcboudreau marcboudreau marked this pull request as draft October 6, 2023 13:58
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Oct 6, 2023
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Build Results:
All builds succeeded! ✅

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

CI Results:
All Go tests succeeded! ✅

@marcboudreau marcboudreau force-pushed the marcboudreau/VAULT-19869/disable-status-paths branch 2 times, most recently from 13583de to 7bbd5bb Compare October 10, 2023 12:42
@marcboudreau marcboudreau marked this pull request as ready for review October 10, 2023 15:54
@marcboudreau marcboudreau requested a review from a team as a code owner October 10, 2023 15:54
Comment on lines 205 to 207
- `disable_replication_status_endpoints` `(bool: false)` - If set true, the
replication status endpoints will be disabled for requests sent to this
listener.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `disable_replication_status_endpoints` `(bool: false)` - If set true, the
replication status endpoints will be disabled for requests sent to this
listener.
- `disable_replication_status_endpoints` `(bool: false)` - Disables replication
status endpoints for the configured listener when set to `true`.

Style correction: write in active voice, avoid "this" as a pronoun

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this reads much better.

@@ -133,6 +133,14 @@ func rateLimitQuotaWrapping(handler http.Handler, core *vault.Core) http.Handler
})
}

func disableReplicationStatusEndpointWrapping(h http.Handler) http.Handler {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needs a go doc comment here 😄

Copy link

@peteski22 peteski22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just needs @schavis' suggestions taken into account for the docs update. 🚀

@marcboudreau
Copy link
Contributor Author

@schavis, I've make the edit you suggested.

@marcboudreau marcboudreau merged commit 01cd9d3 into main Oct 11, 2023
107 checks passed
@marcboudreau marcboudreau deleted the marcboudreau/VAULT-19869/disable-status-paths branch October 11, 2023 18:23
@@ -0,0 +1,3 @@
```release-note:feature
config/listener: allow per-listener configuration setting to disable replication status endpoints.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcboudreau next time please use the correct new feature formatting for new features in the changelog - this also applies to #23534 and #23740. Probably there should be only a single changelog entry introducing the feature as a whole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants