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

[Fleet] Integration Policies List view #83634

Conversation

paul-tavares
Copy link
Contributor

Summary

Adds a Policies tab to the details of an Integration Package that displays the list of all Integration Policies for the given package across all Agent Policies. View allows user to:

  • View a holistic list of integration usage across all Fleet Agent Policies
  • Link to edit integration policy
  • Link to associated Agent Policy details
  • Link to a filtered list of Agents that are currently utilizing the integration

This view is not shown for Integrations that are not yet installed.

Closes #82485

fleet-82485-package-policies-list

Checklist

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0 Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Nov 18, 2020
@paul-tavares paul-tavares requested a review from a team November 18, 2020 14:11
@paul-tavares paul-tavares self-assigned this Nov 18, 2020
@elasticmachine
Copy link
Contributor

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

* value is truncated if it is longer than the width of where it is displayed, while the revision
* always remain visible
*/
export const LinkAndRevision = memo<LinkAndRevisionProps>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI - I would like to add to this component a isOutdated: boolean prop and then refactor the Agents List view to also use this component. I have a feeling that the outdated indicator will also be wanted in other scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

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

this would also be good for our Endpoint list

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 @kevinlog , we would likely have a similar component in our code base.

/**
* Uses URL params for pagination and also persists those to the URL as they are updated
*/
export const useUrlPagination = (): UrlPagination => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about changing the existing usePagination() to always persist the page/perPage to the URL, but that could have a large impact in testing to ensure it does not break anything. So for now, I went with a separate hook which could be merged and/or used if the use case calls for these parameters to be persisted in the URL.

@nchaulet nchaulet self-requested a review November 18, 2020 20:51
import { GetPackagePoliciesResponse } from '../../../../../../../common/types/rest_spec';

export interface PackagePolicyEnriched extends PackagePolicy {
_agentPolicy: GetAgentPoliciesResponseItem | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

is the naming convention with _ just to denote that it's an enriched field and not a part of the saved objet?

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, exactly. I was looking for a property name that would not collide with any future addition to the object. I went with a prefix of _ but had also thought about using a $ suffix (ex. agentPolicy$)

@kevinlog
Copy link
Contributor

LGTM from a functionality perspective, but needs an Ingest review

Copy link
Member

@pzl pzl left a comment

Choose a reason for hiding this comment

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

LGTM, but wait for a fleet thumb

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.

Just one comment otherwise work as expected 🚀

@paul-tavares
Copy link
Contributor Author

paul-tavares commented Nov 19, 2020

Please do not merge this - Even if GitHub says the conflicts can be performed automatically, there would be ESLint failures later that I need to fix. I will address those on Momday.

Never mind - I confused this with the other PR I have pending

…integrations-policies-list

# Conflicts:
#	x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_select_agent_policy.tsx
// once parsed, would be an array. In these case, we take the last value defined
pagination.currentPage = Number(
(Array.isArray(urlParams.currentPage)
? urlParams.currentPage[urlParams.currentPage.length - 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe .pop() will give the same result. If so, I'd prefer it as it's less statements to read.

Suggested change
? urlParams.currentPage[urlParams.currentPage.length - 1]
? urlParams.currentPage.pop()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow - why did I not think of that 🤦
Thanks @jfsiii . going to merge this in, and will adjust this based on your comments in next PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@paul-tavares no worries, It Depends ™️, right :) .pop() does alter the array. I didn't see it used again so seems it works for now, but there are trade-offs.

I really wish we could urlParams.currentPage[-1] like perl/python.

After writing this I went looking and found that we can't do arr[-2] but arr.at(-2) is coming eventually via .at() method. I don't see anything about it in any of the feature support sites but it is stage 3 now, so 🤞

pagination.pageSize =
Number(
(Array.isArray(urlParams.pageSize)
? urlParams.pageSize[urlParams.pageSize.length - 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

same question/comment re: .pop()

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 433 437 +4

Async chunks

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

id before after diff
fleet 1.1MB 1.1MB +23.0KB

History

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

@paul-tavares paul-tavares merged commit 9521ac4 into elastic:master Nov 24, 2020
@paul-tavares paul-tavares deleted the task/fleet-82485-integrations-policies-list branch November 24, 2020 14:43
paul-tavares added a commit that referenced this pull request Nov 24, 2020
* Show table with list of Integration polices
* hook to support list pagination that uses/persist to the URL
rylnd added a commit to rylnd/kibana that referenced this pull request Nov 24, 2020
* master: (41 commits)
  [Maps] fix code-owners (elastic#84265)
  [@kbn/utils] Clean target before build (elastic#84253)
  [code coverage] collect for oss integration tests (elastic#83907)
  [APM] Use `asTransactionRate` consistently everywhere (elastic#84213)
  Attempt to fix incremental build error (elastic#84152)
  Unskip "Copy dashboards to space" (elastic#84115)
  Remove expressions.legacy from README (elastic#79681)
  Expression: Add render mode and use it for canvas interactivity (elastic#83559)
  [deb/rpm] Move systemd service to /usr/lib/systemd/system (elastic#83571)
  [Security Solution][Resolver] Allow a configurable entity_id field (elastic#81679)
  [ML] Space permision checks for job deletion (elastic#83871)
  [build] Provide ARM build of RE2 (elastic#84163)
  TSVB should use "histogram:maxBars" and "histogram:barTarget" settings for auto instead of a default 100 buckets (elastic#83628)
  [Workplace Search] Initial rendering of Org Sources (elastic#84164)
  update geckodriver to 0.28 (elastic#84085)
  Fix timelion vis escapes single quotes (elastic#84196)
  [Security Solution] Fix incorrect time for dns histogram (elastic#83532)
  [DX] Bump TS version to v4.1 (elastic#83397)
  [Security Solution] Add endpoint policy revision number (elastic#83982)
  [Fleet] Integration Policies List view (elastic#83634)
  ...
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:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0 v8.0.0
Projects
None yet
7 participants