Skip to content
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

[Workplace Search] Add a technical preview of connecting GitHub via GitHub apps #119764

Merged
merged 17 commits into from
Dec 6, 2021

Conversation

yakhinvadim
Copy link
Contributor

@yakhinvadim yakhinvadim commented Nov 26, 2021

Summary

This PR introduces a technical preview of connecting GitHub via Github apps.

The new connection flows are available by opening these URLs directly:
http://localhost:5601/iud/app/enterprise_search/workplace_search/sources/add/github_via_app and
http://localhost:5601/iud/app/enterprise_search/workplace_search/sources/add/github_enterprise_server_via_app

As always, best to review commit-by-commit.
Unit tests and i18n were intentionally skipped in this PR.

Please note that this PR only works with the backend on branch chenhui/new-github-service-type.

github_via_app

Screen.Cast.2021-12-02.at.3.07.14.PM.mp4

DLP is not available on Basic license:
image

github_enterprise_server_via_app

Screen.Cast.2021-12-02.at.3.10.29.PM.mp4

DLP is not available on Basic license:
image

Checklist

Intentionally skipped in this PR

For maintainers

@yakhinvadim

This comment has been minimized.

@yakhinvadim

This comment has been minimized.

@yakhinvadim

This comment has been minimized.

Vadim Yakhin added 2 commits December 2, 2021 13:34
Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server
…b apps

Also rename github_app to github_via_app to match service_type on backend
@yakhinvadim yakhinvadim force-pushed the github-app-connection branch 2 times, most recently from 5af1a37 to 67d12c5 Compare December 2, 2021 22:49
@yakhinvadim yakhinvadim changed the title WIP [Workplace Search] Add a technical preview of connecting GitHub via GitHub apps Dec 2, 2021
@yakhinvadim yakhinvadim added auto-backport Deprecated - use backport:version if exact versions are needed v8.0.0 v8.1.0 release_note:skip Skip the PR/issue when compiling release notes labels Dec 2, 2021
@yakhinvadim yakhinvadim marked this pull request as ready for review December 2, 2021 23:17
@yakhinvadim yakhinvadim requested a review from a team December 2, 2021 23:17
@yakhinvadim

This comment has been minimized.

@yakhinvadim
Copy link
Contributor Author

I will also add an icon to the new service types.

Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few NITs. LGTM overall. Will watch for changes mentioned in comments.

@yakhinvadim
Copy link
Contributor Author

Good points, Scotty, all addressed in this commit: b16630d

@yakhinvadim
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest changes LGTM! Great work, Vadim 🎉

@yakhinvadim
Copy link
Contributor Author

yakhinvadim commented Dec 3, 2021

Not merging yet. I need to add icons to new service types, but can't verify that it works, as the backend stopped creating sources after the last rebase. Will revisit on Monday.

@scottybollinger
Copy link
Contributor

@elasticmachine merge upstream

@yakhinvadim yakhinvadim enabled auto-merge (squash) December 6, 2021 19:37
@yakhinvadim
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1336 1339 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 1.3MB 1.3MB +4.1KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@yakhinvadim yakhinvadim merged commit 60f463a into elastic:main Dec 6, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Dec 6, 2021
…itHub apps (elastic#119764)

* Add new routes

Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server

* Add readUploadedFileAsText

* Add new view and logic for creating a GitHub content source via GitHub apps

Also rename github_app to github_via_app to match service_type on backend

* Make editPath (path to connector settings) optional as it is
not available for GitHub apps which are configured on a source level

* Update source_settings and source_logic to include configuration for
new source types.

Add new "secret" field to ContentSourceFullData and mocks

* Rename indexPermissions to index_permissions

* Extract handlePrivateKeyUpload into a utility function

* Extract `github_via_app` and `github_enterprise_server_via_app` to constants

* Add a basic validation: submit button is disabled if fields are empty

* Address PR feedback

* Do not rely on baseUrl field emptyness to define the service type

Rely on explicit parameter instead

* Add icons to the new GitHub service types

* Fix a bug where indexPermissionsValue was true even on basic license.

The solution copied from the add_source component.

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Dec 6, 2021
…itHub apps (#119764) (#120553)

* Add new routes

Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server

* Add readUploadedFileAsText

* Add new view and logic for creating a GitHub content source via GitHub apps

Also rename github_app to github_via_app to match service_type on backend

* Make editPath (path to connector settings) optional as it is
not available for GitHub apps which are configured on a source level

* Update source_settings and source_logic to include configuration for
new source types.

Add new "secret" field to ContentSourceFullData and mocks

* Rename indexPermissions to index_permissions

* Extract handlePrivateKeyUpload into a utility function

* Extract `github_via_app` and `github_enterprise_server_via_app` to constants

* Add a basic validation: submit button is disabled if fields are empty

* Address PR feedback

* Do not rely on baseUrl field emptyness to define the service type

Rely on explicit parameter instead

* Add icons to the new GitHub service types

* Fix a bug where indexPermissionsValue was true even on basic license.

The solution copied from the add_source component.

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Vadim Yakhin <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Dec 7, 2021
…-chromium-before-compiling-pdf

* 'main' of github.com:elastic/kibana: (121 commits)
  FTR should use the new kibana_system user (elastic#120436)
  [Lens] Temporarily exclude Mosaic/Waffle from the suggestions list (elastic#120488)
  [Reporting] Fix slow CSV with large max size bytes (elastic#120365)
  [CTI] Threat Intel Card on Overview page needs to accommodate Fleet TI integrations -  (elastic#120459)
  [Dashboard] Added KibanaThemeProvider.  (elastic#120122)
  add more rule_registry unit tests (elastic#120323)
  [Lens] update xpack.lens.pie.smallValuesWarningMessage text (elastic#120478)
  [Fleet] Simplified package policy create API, enriching default values (elastic#119739)
  mock `elastic-apm-node` in `@kbn/test` jest preset (elastic#120324)
  [RAC] Rename occurrences of alert_type to rule_type in Infra (elastic#120455)
  [Security Solutions] Removes tech debt of exporting all from linter rule for timeline plugin (elastic#120437)
  [Workplace Search] Add API Keys view to replace Access tokens (elastic#120147)
  [Security Solutions] Removes tech debt of exporting all from linter rule for cases plugin in the server section (elastic#120411)
  Add support for threat.feed.name (elastic#120250)
  [Rule Registry] Rewrite APM registry rules for Observability (elastic#117740)
  [docs] Fix artifact layout formatting (elastic#119386)
  [Workplace Search] Add a technical preview of connecting GitHub via GitHub apps (elastic#119764)
  add osquery notes for 7.16 (elastic#120407)
  chore(NA): splits types from code on @kbn/docs-utils (elastic#120533)
  [Reporting] Decouple screenshotting plugin from the reporting (elastic#120110)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.test.ts
#	x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts
#	x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
#	x-pack/plugins/reporting/server/lib/screenshots/observable.ts
#	x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts
Heenawter pushed a commit to Heenawter/kibana that referenced this pull request Dec 8, 2021
…itHub apps (elastic#119764)

* Add new routes

Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server

* Add readUploadedFileAsText

* Add new view and logic for creating a GitHub content source via GitHub apps

Also rename github_app to github_via_app to match service_type on backend

* Make editPath (path to connector settings) optional as it is
not available for GitHub apps which are configured on a source level

* Update source_settings and source_logic to include configuration for
new source types.

Add new "secret" field to ContentSourceFullData and mocks

* Rename indexPermissions to index_permissions

* Extract handlePrivateKeyUpload into a utility function

* Extract `github_via_app` and `github_enterprise_server_via_app` to constants

* Add a basic validation: submit button is disabled if fields are empty

* Address PR feedback

* Do not rely on baseUrl field emptyness to define the service type

Rely on explicit parameter instead

* Add icons to the new GitHub service types

* Fix a bug where indexPermissionsValue was true even on basic license.

The solution copied from the add_source component.

Co-authored-by: Kibana Machine <[email protected]>
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…itHub apps (elastic#119764)

* Add new routes

Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server

* Add readUploadedFileAsText

* Add new view and logic for creating a GitHub content source via GitHub apps

Also rename github_app to github_via_app to match service_type on backend

* Make editPath (path to connector settings) optional as it is
not available for GitHub apps which are configured on a source level

* Update source_settings and source_logic to include configuration for
new source types.

Add new "secret" field to ContentSourceFullData and mocks

* Rename indexPermissions to index_permissions

* Extract handlePrivateKeyUpload into a utility function

* Extract `github_via_app` and `github_enterprise_server_via_app` to constants

* Add a basic validation: submit button is disabled if fields are empty

* Address PR feedback

* Do not rely on baseUrl field emptyness to define the service type

Rely on explicit parameter instead

* Add icons to the new GitHub service types

* Fix a bug where indexPermissionsValue was true even on basic license.

The solution copied from the add_source component.

Co-authored-by: Kibana Machine <[email protected]>
gbamparop pushed a commit to gbamparop/kibana that referenced this pull request Jan 12, 2022
…itHub apps (elastic#119764)

* Add new routes

Render the new github_via_app component on the new routes (added in future commits).

Use isGithubEnterpriseServer prop as differentiator between github and github enterprise server

* Add readUploadedFileAsText

* Add new view and logic for creating a GitHub content source via GitHub apps

Also rename github_app to github_via_app to match service_type on backend

* Make editPath (path to connector settings) optional as it is
not available for GitHub apps which are configured on a source level

* Update source_settings and source_logic to include configuration for
new source types.

Add new "secret" field to ContentSourceFullData and mocks

* Rename indexPermissions to index_permissions

* Extract handlePrivateKeyUpload into a utility function

* Extract `github_via_app` and `github_enterprise_server_via_app` to constants

* Add a basic validation: submit button is disabled if fields are empty

* Address PR feedback

* Do not rely on baseUrl field emptyness to define the service type

Rely on explicit parameter instead

* Add icons to the new GitHub service types

* Fix a bug where indexPermissionsValue was true even on basic license.

The solution copied from the add_source component.

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants