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

[Ingest] Add additional attributes to the Datasources Saved Object #66127

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented May 11, 2020

Summary

Adds 4 additional attributes to the Datasources Saved Object (created/updated_by ++ created/updated_on). In support of this change, a datasources saved object migration was also created for v7.9.0

closes #65904

Checklist

Delete any items that are not applicable to this PR.

  • UI testing:
    • Able to create Datasource from Integrations > [package] > create
    • Able to create Datasource from Configurations > [config] > create
    • Able to update Datasource from Configurations > [config] > edit data source
    • Able to save Endpoint Policy changes (endpoint app)
  • Manual migration testing
    (Elasticsearch + kibana was started using master. Datasources where created using the UI. Source was then switched to the branch containing these changed (Kibana server was restarted). Looking at the API calls made by the UI, I can see the new attributes being returned)

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management v7.9.0 Team:Fleet Team label for Observability Data Collection Fleet team labels May 11, 2020
@paul-tavares paul-tavares self-assigned this May 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-management (Team:Endpoint Management)

@paul-tavares
Copy link
Contributor Author

Still draft - need to go through the code usages against the Datasources service and ensure that a user: AuthenticatedUser is being passed in (since its optional)

@@ -218,8 +219,15 @@ const savedObjectTypes: { [key: string]: SavedObjectsType } = {
},
},
revision: { type: 'integer' },
updated_on: { type: 'keyword' },
updated_by: { type: 'keyword' },
created_on: { type: 'keyword' },
Copy link
Member

Choose a reason for hiding this comment

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

Why not using the type date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point @nchaulet . I copied the definition from the Agent Config SO which had keyword, but now see that Enrollment API Keys SO does use date. I will change it.
Thanks 👍

},
},
migrations: {
Copy link
Member

Choose a reason for hiding this comment

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

Even though I don't expect users to migrate from 7.8 to 7.9 I like that we get into the habit of providing a migration path! 👍

@paul-tavares paul-tavares marked this pull request as ready for review May 12, 2020 14:29
@paul-tavares paul-tavares requested a review from a team as a code owner May 12, 2020 14:29
@paul-tavares paul-tavares requested a review from a team May 12, 2020 14:29
@paul-tavares paul-tavares requested a review from a team as a code owner May 12, 2020 14:29
@paul-tavares
Copy link
Contributor Author

Ok. I think I have captured all the needed updates for this PR. Its ready for review 😬

@nchaulet
Copy link
Member

we have a mix of updated_on and updated_at|created_at looking at other plugins looks like *_at is more used what do you think of changing this? cc @jen-huang I think we should do some cleanup on our side too.

@@ -504,10 +504,15 @@ export class EndpointDocGenerator {
* Generates an Ingest `datasource` that includes the Endpoint Policy data
*/
public generatePolicyDatasource(): PolicyData {
const created = new Date(Date.now() - 8.64e7).toISOString(); // 24h ago
Copy link
Member

Choose a reason for hiding this comment

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

I imagine is just for testing purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, test purposes to mock up data,

@paul-tavares
Copy link
Contributor Author

@nchaulet I'm good with changing the attributes you mentioned to the more commonly used names 👍

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

LGTM for the ingest part (if you can make the change to updated_at|created_at)

@jen-huang
Copy link
Contributor

we have a mix of updated_on and updated_at|created_at looking at other plugins looks like *_at is more used what do you think of changing this? cc @jen-huang I think we should do some cleanup on our side too.

++ on using *_at. Looks like we only have one other *_on property (agent config's updated_on). We should normalize this sooner rather than later. Once this PR goes in, @nchaulet can you work on updating that property and adding a migration for it?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@paul-tavares paul-tavares merged commit ad39997 into elastic:master May 12, 2020
@paul-tavares paul-tavares deleted the task/ingest-65904-datasources-additional-fields branch May 12, 2020 21:35
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request May 12, 2020
…lastic#66127)

* add additional properties to ingest-datasources SO
* Adjusted Types and test generators
* Added datasources migrations to SO
* Add `user` object to calls to datasource.create()
paul-tavares added a commit that referenced this pull request May 13, 2020
…66127) (#66313)

* add additional properties to ingest-datasources SO
* Adjusted Types and test generators
* Added datasources migrations to SO
* Add `user` object to calls to datasource.create()
v1v added a commit to v1v/kibana that referenced this pull request May 13, 2020
* upstream/master: (223 commits)
  [Ingest] Support root level yaml variables in agent stream template (elastic#66120)
  [Snapshot Restore] Fix error when deleting snapshots behind reverse proxy (elastic#66147)
  [Lens] fix empty state for pie (elastic#66206)
  [APM] Improve e2e tests (elastic#66373)
  [ML] Data Frame Analytics: Fix steps to be named phases. (elastic#65855)
  [Discover] Encode context link filter part (elastic#63831)
  [APM] Scope APM alert creation to environment (elastic#65681)
  Move Kibana Usage collectors outside the telemetry plugin (elastic#65663)
  [ML] Data Frame Analytics: Fix confusion matrix data grid width. (elastic#65818)
  Switch to core application service (elastic#63443)
  Removes use of prefer_v2_templates (elastic#66316)
  [Maps] handle case where fit to bounds does not match any documents (elastic#66307)
  log error instead of throw (elastic#66254)
  [plugin-helpers] remove outdated postinstall task (elastic#66324)
  Spaces - migrate default space and enter space view to KP (elastic#66098)
  [APM] Change plugin id for `apm_oss` to `apmOss` (elastic#66164)
  [Maps] convert map_selectors to TS (elastic#65905)
  [uptime/usage-collector] add missing await (elastic#66079)
  [Ingest] Add additional attributes to the Datasources Saved Object (elastic#66127)
  [Endpoint]EMT-339: add new policy response schema (elastic#66264)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ingest] Add additional attributes to the Datasources Stored Object
7 participants