Deprecate URLs with "team" and "query" terminology (#40520)
<!-- 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
This commit is contained in:
@@ -3,11 +3,13 @@ package endpointer
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz"
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/logging"
|
||||
platform_http "github.com/fleetdm/fleet/v4/server/platform/http"
|
||||
"github.com/go-kit/kit/endpoint"
|
||||
kithttp "github.com/go-kit/kit/transport/http"
|
||||
@@ -123,3 +125,104 @@ func (n nopEP) CallHandlerFunc(f testHandlerFunc, ctx context.Context, request a
|
||||
func (n nopEP) Service() any {
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestRegisterDeprecatedPathAliases(t *testing.T) {
|
||||
// Set up a router and register a primary endpoint via CommonEndpointer.
|
||||
r := mux.NewRouter()
|
||||
registry := NewHandlerRegistry()
|
||||
versions := []string{"v1", "2022-04"}
|
||||
|
||||
authMiddleware := func(next endpoint.Endpoint) endpoint.Endpoint {
|
||||
return func(ctx context.Context, req any) (any, error) {
|
||||
if authctx, ok := authz_ctx.FromContext(ctx); ok {
|
||||
authctx.SetChecked()
|
||||
}
|
||||
return next(ctx, req)
|
||||
}
|
||||
}
|
||||
|
||||
ce := &CommonEndpointer[testHandlerFunc]{
|
||||
EP: nopEP{},
|
||||
MakeDecoderFn: func(iface any, requestBodySizeLimit int64) kithttp.DecodeRequestFunc {
|
||||
return func(ctx context.Context, r *http.Request) (request any, err error) {
|
||||
return nopRequest{}, nil
|
||||
}
|
||||
},
|
||||
EncodeFn: func(ctx context.Context, w http.ResponseWriter, i any) error {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
return nil
|
||||
},
|
||||
AuthMiddleware: authMiddleware,
|
||||
Router: r,
|
||||
Versions: versions,
|
||||
HandlerRegistry: registry,
|
||||
}
|
||||
|
||||
// Register the primary endpoint.
|
||||
ce.GET("/api/_version_/fleet/fleets", func(ctx context.Context, request any) (platform_http.Errorer, error) {
|
||||
return nopResponse{}, nil
|
||||
}, nil)
|
||||
|
||||
// Register a deprecated alias for it.
|
||||
RegisterDeprecatedPathAliases(r, versions, registry, []DeprecatedPathAlias{
|
||||
{
|
||||
Method: "GET",
|
||||
PrimaryPath: "/api/_version_/fleet/fleets",
|
||||
DeprecatedPaths: []string{"/api/_version_/fleet/teams"},
|
||||
},
|
||||
})
|
||||
|
||||
s := httptest.NewServer(r)
|
||||
t.Cleanup(s.Close)
|
||||
|
||||
// Both the primary and deprecated paths should return 200.
|
||||
for _, path := range []string{"/api/v1/fleet/fleets", "/api/v1/fleet/teams", "/api/latest/fleet/teams"} {
|
||||
resp, err := http.Get(s.URL + path)
|
||||
require.NoError(t, err)
|
||||
resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "path %s should return 200", path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLogDeprecatedPathAlias(t *testing.T) {
|
||||
// Without deprecated path info in context, LogDeprecatedPathAlias is a no-op.
|
||||
lc := &logging.LoggingContext{}
|
||||
ctx := logging.NewContext(context.Background(), lc)
|
||||
ctx2 := LogDeprecatedPathAlias(ctx, nil)
|
||||
require.Equal(t, ctx, ctx2, "should return same context when no deprecated path info")
|
||||
require.Empty(t, lc.Extras)
|
||||
|
||||
// With deprecated path info, it should set warn level and extras.
|
||||
ctx = context.WithValue(ctx, deprecatedPathInfoKey{}, deprecatedPathInfo{
|
||||
deprecatedPath: "/api/_version_/fleet/teams",
|
||||
primaryPath: "/api/_version_/fleet/fleets",
|
||||
})
|
||||
LogDeprecatedPathAlias(ctx, nil)
|
||||
|
||||
// Extras is a flat []interface{} of key-value pairs.
|
||||
require.Len(t, lc.Extras, 4) // "deprecated_path", value, "deprecation_warning", value
|
||||
require.Equal(t, "deprecated_path", lc.Extras[0])
|
||||
require.Equal(t, "/api/_version_/fleet/teams", lc.Extras[1])
|
||||
require.Equal(t, "deprecation_warning", lc.Extras[2])
|
||||
require.Contains(t, lc.Extras[3], "deprecated")
|
||||
|
||||
// ForceLevel should be set to Warn.
|
||||
require.NotNil(t, lc.ForceLevel)
|
||||
require.Equal(t, slog.LevelWarn, *lc.ForceLevel)
|
||||
}
|
||||
|
||||
func TestRegisterDeprecatedPathAliasesPanicsOnMissing(t *testing.T) {
|
||||
r := mux.NewRouter()
|
||||
registry := NewHandlerRegistry()
|
||||
versions := []string{"v1"}
|
||||
|
||||
require.Panics(t, func() {
|
||||
RegisterDeprecatedPathAliases(r, versions, registry, []DeprecatedPathAlias{
|
||||
{
|
||||
Method: "GET",
|
||||
PrimaryPath: "/api/_version_/fleet/nonexistent",
|
||||
DeprecatedPaths: []string{"/api/_version_/fleet/old"},
|
||||
},
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user