-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[OAS] Support setting availability #196647
Changes from 6 commits
b33f607
ed1e333
4254559
e03d4d2
60be4e4
58d13de
a783311
6fe22e6
251b5ce
5e462cf
a2a9b3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -321,6 +321,22 @@ export interface RouteConfigOptions<Method extends RouteMethod> { | |
* @default false | ||
*/ | ||
httpResource?: boolean; | ||
|
||
/** | ||
* Based on the the ES API specification (see https://github.com/elastic/elasticsearch-specification) | ||
* Kibana APIs can also specify some metadata about API availability. | ||
* | ||
* @remark intended to be used for informational purposes only. | ||
* | ||
*/ | ||
availability?: { | ||
/** @default stable */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Route access defaults to 'internal'. WDYT about reflecting that here and setting the default stability as 'experimental'? That way there's less risk to adding new routes and forgetting to change the stability descriptor to 'experimental' pre-GA. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I considered this too, but then realised our existing public routes would default to "experimental" too unless we explicitly set them all to "stable". So, in practice, we default to "stable" already today. I'm not opposed to changing the default, and I tend to agree with you starting as "experimental" is better I just wanted to avoid a wider effort. We could do it as follow up work, happy to open an issue. I wasn't too concerned about internal routes bc they are in principle "not available", but I think we can capture that in the doc comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We don't have that many public routes as we do internal. From a cost/benefit pov, changing public routes to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds good, LMK when you've created an issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #197003 🙌🏻 |
||
stability?: 'experimental' | 'beta' | 'stable'; | ||
/** | ||
* Must be a stack version | ||
jloleysens marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
since?: string; | ||
}; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ import { | |
type RouterRoute, | ||
type RouteValidatorConfig, | ||
} from '@kbn/core-http-server'; | ||
import { KnownParameters } from './type'; | ||
import { CustomOperationObject, KnownParameters } from './type'; | ||
import type { GenerateOpenApiDocumentOptionsFilters } from './generate_oas'; | ||
|
||
const tagPrefix = 'oas-tag:'; | ||
|
@@ -165,3 +165,17 @@ export const getXsrfHeaderForMethod = ( | |
}, | ||
]; | ||
}; | ||
|
||
export function setXState( | ||
availability: RouteConfigOptions<RouteMethod>['availability'], | ||
operation: CustomOperationObject | ||
): void { | ||
if (availability) { | ||
if (availability.stability === 'experimental') { | ||
operation['x-state'] = 'Technical Preview'; | ||
} | ||
if (availability.stability === 'beta') { | ||
operation['x-state'] = 'Beta'; | ||
} | ||
} | ||
} | ||
Comment on lines
+169
to
+181
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the logic for adding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ES specification has
availability
as eitherstack
orserverless
and we're missing that in the route config options.It's probably not a deal breaker now but would be great to surface in the generated spec.