#1391: ensure all tags are valid (#2124)

## What

This PR does two things:

1. Add tests to ensure that all OpenAPI tags are listed in the root-level `tags` property of the spec.
2. Changes the tags for endpoints that use any of the deprecated tags (specifically `admin`).

## Why

When we moved to using the docusaurus OpenAPI doc generator, we made some changes to existing tags. Moving away from having just 'admin', 'client', and one or two more, we now use a more granular system, which makes it easier to find an endpoint by category.

However, there's currently nothing enforcing this and this bit of knowledge hasn't been clearly communicated to anyone. Because of this, we have some endpoints that are currently just grouped under a general 'admin' tag.

This has two drawbacks:

-   In Swagger UI, all these endpoints are grouped together. This means that they break with the expectation that each endpoint has its own category. Further, being lumped together means that the 'admin' group is hard to read.
-   The 'admin' tag is (on purpose) not included in the root-level `tags` property of the generated OpenAPI spec. This means that the OpenAPI docusaurus generator doesn't pick up those endpoints, which means that they're not included in the docs on docs.getunleash.io.

## How

By implementing tests that:

1. Check that the tags included in the spec are a subset of the "approved" tags

2. By checking each path in the OpenAPI spec and making sure that all its tags are listed in the root-level tag list. If this is not the case, the test fails and it logs an error explaining what is wrong to the console. An example of such a message is:

```
The OpenAPI spec contains path-level tags that are not listed in the root-level tags object. The relevant paths, operation ids, and tags are as follows:

POST /api/admin/feedback (operation id: createFeedback) has the following invalid tags: "admin"

PUT /api/admin/feedback/{id} (operation id: updateFeedback) has the following invalid tags: "admin"

For reference, the root-level tags are: "Addons", "Admin UI", "API tokens", "Archive", "Auth", "Client", "Context", "Edge", "Environments", "Events", "Features", "Import/Export", "Metrics", "Operational", "Playground", "Projects", "Public signup tokens", "Strategies", "Tags", "Unstable", "Users"
```

## Commits

* fix: ensure that all root-level tags in the spec are 'approved tags'

* fix: test that all listed path tags exist in the root-level tags

* fix: use "API tokens" tag for PAT endpoints

* fix: add comment explaining why tags test is there

* fix: Update snapshot

* fix: ensure that spec tags are a subset of the approved Tags

* fix: improve error message when tags don't match

* fix: further tweaks in log format

src/lib/openapi/util/openapi-tags.ts

@@ -1,3 +1,6 @@
+// The "official" OpenAPI tags that we use. These tags are added to the OpenAPI
+// spec as the root-level "tags" list. Consider creating a new entry here when
+// creating a new endpoint.
 const OPENAPI_TAGS = [
     {
         name: 'Addons',
@@ -90,4 +93,6 @@ export const openApiTags = [...OPENAPI_TAGS].sort((a, b) =>
     a.name.localeCompare(b.name),
 );
 
-export type OpenApiTag = typeof OPENAPI_TAGS[number]['name'];
+export type OpenApiTag =
+    // The official OpenAPI tags that we use.
+    typeof OPENAPI_TAGS[number]['name'];

src/lib/routes/admin-api/user/pat.ts

@@ -39,7 +39,7 @@ export default class PatController extends Controller {
             permission: NONE,
             middleware: [
                 openApiService.validPath({
-                    tags: ['admin'],
+                    tags: ['API tokens'],
                     operationId: 'getPats',
                     responses: { 200: createResponseSchema('patsSchema') },
                 }),
@@ -52,7 +52,7 @@ export default class PatController extends Controller {
             permission: NONE,
             middleware: [
                 openApiService.validPath({
-                    tags: ['admin'],
+                    tags: ['API tokens'],
                     operationId: 'createPat',
                     requestBody: createRequestSchema('patSchema'),
                     responses: { 200: createResponseSchema('patSchema') },
@@ -68,7 +68,7 @@ export default class PatController extends Controller {
             permission: NONE,
             middleware: [
                 openApiService.validPath({
-                    tags: ['admin'],
+                    tags: ['API tokens'],
                     operationId: 'deletePat',
                     responses: { 200: emptyResponse },
                 }),

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -7018,7 +7018,7 @@ If the provided project does not exist, the list of events will be empty.",
           },
         },
         "tags": [
-          "admin",
+          "API tokens",
         ],
       },
       "post": {
@@ -7047,7 +7047,7 @@ If the provided project does not exist, the list of events will be empty.",
           },
         },
         "tags": [
-          "admin",
+          "API tokens",
         ],
       },
     },
@@ -7070,7 +7070,7 @@ If the provided project does not exist, the list of events will be empty.",
           },
         },
         "tags": [
-          "admin",
+          "API tokens",
         ],
       },
     },

src/test/e2e/api/openapi/openapi.e2e.test.ts

@@ -4,6 +4,7 @@ import getLogger from '../../../fixtures/no-logger';
 import SwaggerParser from '@apidevtools/swagger-parser';
 import enforcer from 'openapi-enforcer';
 import semver from 'semver';
+import { openApiTags } from '../../../../lib/openapi/util/openapi-tags';
 
 let app;
 let db;
@@ -100,3 +101,93 @@ test('the generated OpenAPI spec is valid', async () => {
 
     expect(enforcerWarning ?? enforcerError).toBe(undefined);
 });
+
+test('all root-level tags are "approved tags"', async () => {
+    const { body: spec } = await app.request
+        .get('/docs/openapi.json')
+        .expect('Content-Type', /json/)
+        .expect(200);
+
+    const specTags = spec.tags;
+    const approvedTags = openApiTags;
+
+    // expect spec tags to be a subset of the approved tags
+    expect(approvedTags).toEqual(expect.arrayContaining(specTags));
+});
+// All tags that are used for OpenAPI path operations must also be listed in the
+// OpenAPI root-level tags list. For us, there's two immediate things that make
+// this important:
+//
+// 1. Swagger UI groups operations by tags. To make sure that endpoints are
+// listed where users would expect to find them, they should be given an
+// appropriate tag.
+//
+// 2. The OpenAPI/docusaurus integration we use does not generate documentation
+// for paths whose tags are not listed in the root-level tags list.
+//
+// If none of the official tags seem appropriate for an endpoint, consider
+// creating a new tag.
+test('all tags are listed in the root "tags" list', async () => {
+    const { body: spec } = await app.request
+        .get('/docs/openapi.json')
+        .expect('Content-Type', /json/)
+        .expect(200);
+
+    const rootLevelTagNames = new Set(spec.tags.map((tag) => tag.name));
+
+    // dictionary of all invalid tags found in the spec
+    let invalidTags = {};
+    for (const [path, data] of Object.entries(spec.paths)) {
+        for (const [operation, opData] of Object.entries(data)) {
+            // ensure that the list of tags for every operation is a subset of
+            // the list of tags defined on the root level
+
+            // check each tag for this operation
+            for (const tag of opData.tags) {
+                if (!rootLevelTagNames.has(tag)) {
+                    // store other invalid tags that already exist on this
+                    // operation
+                    const preExistingTags =
+                        (invalidTags[path] ?? {})[operation]?.invalidTags ?? [];
+
+                    // add information about the invalid tag to the invalid tags
+                    // dict.
+                    invalidTags = {
+                        ...invalidTags,
+                        [path]: {
+                            ...invalidTags[path],
+                            [operation]: {
+                                operationId: opData.operationId,
+                                invalidTags: [...preExistingTags, tag],
+                            },
+                        },
+                    };
+                }
+            }
+        }
+    }
+
+    if (Object.keys(invalidTags).length) {
+        // create a human-readable list of invalid tags per operation
+        const msgs = Object.entries(invalidTags).flatMap(([path, data]) =>
+            Object.entries(data).map(
+                ([operation, opData]) =>
+                    `${operation.toUpperCase()} ${path} (operation id: ${
+                        opData.operationId
+                    }) has the following invalid tags: ${opData.invalidTags
+                        .map((tag) => `"${tag}"`)
+                        .join(', ')}`,
+            ),
+        );
+
+        // format message
+        const errorMessage = `The OpenAPI spec contains path-level tags that are not listed in the root-level tags object. The relevant paths, operation ids, and tags are as follows:\n\n${msgs.join(
+            '\n\n',
+        )}\n\nFor reference, the root-level tags are: ${spec.tags
+            .map((tag) => `"${tag.name}"`)
+            .join(', ')}`;
+
+        console.error(errorMessage);
+    }
+    expect(invalidTags).toStrictEqual({});
+});