1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-25 00:07:47 +01:00
unleash.unleash/src/test/e2e/api/openapi/openapi.e2e.test.ts

194 lines
6.9 KiB
TypeScript
Raw Normal View History

import { setupApp } from '../../helpers/test-helper';
import dbInit from '../../helpers/database-init';
import getLogger from '../../../fixtures/no-logger';
import SwaggerParser from '@apidevtools/swagger-parser';
openapi: improve validation testing (#2058) ## What This PR adds an extra layer of OpenAPI validation testing to what we already have. It also fixes any issues that make the new tests fail. ## Why While the current OpenAPI validation takes care of _some_ things, there's also things it misses, as shown by #2055. By adding the OpenAPI Enforcer package, we should hopefully be able to catch more of these errors in the future. The enforcer _does_ flag the issue in #2055 as an error. ## How By adding the OpenAPI Enforcer package and making whatever changes it picks up on. By adding location headers to all our 201 endpoints. I also had to change some signatures on `create` store methods so that they actually return something (a lot of them just returned `void`). ## Discussion points ### Code changes Some of the code changes may not be necessary or we may want to change more code to align with what changes have been done. It may be worth standardizing on a pattern for `*store.create` methods, so that they always return an identifier or the stored object, for instance. ### On relative URIs The 201 location headers use relative URIs to point to the created resources. This seemed to be the easiest way to me, as we don't need to worry about figuring out what the absolute URL of the instance is (though we could probably just concat it to the request URI?). The algorithm for determining relative URIs is described in [RFC 3986 section 5](https://www.rfc-editor.org/rfc/rfc3986#section-5). There's also some places where I'm not sure we _can_ provide accurate location url. I think they're supposed to point _directly at_ whatever the resource is, but for some resources (such as api tokens), you can only get it as part of a collection. From [RFC 9110 on the location field](https://httpwg.org/specs/rfc9110.html#field.location) (emphasis mine): > the Location header field in a [201 (Created)](https://httpwg.org/specs/rfc9110.html#status.201) response is supposed to provide a URI that is **specific** to the created resource. A link to a collection is not specific. I'm not sure what best to do about this. ### Inline comments I've added a number of inline PR comments that I'd love to get some feedback on too. Have a look and let me know what you think! ### Unfinished business I've added some juicy comments to some of the files here. They contain non-blocking issues that I'm tracking (via github issues). We should resolve them in the future, but right now they can stay as they are. ## Commits * Feat: add openapi-enforcer + tests; fix _some_ issues * Test: allow non-standard string formats * validation: fix _some_ 201 created location header endpoints * #1391: fix remaining 201 location headers missing * Refactor: use the ajv options object instead of add* methods * #1391: flag validation errors and warnings as test failures * #1391: modify patch schema to specify either object or array We don't provide many patch endpoints, so we _could_ create separate patch operation objects for each one. I think that makes sense to do as part of the larger cleanup. For now, I think it's worth to simply turn it into one of these. While it's not entirely accurate, it's better than what we had before. * Refactor: make tests easier to read * #1391: use enum for valid token types This was previously only a description. This may seem like a breaking change because OpenAPI would previously accept any string. However, Joi also performs validation on this, so invalid values wouldn't work previously either. * #1391: Comment out default parameter values for now This isn't the _right_ way, but it's the pragmatic solution. It's not a big deal and this works as a stopgap solution. * #1391: add todo note for api token schema fixes * #1391: update snapshot * Revert "#1391: modify patch schema to specify either object or array" This reverts commit 0dd5d0faa1554a394178c0f632915e096204cac7. Turns out we need to allow much more than just objects and arrays. I'll leave this as is for now. * Chore(#1391): update comment explaining api token schema TODO * #1391: modify some test code and add comment * #1391: update tests and spec to allow 'any' type in schema * Chore: remove comment * #1391: add tests for context field stores * #1391: add location header for public signup links * #1391: fix query parameter description. There was indeed a bug in the package, and this has been addressed now, so we can go back to describing the params properly.
2022-09-23 15:02:09 +02:00
import enforcer from 'openapi-enforcer';
import semver from 'semver';
#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
2022-10-06 17:01:12 +02:00
import { openApiTags } from '../../../../lib/openapi/util/openapi-tags';
let app;
let db;
beforeAll(async () => {
db = await dbInit('openapi', getLogger);
app = await setupApp(db.stores);
});
afterAll(async () => {
await app.destroy();
await db.destroy();
});
test('should serve the OpenAPI UI', async () => {
return app.request
.get('/docs/openapi/')
.expect('Content-Type', /html/)
.expect(200)
.expect((res) => expect(res.text).toMatchSnapshot());
});
test('should serve the OpenAPI spec', async () => {
return app.request
.get('/docs/openapi.json')
.expect('Content-Type', /json/)
.expect(200)
.expect((res) => {
// Don't use the version field in snapshot tests. Having the version
// listed in automated testing causes issues when trying to deploy
// new versions of the API (due to mismatch between new tag versions etc).
delete res.body.info.version;
// This test will fail whenever there's a change to the API spec.
// If the change is intended, update the snapshot with `jest -u`.
expect(res.body).toMatchSnapshot();
});
});
test('should serve the OpenAPI spec with a `version` property', async () => {
return app.request
.get('/docs/openapi.json')
.expect('Content-Type', /json/)
.expect(200)
.expect((res) => {
const { version } = res.body.info;
// ensure there's no whitespace or leading `v`
expect(semver.clean(version)).toStrictEqual(version);
// ensure the version listed is valid semver
expect(semver.parse(version, { loose: false })).toBeTruthy();
});
});
test('the generated OpenAPI spec is valid', async () => {
const { body } = await app.request
.get('/docs/openapi.json')
.expect('Content-Type', /json/)
.expect(200);
// this throws if the swagger parser can't parse it correctly
// also parses examples, but _does_ do some string coercion in examples
try {
await SwaggerParser.validate(body);
} catch (err) {
console.error(err);
openapi: improve validation testing (#2058) ## What This PR adds an extra layer of OpenAPI validation testing to what we already have. It also fixes any issues that make the new tests fail. ## Why While the current OpenAPI validation takes care of _some_ things, there's also things it misses, as shown by #2055. By adding the OpenAPI Enforcer package, we should hopefully be able to catch more of these errors in the future. The enforcer _does_ flag the issue in #2055 as an error. ## How By adding the OpenAPI Enforcer package and making whatever changes it picks up on. By adding location headers to all our 201 endpoints. I also had to change some signatures on `create` store methods so that they actually return something (a lot of them just returned `void`). ## Discussion points ### Code changes Some of the code changes may not be necessary or we may want to change more code to align with what changes have been done. It may be worth standardizing on a pattern for `*store.create` methods, so that they always return an identifier or the stored object, for instance. ### On relative URIs The 201 location headers use relative URIs to point to the created resources. This seemed to be the easiest way to me, as we don't need to worry about figuring out what the absolute URL of the instance is (though we could probably just concat it to the request URI?). The algorithm for determining relative URIs is described in [RFC 3986 section 5](https://www.rfc-editor.org/rfc/rfc3986#section-5). There's also some places where I'm not sure we _can_ provide accurate location url. I think they're supposed to point _directly at_ whatever the resource is, but for some resources (such as api tokens), you can only get it as part of a collection. From [RFC 9110 on the location field](https://httpwg.org/specs/rfc9110.html#field.location) (emphasis mine): > the Location header field in a [201 (Created)](https://httpwg.org/specs/rfc9110.html#status.201) response is supposed to provide a URI that is **specific** to the created resource. A link to a collection is not specific. I'm not sure what best to do about this. ### Inline comments I've added a number of inline PR comments that I'd love to get some feedback on too. Have a look and let me know what you think! ### Unfinished business I've added some juicy comments to some of the files here. They contain non-blocking issues that I'm tracking (via github issues). We should resolve them in the future, but right now they can stay as they are. ## Commits * Feat: add openapi-enforcer + tests; fix _some_ issues * Test: allow non-standard string formats * validation: fix _some_ 201 created location header endpoints * #1391: fix remaining 201 location headers missing * Refactor: use the ajv options object instead of add* methods * #1391: flag validation errors and warnings as test failures * #1391: modify patch schema to specify either object or array We don't provide many patch endpoints, so we _could_ create separate patch operation objects for each one. I think that makes sense to do as part of the larger cleanup. For now, I think it's worth to simply turn it into one of these. While it's not entirely accurate, it's better than what we had before. * Refactor: make tests easier to read * #1391: use enum for valid token types This was previously only a description. This may seem like a breaking change because OpenAPI would previously accept any string. However, Joi also performs validation on this, so invalid values wouldn't work previously either. * #1391: Comment out default parameter values for now This isn't the _right_ way, but it's the pragmatic solution. It's not a big deal and this works as a stopgap solution. * #1391: add todo note for api token schema fixes * #1391: update snapshot * Revert "#1391: modify patch schema to specify either object or array" This reverts commit 0dd5d0faa1554a394178c0f632915e096204cac7. Turns out we need to allow much more than just objects and arrays. I'll leave this as is for now. * Chore(#1391): update comment explaining api token schema TODO * #1391: modify some test code and add comment * #1391: update tests and spec to allow 'any' type in schema * Chore: remove comment * #1391: add tests for context field stores * #1391: add location header for public signup links * #1391: fix query parameter description. There was indeed a bug in the package, and this has been addressed now, so we can go back to describing the params properly.
2022-09-23 15:02:09 +02:00
// there's an error here, so let's exit after showing it in the console.
expect(true).toBe(false);
}
openapi: improve validation testing (#2058) ## What This PR adds an extra layer of OpenAPI validation testing to what we already have. It also fixes any issues that make the new tests fail. ## Why While the current OpenAPI validation takes care of _some_ things, there's also things it misses, as shown by #2055. By adding the OpenAPI Enforcer package, we should hopefully be able to catch more of these errors in the future. The enforcer _does_ flag the issue in #2055 as an error. ## How By adding the OpenAPI Enforcer package and making whatever changes it picks up on. By adding location headers to all our 201 endpoints. I also had to change some signatures on `create` store methods so that they actually return something (a lot of them just returned `void`). ## Discussion points ### Code changes Some of the code changes may not be necessary or we may want to change more code to align with what changes have been done. It may be worth standardizing on a pattern for `*store.create` methods, so that they always return an identifier or the stored object, for instance. ### On relative URIs The 201 location headers use relative URIs to point to the created resources. This seemed to be the easiest way to me, as we don't need to worry about figuring out what the absolute URL of the instance is (though we could probably just concat it to the request URI?). The algorithm for determining relative URIs is described in [RFC 3986 section 5](https://www.rfc-editor.org/rfc/rfc3986#section-5). There's also some places where I'm not sure we _can_ provide accurate location url. I think they're supposed to point _directly at_ whatever the resource is, but for some resources (such as api tokens), you can only get it as part of a collection. From [RFC 9110 on the location field](https://httpwg.org/specs/rfc9110.html#field.location) (emphasis mine): > the Location header field in a [201 (Created)](https://httpwg.org/specs/rfc9110.html#status.201) response is supposed to provide a URI that is **specific** to the created resource. A link to a collection is not specific. I'm not sure what best to do about this. ### Inline comments I've added a number of inline PR comments that I'd love to get some feedback on too. Have a look and let me know what you think! ### Unfinished business I've added some juicy comments to some of the files here. They contain non-blocking issues that I'm tracking (via github issues). We should resolve them in the future, but right now they can stay as they are. ## Commits * Feat: add openapi-enforcer + tests; fix _some_ issues * Test: allow non-standard string formats * validation: fix _some_ 201 created location header endpoints * #1391: fix remaining 201 location headers missing * Refactor: use the ajv options object instead of add* methods * #1391: flag validation errors and warnings as test failures * #1391: modify patch schema to specify either object or array We don't provide many patch endpoints, so we _could_ create separate patch operation objects for each one. I think that makes sense to do as part of the larger cleanup. For now, I think it's worth to simply turn it into one of these. While it's not entirely accurate, it's better than what we had before. * Refactor: make tests easier to read * #1391: use enum for valid token types This was previously only a description. This may seem like a breaking change because OpenAPI would previously accept any string. However, Joi also performs validation on this, so invalid values wouldn't work previously either. * #1391: Comment out default parameter values for now This isn't the _right_ way, but it's the pragmatic solution. It's not a big deal and this works as a stopgap solution. * #1391: add todo note for api token schema fixes * #1391: update snapshot * Revert "#1391: modify patch schema to specify either object or array" This reverts commit 0dd5d0faa1554a394178c0f632915e096204cac7. Turns out we need to allow much more than just objects and arrays. I'll leave this as is for now. * Chore(#1391): update comment explaining api token schema TODO * #1391: modify some test code and add comment * #1391: update tests and spec to allow 'any' type in schema * Chore: remove comment * #1391: add tests for context field stores * #1391: add location header for public signup links * #1391: fix query parameter description. There was indeed a bug in the package, and this has been addressed now, so we can go back to describing the params properly.
2022-09-23 15:02:09 +02:00
const [, enforcerError, enforcerWarning] = await enforcer(body, {
fullResult: true,
componentOptions: {
exceptionSkipCodes: [
// allow non-standard formats for strings (including 'uri')
'WSCH001',
// Schemas with an indeterminable type cannot serialize,
// deserialize, or validate values. [WSCH005]
//
// This allows specifying the 'any' type for schemas (such as the
// patchSchema)
'WSCH005',
],
},
});
if (enforcerWarning !== undefined) {
console.warn(enforcerWarning);
}
if (enforcerError !== undefined) {
console.error(enforcerError);
}
expect(enforcerWarning ?? enforcerError).toBe(undefined);
});
#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
2022-10-06 17:01:12 +02:00
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({});
});