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] Improve UX for policy secrets #171405

Merged
merged 10 commits into from
Nov 16, 2023

Conversation

kpollich
Copy link
Member

@kpollich kpollich commented Nov 16, 2023

Summary

Closes #171225

  • Highlights secrets during package policy creation with a distinct background and icon
  • Add tooltip + docs link for secrets where appropriate
  • Detect "new secrets" during policy upgrade and alert the user in a separate callout

To do

  • Fix any failing tests
  • Add tests for "new secrets" detection logic

Screenshots

image

image

image

How to test

There's probably an easier way to do this, but this is what I did

  1. Clone https://github.com/elastic/package-registry and https://github.com/elastic/integrations
  2. Add the following to config.yml in your package-registry repo
package_paths:
  - path/to/your/integrations/build/packages
  1. Build a version of an integration with some secrets: true for various variables. I used 1password
cd integrations/packages/1password
# Edit `manifest.yml` or a given `data_stream/*/manifest.yml` file to change some variables to `secret: true`. Also bump the version and update `changelog.yml`
elastic-package build
  1. Run the local package registry e.g.
cd package-registry
go run . --feature-proxy-mode=true -proxy-to=https://epr.elastic.co # makes it so you can still see EPR packages in Kibana
  1. Update your kibana.dev.yml to point at your local package registry
xpack.fleet.registryUrl: http://localhost:8080
  1. Start Kibana and Elasticsearch and install, upgrade, etc your package in question to verify the changes

- Highlight secrets in policy editor "create" mode with a special background and tooltip
- Add docs link where appropriate
- Highlight "new secrets" in a separate callout during policy upgrade
@kpollich kpollich added release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 16, 2023
@kpollich kpollich self-assigned this Nov 16, 2023
@kpollich kpollich requested review from a team as code owners November 16, 2023 14:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@kilfoyle kilfoyle 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 text and the UI docs link. 👍

…policy/create_package_policy_page/components/steps/components/package_policy_input_var_field.tsx

Co-authored-by: David Kilfoyle <41695641+kilfoyle@users.noreply.github.com>
@@ -117,6 +117,44 @@ export const EditPackagePolicyForm = memo<{

const canWriteIntegrationPolicies = useAuthz().integrations.writeIntegrationPolicies;

const newSecrets = useMemo(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be great to move that to an util function out of that already huge file, so we can better separate/unit test that logic what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 286b223

const [currentPackagePolicy, proposedUpgradePackagePolicy] = dryRunData[0].diff || [];
const isReadyForUpgrade = currentPackagePolicy && !dryRunData[0].hasErrors;

const HasNewSecretsCallOut = () => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: I would prefer to have all those callout extracted to there own components (could still be in the same file but outside of that UpgradeStatusCallout) in my opinion it make things clearer

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 7c6091a

@kpollich
Copy link
Member Author

After some chats with the EUI team and Kuldeep, here's what the new secrets interface looks like on the policy editor:

image

@kpollich
Copy link
Member Author

I've added the same label/tooltip treatment to the "Edit" view of secrets as well
image

@kpollich
Copy link
Member Author

One last update here, we've moved the "learn more" link to the bottom of the EuiPanel that contains the secret field, e.g.

image

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.

Thanks for addressing my comments, tested locally and LGTM 🚀

@kpollich kpollich enabled auto-merge (squash) November 16, 2023 18:45
@kilfoyle
Copy link
Contributor

The latest UI changes (the tooltips and the moved docs link) all look good to me!

@kpollich kpollich merged commit 9396ef3 into elastic:main Nov 16, 2023
28 checks passed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 946 947 +1

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.2MB 1.2MB +3.9KB
lists 147.9KB 148.0KB +65.0B
securitySolution 12.8MB 12.8MB +6.4KB
securitySolutionServerless 330.6KB 336.9KB +6.3KB
total +16.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 372.7KB 372.8KB +65.0B
securitySolutionServerless 41.7KB 41.7KB -1.0B
total +64.0B

History

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

cc @kpollich

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.11 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.11:
- [EDR Workflows] Protection updates copy change (#171318)

Manual backport

To create the backport manually run:

node scripts/backport --pr 171405

Questions ?

Please refer to the Backport tool documentation

@kpollich
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.11

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kpollich added a commit to kpollich/kibana that referenced this pull request Nov 28, 2023
## Summary

Closes elastic#171225

- Highlights secrets during package policy creation with a distinct
background and icon
- Add tooltip + docs link for secrets where appropriate
- Detect "new secrets" during policy upgrade and alert the user in a
separate callout

## To do
- [x] Fix any failing tests
- [x] Add tests for "new secrets" detection logic

## Screenshots

![image](https://github.com/elastic/kibana/assets/6766512/e943a3e8-68db-40eb-a5c3-b108e7d299ff)

![image](https://github.com/elastic/kibana/assets/6766512/751bbe50-7553-4dcc-a8dc-b9802f331013)

![image](https://github.com/elastic/kibana/assets/6766512/6cceb4cd-0b8e-42cd-aafb-d2e3ddcd23a8)

## How to test

There's probably an easier way to do this, but this is what I did

1. Clone https://github.com/elastic/package-registry and
https://github.com/elastic/integrations
2. Add the following to `config.yml` in your package-registry repo

```yml
package_paths:
  - path/to/your/integrations/build/packages
```

3. Build a version of an integration with some `secrets: true` for
various variables. I used `1password`

```shell
cd integrations/packages/1password
# Edit `manifest.yml` or a given `data_stream/*/manifest.yml` file to change some variables to `secret: true`. Also bump the version and update `changelog.yml`
elastic-package build
```

4. Run the local package registry e.g.

```shell
cd package-registry
go run . --feature-proxy-mode=true -proxy-to=https://epr.elastic.co # makes it so you can still see EPR packages in Kibana
```

5. Update your `kibana.dev.yml` to point at your local package registry

```yml
xpack.fleet.registryUrl: http://localhost:8080
```

6. Start Kibana and Elasticsearch and install, upgrade, etc your package
in question to verify the changes

---------

Co-authored-by: David Kilfoyle <41695641+kilfoyle@users.noreply.github.com>
(cherry picked from commit 9396ef3)

# Conflicts:
#	packages/kbn-doc-links/src/get_doc_links.ts
#	packages/kbn-doc-links/src/types.ts
#	x-pack/plugins/translations/translations/fr-FR.json
#	x-pack/plugins/translations/translations/ja-JP.json
#	x-pack/plugins/translations/translations/zh-CN.json
kpollich added a commit that referenced this pull request Nov 29, 2023
# Backport

This will backport the following commits from `main` to `8.11`:
- [[Fleet] Improve UX for policy secrets
(#171405)](#171405)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kyle
Pollich","email":"kyle.pollich@elastic.co"},"sourceCommit":{"committedDate":"2023-11-16T19:35:19Z","message":"[Fleet]
Improve UX for policy secrets (#171405)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/171225\r\n\r\n- Highlights
secrets during package policy creation with a distinct\r\nbackground and
icon\r\n- Add tooltip + docs link for secrets where appropriate\r\n-
Detect \"new secrets\" during policy upgrade and alert the user in
a\r\nseparate callout\r\n\r\n## To do\r\n- [x] Fix any failing
tests\r\n- [x] Add tests for \"new secrets\" detection logic\r\n\r\n##
Screenshots\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/e943a3e8-68db-40eb-a5c3-b108e7d299ff)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/751bbe50-7553-4dcc-a8dc-b9802f331013)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/6cceb4cd-0b8e-42cd-aafb-d2e3ddcd23a8)\r\n\r\n##
How to test\r\n\r\nThere's probably an easier way to do this, but this
is what I did\r\n\r\n1. Clone
https://github.com/elastic/package-registry
and\r\nhttps://github.com/elastic/integrations\r\n2. Add the following
to `config.yml` in your package-registry
repo\r\n\r\n```yml\r\npackage_paths:\r\n -
path/to/your/integrations/build/packages\r\n```\r\n\r\n3. Build a
version of an integration with some `secrets: true` for\r\nvarious
variables. I used `1password`\r\n\r\n```shell\r\ncd
integrations/packages/1password\r\n# Edit `manifest.yml` or a given
`data_stream/*/manifest.yml` file to change some variables to `secret:
true`. Also bump the version and update
`changelog.yml`\r\nelastic-package build\r\n```\r\n\r\n4. Run the local
package registry e.g. \r\n\r\n```shell\r\ncd package-registry\r\ngo run
. --feature-proxy-mode=true -proxy-to=https://epr.elastic.co # makes it
so you can still see EPR packages in Kibana\r\n```\r\n\r\n5. Update your
`kibana.dev.yml` to point at your local package
registry\r\n\r\n```yml\r\nxpack.fleet.registryUrl:
http://localhost:8080\r\n```\r\n\r\n6. Start Kibana and Elasticsearch
and install, upgrade, etc your package\r\nin question to verify the
changes\r\n\r\n---------\r\n\r\nCo-authored-by: David Kilfoyle
<41695641+kilfoyle@users.noreply.github.com>","sha":"9396ef3d6bed213b681970a4914eeb558a30ed44","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Team:Fleet","backport:prev-minor","v8.12.0"],"number":171405,"url":"https://github.com/elastic/kibana/pull/171405","mergeCommit":{"message":"[Fleet]
Improve UX for policy secrets (#171405)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/171225\r\n\r\n- Highlights
secrets during package policy creation with a distinct\r\nbackground and
icon\r\n- Add tooltip + docs link for secrets where appropriate\r\n-
Detect \"new secrets\" during policy upgrade and alert the user in
a\r\nseparate callout\r\n\r\n## To do\r\n- [x] Fix any failing
tests\r\n- [x] Add tests for \"new secrets\" detection logic\r\n\r\n##
Screenshots\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/e943a3e8-68db-40eb-a5c3-b108e7d299ff)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/751bbe50-7553-4dcc-a8dc-b9802f331013)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/6cceb4cd-0b8e-42cd-aafb-d2e3ddcd23a8)\r\n\r\n##
How to test\r\n\r\nThere's probably an easier way to do this, but this
is what I did\r\n\r\n1. Clone
https://github.com/elastic/package-registry
and\r\nhttps://github.com/elastic/integrations\r\n2. Add the following
to `config.yml` in your package-registry
repo\r\n\r\n```yml\r\npackage_paths:\r\n -
path/to/your/integrations/build/packages\r\n```\r\n\r\n3. Build a
version of an integration with some `secrets: true` for\r\nvarious
variables. I used `1password`\r\n\r\n```shell\r\ncd
integrations/packages/1password\r\n# Edit `manifest.yml` or a given
`data_stream/*/manifest.yml` file to change some variables to `secret:
true`. Also bump the version and update
`changelog.yml`\r\nelastic-package build\r\n```\r\n\r\n4. Run the local
package registry e.g. \r\n\r\n```shell\r\ncd package-registry\r\ngo run
. --feature-proxy-mode=true -proxy-to=https://epr.elastic.co # makes it
so you can still see EPR packages in Kibana\r\n```\r\n\r\n5. Update your
`kibana.dev.yml` to point at your local package
registry\r\n\r\n```yml\r\nxpack.fleet.registryUrl:
http://localhost:8080\r\n```\r\n\r\n6. Start Kibana and Elasticsearch
and install, upgrade, etc your package\r\nin question to verify the
changes\r\n\r\n---------\r\n\r\nCo-authored-by: David Kilfoyle
<41695641+kilfoyle@users.noreply.github.com>","sha":"9396ef3d6bed213b681970a4914eeb558a30ed44"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/171405","number":171405,"mergeCommit":{"message":"[Fleet]
Improve UX for policy secrets (#171405)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/171225\r\n\r\n- Highlights
secrets during package policy creation with a distinct\r\nbackground and
icon\r\n- Add tooltip + docs link for secrets where appropriate\r\n-
Detect \"new secrets\" during policy upgrade and alert the user in
a\r\nseparate callout\r\n\r\n## To do\r\n- [x] Fix any failing
tests\r\n- [x] Add tests for \"new secrets\" detection logic\r\n\r\n##
Screenshots\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/e943a3e8-68db-40eb-a5c3-b108e7d299ff)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/751bbe50-7553-4dcc-a8dc-b9802f331013)\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/6766512/6cceb4cd-0b8e-42cd-aafb-d2e3ddcd23a8)\r\n\r\n##
How to test\r\n\r\nThere's probably an easier way to do this, but this
is what I did\r\n\r\n1. Clone
https://github.com/elastic/package-registry
and\r\nhttps://github.com/elastic/integrations\r\n2. Add the following
to `config.yml` in your package-registry
repo\r\n\r\n```yml\r\npackage_paths:\r\n -
path/to/your/integrations/build/packages\r\n```\r\n\r\n3. Build a
version of an integration with some `secrets: true` for\r\nvarious
variables. I used `1password`\r\n\r\n```shell\r\ncd
integrations/packages/1password\r\n# Edit `manifest.yml` or a given
`data_stream/*/manifest.yml` file to change some variables to `secret:
true`. Also bump the version and update
`changelog.yml`\r\nelastic-package build\r\n```\r\n\r\n4. Run the local
package registry e.g. \r\n\r\n```shell\r\ncd package-registry\r\ngo run
. --feature-proxy-mode=true -proxy-to=https://epr.elastic.co # makes it
so you can still see EPR packages in Kibana\r\n```\r\n\r\n5. Update your
`kibana.dev.yml` to point at your local package
registry\r\n\r\n```yml\r\nxpack.fleet.registryUrl:
http://localhost:8080\r\n```\r\n\r\n6. Start Kibana and Elasticsearch
and install, upgrade, etc your package\r\nin question to verify the
changes\r\n\r\n---------\r\n\r\nCo-authored-by: David Kilfoyle
<41695641+kilfoyle@users.noreply.github.com>","sha":"9396ef3d6bed213b681970a4914eeb558a30ed44"}}]}]
BACKPORT-->

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team v8.11.2 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Minor secrets UX improvements
7 participants