-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Move @kbn/config-schema
to server] stack_connectors
#191864
base: main
Are you sure you want to change the base?
[Move @kbn/config-schema
to server] stack_connectors
#191864
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
Pinging @elastic/appex-ai-infra (Team:AI Infra) |
export type SecretsConfigurationType = TypeOf<typeof SecretConfigurationSchema>; | ||
export type CAType = TypeOf<typeof AuthConfiguration.ca>; | ||
export type VerificationModeType = TypeOf<typeof AuthConfiguration.verificationMode>; | ||
export type { |
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.
Reexporting the types from common
to avoid more changes in other parts of the code.
|
||
export const SecretConfigurationSchemaValidation = { | ||
validate: (secrets: any) => { |
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.
any
is flagged as an error in the server
but not in common
😅
So I chose to remove it instead of skipping it.
💚 Build Succeeded
Metrics [docs]
To update your PR or re-run it, just comment with: |
SecretConfigurationSchemaValidation | ||
); | ||
|
||
export type HasAuth = TypeOf<typeof hasAuthSchema>; |
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.
Could you please preserve the initial file structure and keep these in a separate auth/types.ts
? 🙏
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.
ACK! I'm not sure if that'll defeat the purpose since the types are based on a runtime object. I'll play around with this to see if there's a more elegant way :)
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.
Defend workflows - LGTM 🚀
Will leave review up to @elastic/security-generative-ai and @elastic/appex-ai-infra, don't want to approve for them (which IIUC will happen if I approve on behalf of |
Summary
Part of #189476.
We want to avoid
@kbn/config-schema
from leaking to the browser, and this plugin is using it outside the./server
directory.I found that the UI only references this code via
import type
, so I don't expect it to have any impact on bundle metrics, but this PR is still useful to avoid potential future leaks.For maintainers