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

[Connectors][API] Updated connectors with isMissingSecrets flag #98223

Merged
merged 16 commits into from
Apr 27, 2021

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Apr 23, 2021

Resolves #94106

Current PR extended connectors server API with the new field isMissingSecrets, which is readonly from the API.
Update connectors APIs - create and update always set isMissingSecrets as true.
Add migration to add isMissingSecrets: true to all existing connectors.

For the exported connectors ndjson:

{
   "attributes":{
      "actionTypeId":".email",
      "config":{
         "from":"jo.naumenko@gmail.com",
         "hasAuth":true,
         "host":"lghjg",
         "port":7878,
         "secure":true,
         "service":null
      },
      "isMissingSecrets":true,
      "name":"test"
   },
   "coreMigrationVersion":"8.0.0",
   "id":"e8aa94e0-a6ef-11eb-bd58-6b2eae02c6ef",
   "migrationVersion":{
      "action":"7.14.0"
   },
   "references":[
      
   ],
   "type":"action",
   "updated_at":"2021-04-27T00:31:29.598Z",
   "version":"WzE4LDFd"
}{
   "attributes":{
      "actionTypeId":".server-log",
      "config":{
         
      },
      "isMissingSecrets":false,
      "name":"test"
   },
   "coreMigrationVersion":"8.0.0",
   "id":"ed02cb70-a6ef-11eb-bd58-6b2eae02c6ef",
   "migrationVersion":{
      "action":"7.14.0"
   },
   "references":[
      
   ],
   "type":"action",
   "updated_at":"2021-04-27T00:31:36.883Z",
   "version":"WzE5LDFd"
}{
   "exportedCount":2,
   "missingRefCount":0,
   "missingReferences":[
      
   ]
}

Screen Shot 2021-04-27 at 10 16 45 AM

Export will be done in the separate issue

@YulNaumenko YulNaumenko self-assigned this Apr 23, 2021
@YulNaumenko YulNaumenko added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Actions v7.14.0 v8.0.0 release_note:enhancement labels Apr 25, 2021
@YulNaumenko YulNaumenko marked this pull request as ready for review April 26, 2021 00:38
@YulNaumenko YulNaumenko requested a review from a team as a code owner April 26, 2021 00:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@YulNaumenko YulNaumenko requested a review from a team as a code owner April 26, 2021 04:15
@mikecote mikecote requested a review from ymao1 April 26, 2021 11:32
@mikecote
Copy link
Contributor

@ymao1 I've added you as a reviewer to ensure this aligns with what you were thinking for import/export.

@YulNaumenko YulNaumenko changed the title [Connectors][API] Updated connectors with enabledAfterImport flag [Connectors][API] Updated connectors with isMissingSecrets flag Apr 26, 2021
@ymao1
Copy link
Contributor

ymao1 commented Apr 27, 2021

I tried testing out the migration by running master and creating an action, then running this branch. I get this error:

Error: Unable to complete saved object migrations for the [.kibana] index. Error: [{"index":{"error":{"type":"strict_dynamic_mapping_exception","reason":"mapping set to strict, dynamic introduction of [isMissingSecrets] within [action] is not allowed"}}}]

I see the addition of the field within the actions mapping.json, so maybe I'm doing something wrong? 🤔

@mikecote
Copy link
Contributor

I tried testing out the migration by running master and creating an action, then running this branch.

@ymao1 (just popping my head) This may be happening bacause the migrations got skipped. It may have detected you were on the same Kibana version as master. If true, to make migrations happen, you'd need to run a prior version (ex: 7.x) ES + Kibana and then upgrade both. If you're using snapshots, you'll also need to make it persist the Elasticsearch data by using something like yarn es snapshot --ssl -E path.data=/my/path/to/data. Feel free to reach out to me if a manual run-through is easier. It can get a bit complex.

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a minor comment about formatting in the legacy API docs

docs/api/actions-and-connectors/legacy/create.asciidoc Outdated Show resolved Hide resolved
docs/api/actions-and-connectors/legacy/get.asciidoc Outdated Show resolved Hide resolved
docs/api/actions-and-connectors/legacy/get_all.asciidoc Outdated Show resolved Hide resolved
docs/api/actions-and-connectors/legacy/update.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

Threat hunting LGTM

@jonathan-buttner
Copy link
Contributor

@XavierM Do we need to do a migration in the detection rules area like we had to do last time?

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
actions 113 114 +1

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
action 5 6 +1
Unknown metric groups

API count

id before after diff
actions 113 114 +1

History

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

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit 33f47ba into elastic:master Apr 27, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Apr 27, 2021
…tic#98223)

* [Connectors][API] Updated connectors with enabledAfterImport flag

* fixed functional tests

* added new field to connectors API docs

* added update unit test

* fixed test

* renamed enableAfterImport to isMissingSecrets

* removed onExport

* revert the logic of true/false for isMissingSecrets

* fixed test

* fixed tests

* added unit test

* fixed docs

* fixed import text and button labels

* fixed import text

* fixed text
YulNaumenko added a commit that referenced this pull request Apr 27, 2021
…) (#98552)

* [Connectors][API] Updated connectors with enabledAfterImport flag

* fixed functional tests

* added new field to connectors API docs

* added update unit test

* fixed test

* renamed enableAfterImport to isMissingSecrets

* removed onExport

* revert the logic of true/false for isMissingSecrets

* fixed test

* fixed tests

* added unit test

* fixed docs

* fixed import text and button labels

* fixed import text

* fixed text
madirey pushed a commit to madirey/kibana that referenced this pull request May 11, 2021
…tic#98223)

* [Connectors][API] Updated connectors with enabledAfterImport flag

* fixed functional tests

* added new field to connectors API docs

* added update unit test

* fixed test

* renamed enableAfterImport to isMissingSecrets

* removed onExport

* revert the logic of true/false for isMissingSecrets

* fixed test

* fixed tests

* added unit test

* fixed docs

* fixed import text and button labels

* fixed import text

* fixed text
@mikecote mikecote added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Actions release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Connectors][API] Update connectors with enabledAfterImport flag
9 participants