Deprecate URLs with "team" and "query" terminology#40520
Conversation
| kithttp.ServerBefore( | ||
| kithttp.PopulateRequestContext, // populate the request context with common fields | ||
| auth.SetRequestsContexts(svc), | ||
| endpointer.LogDeprecatedPathAlias, // log deprecation warning for deprecated URL path aliases |
There was a problem hiding this comment.
The reason we need this new middleware, vs. just adding the logging code directly in the deprecated path's wrappedHandler, is that we don't have a logging context at the time that the handler is called (the context is created in SetRequestsContexts). We could have the wrapper create its own context and have SetRequestsContexts use that one if it exists, but I wasn't comfortable messing around with how our log context gets instantiated. As it is, this middleware is very light -- if the log topic is disabled or there's no deprecation info in the context, it bails.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40520 +/- ##
==========================================
+ Coverage 66.27% 66.29% +0.02%
==========================================
Files 2461 2465 +4
Lines 197427 197492 +65
Branches 8618 8618
==========================================
+ Hits 130842 130926 +84
+ Misses 54740 54723 -17
+ Partials 11845 11843 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #40519 # Details This PR adds a new system for registering deprecated URLs separately from the main URLs (i.e. not clogging up `handler.go` with a bunch of `.WithAltPaths()` or similar. It uses a registry that's shared between all the different endpointer, which is then iterated over and a new handler is created for the deprecated endpoint which stores info about the deprecation (the old and new URLs) in the context. A new middleware looks for that context info and, if found, logs a deprecation warning (if the topic is enabled). # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. no need for a changelog as we are not logging the warnings by default ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually * Verified that going to `/teams` with `--logging_enable_topics=deprecated-field-names` got me this log: ``` deprecated_path=/api/_version_/fleet/teams deprecation_warning="API `/api/_version_/fleet/teams` is deprecated, use `/api/_version_/fleet/fleets` instead ``` * Going to `/fleets` with that flag enabled resulted in no deprecation log * Going to `/teams` _without_ the flag enabled resulted in no deprecation log
Related issue: Resolves #40519
Details
This PR adds a new system for registering deprecated URLs separately from the main URLs (i.e. not clogging up
handler.gowith a bunch of.WithAltPaths()or similar. It uses a registry that's shared between all the different endpointer, which is then iterated over and a new handler is created for the deprecated endpoint which stores info about the deprecation (the old and new URLs) in the context. A new middleware looks for that context info and, if found, logs a deprecation warning (if the topic is enabled).Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
no need for a changelog as we are not logging the warnings by default
Testing
/teamswith--logging_enable_topics=deprecated-field-namesgot me this log:/fleetswith that flag enabled resulted in no deprecation log/teamswithout the flag enabled resulted in no deprecation log