[File upload] Migrate routing to NP & add route validation #52313
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💚 Build Succeeded |
added 2 commits
December 23, 2019 16:27
…-validation # Conflicts: # x-pack/legacy/plugins/file_upload/server/plugin.js # x-pack/legacy/plugins/file_upload/server/routes/file_upload.js
💚 Build SucceededTo update your PR or re-run it, just comment with: |
added 19 commits
December 26, 2019 08:49
…gic on server-side
kindsun
commented
Feb 3, 2020
| const { appName, index, data, settings, mappings, ingestPipeline } = indexingDetails; | ||
|
|
||
| return await httpService({ | ||
| url: `${apiBasePath}/fileupload/import${paramString}`, | ||
| url: `/api/fileupload/import`, |
There was a problem hiding this comment.
Using NP internal fetch, it's no longer necessary to prepend a basepath
kindsun
commented
Feb 3, 2020
| method: 'POST', | ||
| ...(query ? { query }: {}), |
There was a problem hiding this comment.
We were appending this to the request string. Add here as a query instead when it exists
kindsun
added
release_note:skip
kindsun
changed the title
joshdover
reviewed
Feb 3, 2020
| @@ -17,11 +14,10 @@ export async function http(options) { | |||
| }); | |||
| } | |||
| const url = options.url || ''; | |||
| const headers = addSystemApiHeader({ | |||
There was a problem hiding this comment.
FYI you should be passing the asSystemRequest option to http.fetch if you still need this. That said, I'm not sure this request should be a system request. That flag is mostly used to bypass logic like session expiration or unauthorized responses that should redirect to the login page.
There was a problem hiding this comment.
Good to know. I don't think we'd want to treat it as a system request since eventually we'll want it to redirect to login if needed (i.e.- not bypass the logic that checks auth, etc.). So, sounds like the changes in place make sense?
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
thomasneirynck
approved these changes
Feb 6, 2020
kindsun
pushed a commit
to kindsun/kibana
that referenced
this pull request
Feb 6, 2020
…2313) * Cursory validation * Handle empty data arrays and settings conditionally * Set validation defaults. Move logic to routes folder and separate for testing * Move plugin init back into routes folder. Syntax updates * Migrate router to NP * Use new np router and fetch. Add placeholder schema validation * Override default 'maxBytes' * Body with first-level schema keys in place * Add conditional validation of mappings, data and settings. Clean up old joi code * Ensure query is handled correctly on both sides. Iron out decision logic on server-side * Move conditional validation to first step in payload handling * Update http_service to work with latest NP changes on master * Some reorg. Only update telemetry if no errors * Clean up * Test file upload validation logic * Linting * Review feedback. Remove unneeded apiBasePath var * Pass entire req object with through to ES, not just the validated fields
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Feb 7, 2020
…b.com:jloleysens/kibana into console/feature/text-objects-in-saved-objects * 'console/feature/text-objects-in-saved-objects' of github.com:jloleysens/kibana: (103 commits) fix auto closing new vis modal when navigating to lens or when navigating away with browser history (elastic#56998) TS of esKuery\node_types (elastic#56857) Kibana app migration: Move static code dependencies into kibana_legacy plugin, part 1 (elastic#56408) Retry ES API calls that fail with 410/Gone (elastic#56950) [APM] Show missing permissions message to the user on the Services overview (elastic#56374) Fixing flaky CI tests for custom appRoutes (elastic#55763) [State Management][Docs] State syncing utils docs (elastic#56479) [Index management] Remove index mapper setting in tests (elastic#57066) Exposed common EuiExpressions to separate components be able to reuse for building new for Alert Types (elastic#56466) [SIEM] update url state between page if date is relative (elastic#56813) fix for chart_types test (elastic#57056) chore(NA): remove compress from dll minimizer (elastic#57023) [File upload] Migrate routing to NP & add route validation (elastic#52313) Adding docs for grouped nav advanced setting (elastic#57013) Use i18n titles for field formatters, human names for numeral locales (elastic#56348) [Maps] Remove EMS catalogue url from docs (elastic#57020) [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (elastic#56538) [DOCS] Adds Apple notarization info to install doc (elastic#57042) [ML] New Platform server shim: update results service routes to use new platform router (elastic#56886) Fix typo on detection engine rule (elastic#56993) ...
kindsun
pushed a commit
that referenced
this pull request
Feb 7, 2020
…57063) * Cursory validation * Handle empty data arrays and settings conditionally * Set validation defaults. Move logic to routes folder and separate for testing * Move plugin init back into routes folder. Syntax updates * Migrate router to NP * Use new np router and fetch. Add placeholder schema validation * Override default 'maxBytes' * Body with first-level schema keys in place * Add conditional validation of mappings, data and settings. Clean up old joi code * Ensure query is handled correctly on both sides. Iron out decision logic on server-side * Move conditional validation to first step in payload handling * Update http_service to work with latest NP changes on master * Some reorg. Only update telemetry if no errors * Clean up * Test file upload validation logic * Linting * Review feedback. Remove unneeded apiBasePath var * Pass entire req object with through to ES, not just the validated fields
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds validation for file_upload route
/api/fileupload/importfor both initial index creation and subsequent index population using the NP router and schema validation