[APM] Add Agent Keys in APM settings - Create agent keys #120373

gbamparop · 2021-12-03T16:26:32Z

Summary

This PR adds the following:

Flyout to create agent keys
Callout to copy the Base64 token for the key created
Public API for creating agent keys

Agent keys table was added in #119543

There will be other PRs for the below:

Functionality to create agent keys
Documentation update
Stories and tests

Addresses part of #77966

Screen.Recording.2021-12-03.at.17.10.05.mov

gbamparop · 2021-12-03T16:27:12Z

@kibanamachine merge upstream

kibanamachine · 2021-12-03T16:27:14Z

merge conflict between base and head

x-pack/plugins/apm/server/routes/agent_keys/create_agent_key.ts

gbamparop · 2021-12-03T16:42:27Z

x-pack/plugins/apm/server/routes/agent_keys/create_agent_key.ts

+      .filter((x) => !x[1])
+      .map((x) => x[0])
+      .join(', ');
+    const error = `${username} is missing the following requested privilege(s): ${missingPrivileges}.\


I believe it makes sense to show this error in the UI as this requirement doesn't seem to be documented. Is it ok if we display whichever error is thrown from this route or hardcode the text in the UI as well?

@cauemarcondes @sqren what do you think?

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx

gbamparop · 2021-12-03T17:15:42Z

@formgeist could you please have a look at this and #119543 ?

elasticmachine · 2021-12-03T17:18:05Z

Pinging @elastic/apm-ui (Team:apm)

gbamparop · 2021-12-06T11:13:47Z

@kibanamachine merge upstream

formgeist

Overall LGTM - I've added some copy comments and suggestions to the layout in some scenarios

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx

...plugins/apm/public/components/app/Settings/agent_keys/create_agent_key/agent_key_callout.tsx

x-pack/plugins/apm/public/components/app/Settings/agent_keys/index.tsx

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx

Co-authored-by: Casper Hübertz <casper@formgeist.com>

formgeist · 2021-12-06T12:37:57Z

@gbamparop Another request - let me know if you'd rather have a separate issue for this... But I reckon we should move the agent keys closer to the agent configuration in the list of tabs in Settings. Also follows better the alphabetical sorting that (unintentionally) is happening there.

…llout when invalidating a key

gbamparop · 2021-12-06T14:00:38Z

@formgeist thanks for the feedback! All comments have been addressed in this PR

formgeist

LGTM 🚢

cauemarcondes

Nice work @gbamparop just added a few comments

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx

...plugins/apm/public/components/app/Settings/agent_keys/create_agent_key/agent_key_callout.tsx

cauemarcondes · 2021-12-06T15:49:13Z

...plugins/apm/public/components/app/Settings/agent_keys/create_agent_key/agent_key_callout.tsx

+            </EuiCopy>
+          }
+        >
+          <input


Any reason why you didn't use <EuiFieldText/> here?

I was following the UI pattern from api keys management with a custom EuiFormControlLayout so we can use the prepend and append elements. I could change it if you think there's a better solution

This is addressed in #120765

x-pack/plugins/apm/public/components/app/Settings/agent_keys/index.tsx

x-pack/plugins/apm/server/routes/agent_keys/create_agent_key.ts

sorenlouv · 2021-12-07T12:53:08Z

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx

+  const [keyName, setKeyName] = useState('');
+  const [agentConfigChecked, setAgentConfigChecked] = useState(true);
+  const [eventWriteChecked, setEventWriteChecked] = useState(true);
+  const [sourcemapChecked, setSourcemapChecked] = useState(true);


WDYT about combining these into a single state?

Suggested change

const [keyName, setKeyName] = useState('');

const [agentConfigChecked, setAgentConfigChecked] = useState(true);

const [eventWriteChecked, setEventWriteChecked] = useState(true);

const [sourcemapChecked, setSourcemapChecked] = useState(true);

const [agentKeyBody, setAgentKeyBody] = useState<AgentKeyBody>({

name: '',

sourcemap: true,

event: true,

agentConfig: true

});

It's a matter of preference and code consistency when working with forms, I normally use different states but happy to change it if a single state is preferred

Changed to single state in #120765

gbamparop · 2021-12-07T13:31:35Z

@elasticmachine merge upstream

gbamparop · 2021-12-07T13:32:40Z

@cauemarcondes @sqren thanks for the feedback! I have addressed most of the changes, please let me know what you think.

kibana-ci · 2021-12-07T15:07:50Z

💛 Build succeeded, but was flaky

Test Failures

[job] [logs] Default CI Group #1 / APM API tests trial apm_mappings_only_8.0.0 fetching service anomalies with a trial license with ml jobs "before all" hook for "returns a 200 for a user with access to ML"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1172	1174	+2

Async chunks

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

id	before	after	diff
`apm`	2.8MB	2.8MB	+5.5KB

History

💚 Build #11728 succeeded cf7052b
💔 Build #11696 failed 5d906de
💔 Build #11682 failed abcb497
💔 Build #11678 failed 7fc9deb
💚 Build #11319 succeeded 2d30f02
💚 Build #11271 succeeded 8f55489

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

cauemarcondes

LGTM, great work!

) * Add support for creating agent keys Co-authored-by: Casper Hübertz <casper@formgeist.com> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

kibanamachine · 2021-12-07T16:37:17Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

…120652) * Add support for creating agent keys Co-authored-by: Casper Hübertz <casper@formgeist.com> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com> Co-authored-by: Giorgos Bamparopoulos <georgios.bamparopoulos@elastic.co> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

) * Add support for creating agent keys Co-authored-by: Casper Hübertz <casper@formgeist.com> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

sorenlouv · 2021-12-08T19:19:42Z

x-pack/plugins/apm/server/routes/agent_keys/create_agent_key.ts

+  if (!sourcemap && !event && !agentConfig) {
+    privileges.push(
+      PrivilegeType.SOURCEMAP,
+      PrivilegeType.EVENT,
+      PrivilegeType.AGENT_CONFIG
+    );
+  }


@gbamparop Just noticing this. So if all 3 are false, we'll add the privileges for all of them anyway. Why?

Changed in #120765

sorenlouv · 2021-12-08T19:20:53Z

x-pack/plugins/apm/server/routes/agent_keys/route.ts

+      t.partial({
+        sourcemap: toBooleanRt,
+        event: toBooleanRt,
+        agentConfig: toBooleanRt,
+      }),


As mentioned in the docs, I think this should simply take a list of the privileges, restricted to the 3 APM privileges.

Changed in #120765

bmorelli25 · 2021-12-09T01:22:07Z

Sorry, I too am late to the party 🙃

With Elastic Agent in the picture, I wonder if we should be more descriptive with our UI text. The word "agent" appears a lot on this screen, but it might not be clear to users whether we're referring to Elastic Agent, APM agents, or both. Sure, we're in the APM app, but I don't think that's reason enough for users to assume we're talking about APM agents.

What if we changed the buttons to each say, Create APM agent key, and changed the description to say, Create APM agent keys to authorize APM agent requests to the APM Server? With that being said, I can see a case for renaming all ..agent... instances to ...APM agent.., except for maybe the tab names at the top.

formgeist · 2021-12-09T08:07:49Z

@bmorelli25 Very good points, I agree we can be more explicit.

What if we changed the buttons to each say, Create APM agent key, and changed the description to say, Create APM agent keys to authorize APM agent requests to the APM Server? With that being said, I can see a case for renaming all ..agent... instances to ...APM agent.., except for maybe the tab names at the top.

Makes sense to me - will you also look into aligning this in the other tab contents for agent config etc.?

gbamparop · 2021-12-09T13:17:58Z

@bmorelli25 thanks for the feedback, I've changed all ...agent... instances apart from the tab in #120765

) * Add support for creating agent keys Co-authored-by: Casper Hübertz <casper@formgeist.com> Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

Add support for creating agent keys

0ac9f23

gbamparop force-pushed the agent-key-management branch from 68fd4fb to 0ac9f23 Compare December 3, 2021 16:37

gbamparop commented Dec 3, 2021

View reviewed changes

x-pack/plugins/apm/server/routes/agent_keys/create_agent_key.ts Outdated Show resolved Hide resolved

gbamparop commented Dec 3, 2021

View reviewed changes

x-pack/plugins/apm/public/components/app/Settings/agent_keys/create_agent_key.tsx Show resolved Hide resolved

gbamparop added auto-backport Deprecated - use backport:version if exact versions are needed release_note:feature Makes this part of the condensed release notes Team:APM All issues that need APM UI Team support v8.0.0 v8.1.0 labels Dec 3, 2021

gbamparop requested review from formgeist and a team December 3, 2021 17:15

gbamparop marked this pull request as ready for review December 3, 2021 17:18

Add click handler to the empty prompt button

ae64f89

Merge branch 'main' into agent-key-management

538ca3e

formgeist reviewed Dec 6, 2021

View reviewed changes

Apply suggestions from code review

8f55489

Co-authored-by: Casper Hübertz <casper@formgeist.com>

gbamparop mentioned this pull request Dec 6, 2021

[APM] Agent API Key Management #77966

Closed

2 tasks

gbamparop requested a review from a team December 6, 2021 12:16

gbamparop added 2 commits December 6, 2021 15:52

Move agent keys tab after agent configuration

de2585e

Hide the 'create agent' button when there are no keys / Remove the ca…

2d30f02

…llout when invalidating a key

formgeist approved these changes Dec 6, 2021

View reviewed changes

cauemarcondes requested changes Dec 6, 2021

View reviewed changes

gbamparop requested review from sorenlouv and cauemarcondes December 7, 2021 12:49

sorenlouv reviewed Dec 7, 2021

View reviewed changes

Move Agent keys page content to another component

3571120

Merge branch 'main' into agent-key-management

147ca68

This was referenced Dec 7, 2021

Docs: Update documentation for managing API keys to authorize requests to the APM Server elastic/apm-server#6844

Closed

[APM] docs: Add documentation for the agent keys API in the APM app #120622

Closed

cauemarcondes approved these changes Dec 7, 2021

View reviewed changes

gbamparop merged commit 79e013f into elastic:main Dec 7, 2021

kibanamachine mentioned this pull request Dec 7, 2021

[8.0] [APM] Add Agent Keys in APM settings - Create agent keys (#120373) #120652

Merged

gbamparop deleted the agent-key-management branch December 8, 2021 12:19

sorenlouv reviewed Dec 8, 2021

View reviewed changes

sorenlouv added the apm:test-plan-8.0.0 label Dec 21, 2021

MiriamAparicio self-assigned this Dec 22, 2021

MiriamAparicio added the apm:test-plan-done Pull request that was successfully tested during the test plan label Dec 22, 2021

gbamparop mentioned this pull request Jan 17, 2022

[APM] Add tests for agent key management #120796

Open

11 tasks

axw mentioned this pull request Oct 26, 2022

Deprecate apm-server apikey elastic/apm-server#9444

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Add Agent Keys in APM settings - Create agent keys #120373

[APM] Add Agent Keys in APM settings - Create agent keys #120373

gbamparop commented Dec 3, 2021 •

edited

Loading

gbamparop commented Dec 3, 2021

kibanamachine commented Dec 3, 2021

gbamparop Dec 3, 2021

gbamparop Dec 7, 2021

gbamparop commented Dec 3, 2021

elasticmachine commented Dec 3, 2021

gbamparop commented Dec 6, 2021

formgeist left a comment

formgeist commented Dec 6, 2021

gbamparop commented Dec 6, 2021

formgeist left a comment

cauemarcondes left a comment

cauemarcondes Dec 6, 2021 •

edited

Loading

gbamparop Dec 7, 2021

gbamparop Dec 9, 2021

sorenlouv Dec 7, 2021 •

edited

Loading

gbamparop Dec 7, 2021

gbamparop Dec 9, 2021

gbamparop commented Dec 7, 2021

gbamparop commented Dec 7, 2021

kibana-ci commented Dec 7, 2021

cauemarcondes left a comment

kibanamachine commented Dec 7, 2021

sorenlouv Dec 8, 2021 •

edited

Loading

gbamparop Dec 9, 2021

sorenlouv Dec 8, 2021

gbamparop Dec 9, 2021

bmorelli25 commented Dec 9, 2021 •

edited

Loading

formgeist commented Dec 9, 2021

gbamparop commented Dec 9, 2021

-  const [keyName, setKeyName] = useState('');
-  const [agentConfigChecked, setAgentConfigChecked] = useState(true);
-  const [eventWriteChecked, setEventWriteChecked] = useState(true);
-  const [sourcemapChecked, setSourcemapChecked] = useState(true);
+  const [agentKeyBody, setAgentKeyBody] = useState<AgentKeyBody>({
+    name: '',
+    sourcemap: true,
+    event: true,
+    agentConfig: true
+  });

[APM] Add Agent Keys in APM settings - Create agent keys #120373

[APM] Add Agent Keys in APM settings - Create agent keys #120373

Conversation

gbamparop commented Dec 3, 2021 • edited Loading

Summary

gbamparop commented Dec 3, 2021

kibanamachine commented Dec 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbamparop commented Dec 3, 2021

elasticmachine commented Dec 3, 2021

gbamparop commented Dec 6, 2021

formgeist left a comment

Choose a reason for hiding this comment

formgeist commented Dec 6, 2021

gbamparop commented Dec 6, 2021

formgeist left a comment

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

cauemarcondes Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbamparop commented Dec 7, 2021

gbamparop commented Dec 7, 2021

kibana-ci commented Dec 7, 2021

💛 Build succeeded, but was flaky

Test Failures

Metrics [docs]

Module Count

Async chunks

History

cauemarcondes left a comment

Choose a reason for hiding this comment

kibanamachine commented Dec 7, 2021

💚 Backport successful

sorenlouv Dec 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmorelli25 commented Dec 9, 2021 • edited Loading

formgeist commented Dec 9, 2021

gbamparop commented Dec 9, 2021

gbamparop commented Dec 3, 2021 •

edited

Loading

cauemarcondes Dec 6, 2021 •

edited

Loading

sorenlouv Dec 7, 2021 •

edited

Loading

sorenlouv Dec 8, 2021 •

edited

Loading

bmorelli25 commented Dec 9, 2021 •

edited

Loading