Create the ftrSoApis FTR plugin #149188

pgayvallet · 2023-01-19T09:16:02Z

Summary

More and more SO types will not be accessible from the HTTP APIs (either hidden:true or hiddenFromHTTPApis: true).

However, the FTR SO client (KbnClientSavedObjects) still needs to be able to access and manipulate all SO types.

This PR introduces a ftrSoApis plugin that is loaded for all FTR suites. This plugin exposes SO APIs that are used by the FTR client instead of the public SO HTTP APIs. These APIs are configured to know about all types, even hidden ones.

…fix'

elasticmachine · 2023-01-19T16:27:28Z

Pinging @elastic/kibana-core (Team:Core)

spalger

At first I thought this sounded like a great idea, but then I remembered how much testing we do against ESS Kibana Instances, which won't and can't have the ftrSoApis plugin. If we make this change in this way, all tests which interact with saved objects will not be runnable against production instances. Additionally, I'm really concerned about the amount of coverage we're shifting away from the public saved objects APIs and to internal versions.

IMO the ideal way for functional tests to run is with APIs which are actually publicly available, even if they require special roles or something that are only enabled in staging environments or something. Is there any other option here that doesn't have these downsides?

TinaHeiligers · 2023-01-19T18:51:19Z

Is there any other option here that doesn't have these downsides?

We discussed our options in #147150 (comment) and the pros and cons of each, keeping timing in mind too.

Of these, this PR takes the less ideal approach but we were hoping it would work, at least, for now.
The other options are to add an override to each public SO HTTP API but that would "leak" dev requirements into Prod.

TinaHeiligers · 2023-01-19T22:59:01Z

APIs which are actually publicly available, even if they require special roles or something that are only enabled in staging environments or something

ATM, Core doesn't know about the user making a request, and restricting access to such 'public but not normally accessible' APIs could be tricky to implement directly in Core. Are you thinking about some public plugin that's restricted to a special role and hidden behind a tag? This would map to something along the lines that @rudolf suggested:

In a dedicated plugin only enabled during testing
With a tag similar to 'access:migrateSavedObjects' to prevent production usage

I'm really not too sure how we're going to prevent dev requirements from leaking into Prod. @lukeelmers can you think of a novel approach?

spalger · 2023-01-23T03:32:09Z

I'm really not too sure how we're going to prevent dev requirements from leaking into Prod

Avoiding this is the problem. We run our tests against production instances, which means that we can't have test-only APIs

pgayvallet · 2023-01-23T07:13:40Z

I remembered how much testing we do against ESS Kibana Instances

Yeah... This is absolutely correct.

I thought about it this week-end tbh. We talked about it a while ago but somehow forgot about it when we opened the dedicated #148412 issue.

Is there any other option here that doesn't have these downsides?

I'm going to start by saying that the saved object APIs will eventually be removed in a not-so-far futur. That is a mere fact, and we need to find a solution. Just declining all our options isn't doing to do anything.

Now,

We could effectively expose these test only APIs in a 'standard'/'normal' plugin (just move the plugin introduced in this PR to /src/plugins), so that they are automatically available in any production environment. I kinda dislike it, but it would work.

It would have to upside to switch from using 'public' (/api/) endpoint in favor of internal (/internal) ones, so even if exposed on all deployments, we wouldn't have to consider those FTR-specific endpoint as public.

In term of security it would be fine too, standard SO security would apply, so a user would only be able to manipulate the types they have access to.

We could also eventually disable that plugin by default, so that all Cloud would need to do it to enable it during testing (via a simple configuration property, e.g ftrApis.enable: true). It doesn't sounds like that much of a burden, so maybe it would be acceptable for Cloud? We probably need to confirm that with them though.

even if they require special roles or something that are only enabled in staging environments or something

@spalger I though you just said these tests were supposed to be able to run on any production environment?

I'm really concerned about the amount of coverage we're shifting away from the public saved objects APIs and to internal versions

The 'special' APIs are using the SOR under the hood. We're not shifting that much, so I'm not really sure to see how this is an issue. We're only adapting the underlying implementation of one of FTR's service. Mind to elaborate what your concerns are about?

cc @rudolf @lukeelmers : do you know who could help on Cloud side to decide if a solution is acceptable for them or not? Do you think it would be acceptable to just have these FTR apis be exposed from a normal plugin and always enabled (which is basically our easiest option while still being secured)? Do we need to do more?

pgayvallet · 2023-01-23T16:44:32Z

So we discussed with @spalger sync, and he's fine having these dedicated FTR APIs as long as they can easily (understand: with a config setting) be disabled or enabled.

Note that in his opinion, he thinks it would make more sense to have those APIs exposed from Core rather than from a plugin, but he would be fine either way.

The only remaining question now is regarding if this 'FTR APIs' plugin should be enabled by default, or if it would be acceptable to have it disabled by default and have Cloud enable it for testing.

Note that we could start with this plugin being just enabled to avoid breaking anything, so that this PR get merged faster, and then reach out to cloud to figure out if we can switch to have it disabled by default.

lukeelmers · 2023-01-23T18:36:24Z

I think we all agree none of our options here are great, but the point about testing on ESS deployments is definitely something we overlooked. The approach Pierre outlined seems reasonable (have a separate plugin exposing APIs that are specifically marked as internal). I definitely prefer the idea of having something disable-able -- even if it is only done as a follow up step -- so that those routes aren't registered on every deployment.

I know there is already a thread discussing how much of a lift this would be on the QA side to configure our cloud FTR runs to enable a special plugin. To prevent confusion in the case of someone trying to run ad hoc tests on a miscellaneous ESS deployment (which tbh I don't know how often we do this today), it might be helpful if we found a way to write the tests so they can fail with a specific error if they detect the plugin isn't available. "You need to set foo.enabled on this deployment before running tests against it"

It'd make me nervous to do any of this as a long-term/permanent solution, but as this is only intended to exist as an aid to help folks transition to the new SO architecture in the mid-term, I think the tradeoffs are okay.

pgayvallet · 2023-01-25T07:30:44Z

@elasticmachine merge upstream

…ocs'

…-fix'

spalger

I think I've made my points clear, I'll leave the decision making up to Core and just say that the operations specific code changes LGTM

TinaHeiligers

Code looks great, thanks for taking care of this!
I've left a few comments/questions but nothing that halts merging.

TinaHeiligers · 2023-01-25T17:17:57Z

src/plugins/ftr_apis/README.md

@@ -0,0 +1,3 @@
+# ftrApis plugin
+
+Set of APIs used internally by the FTR.


nit: We should be more specific about what APIs these are and that they are only for ftr testing. e.g.
Set of APIs for the kbn server's saved object's client used internally by the functional test runner

or, more concisely:
Set of kbnServer saved objects' client APIs used internally by the functional test runner

TinaHeiligers · 2023-01-25T17:43:40Z

src/plugins/ftr_apis/server/routes/kbn_client_so/bulk_delete.ts

+        ),
+      },
+    },
+    catchAndReturnBoomErrors(async (ctx, req, res) => {


FYI: I asked about catchAndReturnBoomErrors and apparently it should be dead / unecessary code and safe to remove. We're probably less likely to replace these APIs in the short to medium term so should we remove it?

Yeah, I wanted to, but it's actually used by the FTR KbnClient. Removing it causes some errors to not be properly wrapped, causing a lot of test suites to fail.

TinaHeiligers · 2023-01-25T19:41:14Z

src/plugins/ftr_apis/server/routes/kbn_client_so/index.ts

+import { registerGetRoute } from './get';
+import { registerUpdateRoute } from './update';
+
+export const registerKbnClientSoRoutes = (router: IRouter) => {


If this plugin sticks around for a while, should we consider versioning it? Even if we keep the plugin HTTP API's fixed at some particular Kibana version, it's better than some arbitrary API that might be interpreted to "work" for all time.

In theory, internal APIs won't need versioning, at least for now (plus, we're still waiting for a spec for API versioning...).

…client-plugin

…ocs'

kibana-ci · 2023-01-26T08:47:30Z

💚 Build Succeeded

Buildkite Build
Commit: ab041e7

Metrics [docs]

✅ unchanged

History

💚 Build #102618 succeeded f9f1e1f
💚 Build #102480 succeeded f24e7d4
💔 Build #102457 failed 6bfd5a5
💚 Build #102433 succeeded 12c6470
💚 Build #101467 succeeded 9384667
💛 Build #101396 was flaky 13db670

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

## Summary Follow-up of #149188 - Use the bulkDelete API for `KbnClientSavedObjects.bulkDelete` - Create a dedicated `/_clean` endpoint for `KbnClientSavedObjects.clean` and `KbnClientSavedObjects.cleanStandardList` --------- Co-authored-by: kibanamachine <[email protected]>

## Summary Fix elastic#148412 More and more SO types will not be accessible from the HTTP APIs (either `hidden:true` or `hiddenFromHTTPApis: true`). However, the FTR SO client (`KbnClientSavedObjects`) still needs to be able to access and manipulate all SO types. This PR introduces a `ftrSoApis` plugin that is loaded for all FTR suites. This plugin exposes SO APIs that are used by the FTR client instead of the public SO HTTP APIs. These APIs are configured to know about all types, even hidden ones. Co-authored-by: kibanamachine <[email protected]>

## Summary Follow-up of elastic#149188 - Use the bulkDelete API for `KbnClientSavedObjects.bulkDelete` - Create a dedicated `/_clean` endpoint for `KbnClientSavedObjects.clean` and `KbnClientSavedObjects.cleanStandardList` --------- Co-authored-by: kibanamachine <[email protected]>

Create the kbnSoApis FTR plugin

fceb917

pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Saved Objects Feature:Functional Testing release_note:skip Skip the PR/issue when compiling release notes v8.7.0 labels Jan 19, 2023

pgayvallet and others added 4 commits January 19, 2023 10:31

update config base

90136ae

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

959f3fd

…fix'

use inline strings

122916b

converting boom errors

13db670

pgayvallet marked this pull request as ready for review January 19, 2023 16:27

pgayvallet requested a review from a team as a code owner January 19, 2023 16:27

pgayvallet requested a review from a team January 19, 2023 16:27

pgayvallet changed the title ~~Create the kbnSoApis FTR plugin~~ Create the ftrSoApis FTR plugin Jan 19, 2023

fix description

9384667

TinaHeiligers self-requested a review January 19, 2023 16:33

spalger suggested changes Jan 19, 2023

View reviewed changes

kibanamachine and others added 5 commits January 25, 2023 02:30

Merge branch 'main' into kbn-148412-ftr-kbn-client-plugin

12c6470

start moving plugin to src

87db010

split APIs per file

1bf8ba1

add access tag

6bfd5a5

updating config file

1c829b5

kibanamachine added 2 commits January 25, 2023 09:38

[CI] Auto-commit changed files from 'node scripts/build_plugin_list_d…

74e2717

…ocs'

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

f24e7d4

…-fix'

spalger approved these changes Jan 25, 2023

View reviewed changes

pgayvallet added 2 commits January 25, 2023 15:54

preparing api integ tests

63cafed

adapt the FTR tests

f9f1e1f

TinaHeiligers approved these changes Jan 25, 2023

View reviewed changes

pgayvallet added 2 commits January 26, 2023 08:36

update doc

934fef3

Merge remote-tracking branch 'upstream/main' into kbn-148412-ftr-kbn-…

46a9a6b

…client-plugin

pgayvallet enabled auto-merge (squash) January 26, 2023 07:36

[CI] Auto-commit changed files from 'node scripts/build_plugin_list_d…

ab041e7

…ocs'

pgayvallet merged commit cd9a53f into elastic:main Jan 26, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jan 26, 2023

pgayvallet mentioned this pull request Jan 26, 2023

[FTR] KbnClientSavedObjects improvements #149582

Merged

yuliacech mentioned this pull request Jan 26, 2023

[Guided onboarding] Change the saved objects' type to hidden #148983

Merged

TinaHeiligers mentioned this pull request Sep 11, 2024

[http] api_integration tests handle internal route restriction #192407

Merged

1 task

jeramysoucy mentioned this pull request Nov 13, 2024

[Ownership] Assign test files to platform security team #199795

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create the ftrSoApis FTR plugin #149188

Create the ftrSoApis FTR plugin #149188

pgayvallet commented Jan 19, 2023 •

edited

Loading

elasticmachine commented Jan 19, 2023

spalger left a comment

TinaHeiligers commented Jan 19, 2023

TinaHeiligers commented Jan 19, 2023 •

edited

Loading

spalger commented Jan 23, 2023

pgayvallet commented Jan 23, 2023 •

edited

Loading

pgayvallet commented Jan 23, 2023

lukeelmers commented Jan 23, 2023

pgayvallet commented Jan 25, 2023

spalger left a comment

TinaHeiligers left a comment

TinaHeiligers Jan 25, 2023

TinaHeiligers Jan 25, 2023

pgayvallet Jan 26, 2023

TinaHeiligers Jan 25, 2023

pgayvallet Jan 26, 2023 •

edited

Loading

kibana-ci commented Jan 26, 2023

		@@ -0,0 +1,3 @@
		# ftrApis plugin

		Set of APIs used internally by the FTR.

Create the ftrSoApis FTR plugin #149188

Create the ftrSoApis FTR plugin #149188

Conversation

pgayvallet commented Jan 19, 2023 • edited Loading

Summary

elasticmachine commented Jan 19, 2023

spalger left a comment

Choose a reason for hiding this comment

TinaHeiligers commented Jan 19, 2023

TinaHeiligers commented Jan 19, 2023 • edited Loading

spalger commented Jan 23, 2023

pgayvallet commented Jan 23, 2023 • edited Loading

pgayvallet commented Jan 23, 2023

lukeelmers commented Jan 23, 2023

pgayvallet commented Jan 25, 2023

spalger left a comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

TinaHeiligers Jan 25, 2023

Choose a reason for hiding this comment

TinaHeiligers Jan 25, 2023

Choose a reason for hiding this comment

pgayvallet Jan 26, 2023

Choose a reason for hiding this comment

TinaHeiligers Jan 25, 2023

Choose a reason for hiding this comment

pgayvallet Jan 26, 2023 • edited Loading

Choose a reason for hiding this comment

kibana-ci commented Jan 26, 2023

💚 Build Succeeded

Metrics [docs]

History

pgayvallet commented Jan 19, 2023 •

edited

Loading

TinaHeiligers commented Jan 19, 2023 •

edited

Loading

pgayvallet commented Jan 23, 2023 •

edited

Loading

pgayvallet Jan 26, 2023 •

edited

Loading