[APM] Use callWithInternalUser for agent configuration endpoints #50211

dgieselaar · 2019-11-11T20:49:38Z

Closes #50050.

elasticmachine · 2019-11-11T21:59:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a89032c6a0e57a8e21038057511e96c7f570e407

...r/lib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts

elasticmachine · 2019-11-12T12:01:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 5b7057bb00a3c3e1942a7a3475e8cf14548e48f0

sorenlouv · 2019-11-12T13:34:26Z

x-pack/legacy/plugins/apm/server/lib/helpers/es_client.ts

Looking forward to partial application coming to javascript. Will enable us to do cluster.callWithRequest(req, ?, ?). Until then, bind is our friend :)

sorenlouv · 2019-11-12T13:37:11Z

x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.ts

I think you should just leave the name as clientAsInternalUser. I find having multiple names for the same thing is often (not always) more confusing that it does good

... and I think asInternalUser everywhere is better than clientAsInternalUser everywhere

sorenlouv · 2019-11-12T13:58:22Z

x-pack/legacy/plugins/apm/server/routes/settings/agent_configuration.ts

Down the line do you think setupRequest should also expose a savedObjects client which takes clientAsInternalUser into account?

Maybe! But I'm not yet 100% sold on the one-role-per-request approach yet 😬

sorenlouv

LGTM - only nits 👍

elasticmachine · 2019-11-12T17:11:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 27ed01c50da39d3cb8a4c14c3e5e8bb08b0d422f

dgieselaar · 2019-11-12T20:21:21Z

Decided on zoom/slack after talking to @kobelb to only use callWithInternalUser for the agent configuration index, and callWithRequest for our data indices (apm-*).

sorenlouv

Latest changes look good too 👍

sorenlouv · 2019-11-12T20:38:26Z

x-pack/legacy/plugins/apm/server/routes/create_api/index.test.ts

Maybe make a helper to make this more readable:

const thirdRoute = getRouteByIndex(2)

Or if you wanna get real fancy:

const thirdRoute = getRoute('PUT', '/baz')

... or leave it as-is. Probably not worth it.

elasticmachine · 2019-11-12T21:41:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 10fb539e506649eaec751cbc5cda2973feac2f36

kobelb · 2019-11-12T22:30:52Z

x-pack/legacy/plugins/apm/server/routes/settings/agent_configuration.ts

There are a few routes in here without any access: tags:

path: '/api/apm/settings/agent-configuration',

method: 'GET', path: '/api/apm/settings/agent-configuration/services',

path: '/api/apm/settings/agent-configuration/environments',

path: '/api/apm/settings/agent-configuration/agent_name',

method: 'POST', path: '/api/apm/settings/agent-configuration/search',

Are there defaults being supplied somewhere?

dgieselaar · 2019-11-12T22:39:26Z

Yes, the default is ` [ 'access:apm'] `. (would love to point you to the file, but on my phone. I think in server/routes/create_api/index.ts). Op di 12 nov. 2019 23:36 schreef Brandon Kobel <[email protected]>:

…

***@***.**** commented on this pull request. ------------------------------ In x-pack/legacy/plugins/apm/server/routes/settings/agent_configuration.ts <#50211 (comment)>: > @@ -31,6 +31,9 @@ export const agentConfigurationRoute = createRoute(core => ({ export const deleteAgentConfigurationRoute = createRoute(() => ({ There are a few routes in here without any access: tags: path: '/api/apm/settings/agent-configuration', method: 'GET', path: '/api/apm/settings/agent-configuration/services', path: '/api/apm/settings/agent-configuration/environments', path: '/api/apm/settings/agent-configuration/agent_name', method: 'POST', path: '/api/apm/settings/agent-configuration/search', Are there default being supplied somewhere? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#50211?email_source=notifications&email_token=AACWDXB4QATJSBPGKYOC2LLQTMVVPA5CNFSM4JL2T6NKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLKBR3A#pullrequestreview-315889900>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACWDXBLZSFANPABDTQHPF3QTMVVPANCNFSM4JL2T6NA> .

kobelb · 2019-11-12T22:43:23Z

Thanks @dgieselaar, I see it there now!

elasticmachine · 2019-11-13T09:29:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9ab574d5a81bec7e540a66c1214f4964746bdd84

elasticmachine · 2019-11-13T11:48:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: fa7628ef8f21c824045a0242bfa10914a9bded9f

Closes elastic#50050.

elasticmachine · 2019-11-13T14:34:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: fa0606b

…tic#50211) * [APM] Use callWithInternalUser for agent configuration endpoints Closes elastic#50050. * Review feedback * Use internalClient for agent conf queries only

dgieselaar requested a review from a team as a code owner November 11, 2019 20:49

dgieselaar added release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Nov 11, 2019

sorenlouv reviewed Nov 11, 2019

View reviewed changes

...r/lib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts Outdated Show resolved Hide resolved

dgieselaar force-pushed the secure-agent-endpoints branch from a89032c to 5b7057b Compare November 12, 2019 10:55

sorenlouv reviewed Nov 12, 2019

View reviewed changes

sorenlouv approved these changes Nov 12, 2019

View reviewed changes

dgieselaar requested a review from kobelb November 12, 2019 20:20

dgieselaar force-pushed the secure-agent-endpoints branch from 27ed01c to 10fb539 Compare November 12, 2019 20:20

sorenlouv approved these changes Nov 12, 2019

View reviewed changes

sorenlouv reviewed Nov 12, 2019

View reviewed changes

kobelb reviewed Nov 12, 2019

View reviewed changes

kobelb approved these changes Nov 12, 2019

View reviewed changes

dgieselaar force-pushed the secure-agent-endpoints branch from 10fb539 to 9ab574d Compare November 13, 2019 08:07

dgieselaar force-pushed the secure-agent-endpoints branch from 9ab574d to fa7628e Compare November 13, 2019 10:32

dgieselaar added 3 commits November 13, 2019 14:14

[APM] Use callWithInternalUser for agent configuration endpoints

cf1b6e7

Closes elastic#50050.

Review feedback

67b652f

Use internalClient for agent conf queries only

fa0606b

dgieselaar force-pushed the secure-agent-endpoints branch from fa7628e to fa0606b Compare November 13, 2019 13:15

dgieselaar merged commit f317c25 into elastic:master Nov 13, 2019

dgieselaar deleted the secure-agent-endpoints branch November 13, 2019 14:42

dgieselaar mentioned this pull request Nov 13, 2019

[7.x] [APM] Use callWithInternalUser for agent configuration endpoin… (#50211) #50488

Merged

dgieselaar mentioned this pull request Nov 13, 2019

[7.5] [APM] Use callWithInternalUser for agent configuration endpoin… (#50211) #50498

Merged

[APM] Use callWithInternalUser for agent configuration endpoints #50211

[APM] Use callWithInternalUser for agent configuration endpoints #50211

Uh oh!

Conversation

dgieselaar commented Nov 11, 2019

Uh oh!

elasticmachine commented Nov 11, 2019

💚 Build Succeeded

Uh oh!

Uh oh!

elasticmachine commented Nov 12, 2019

💔 Build Failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sorenlouv left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 12, 2019

💔 Build Failed

Uh oh!

dgieselaar commented Nov 12, 2019

Uh oh!

sorenlouv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 12, 2019

💔 Build Failed

Uh oh!

kobelb Nov 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgieselaar commented Nov 12, 2019 via email

Uh oh!

kobelb commented Nov 12, 2019

Uh oh!

elasticmachine commented Nov 13, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Nov 13, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Nov 13, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kobelb Nov 12, 2019 •

edited

Loading