[Fleet] using @kbn/config-schema part 2 (outputs and other apis) #193326

juliaElastic · 2024-09-18T15:08:08Z

Summary

Relates #184685
Added tests to verify response schemas.

To verify, go to Fleet settings and try create/update different output types, fleet server hosts, outputs.

GET kbn:/api/oas?pathStartsWith=/api/fleet/outputs
GET kbn:/api/oas?pathStartsWith=/api/fleet/health_check
GET kbn:/api/oas?pathStartsWith=/api/fleet/message_signing_service/rotate_key_pair
GET kbn:/api/oas?pathStartsWith=/api/fleet/fleet_server_hosts
GET kbn:/api/oas?pathStartsWith=/api/fleet/data_streams

# test APIs
POST kbn:/api/fleet/health_check
{
  "id": "default-fleet-server"
}
POST kbn:/api/fleet/message_signing_service/rotate_key_pair
GET kbn:/api/fleet/data_streams
GET kbn:/api/fleet/check-permissions
POST kbn:/api/fleet/service_tokens

obltmachine · 2024-09-18T15:08:24Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

elasticmachine · 2024-09-19T10:59:05Z

Pinging @elastic/fleet (Team:Fleet)

juliaElastic · 2024-09-19T12:23:11Z

x-pack/plugins/fleet/server/routes/schema/errors.ts

@@ -10,8 +10,8 @@ import { schema } from '@kbn/config-schema';
 export const genericErrorResponse = () =>
  schema.object(
    {
-      statusCode: schema.number(),
-      error: schema.string(),
+      statusCode: schema.maybe(schema.number()),


it seems in some cases statusCode is missing from the response, so making it optional

juliaElastic · 2024-09-19T12:29:58Z

...rity_solution/docs/openapi/ess/security_solution_timeline_api_2023_10_31.bundled.schema.yaml

@@ -131,7 +131,7 @@ paths:
                  type: string
              required:
                - note
-        description: 'The note to add or update, along with additional metadata.'
+        description: The note to add or update, along with additional metadata.


these changes are coming from an auto commit
561ff93

PhilippeOberti

code LGTM for the Threat Hunting Investigations team, though it seems that the changes have already been merged by this PR

criamico · 2024-09-20T08:25:22Z

@juliaElastic I'm testing the branch locally and found an issue related to outputs. This happens when trying to update an output from a type to another, in my case I had a Kafka output like this:

{
    "item": {
        "id": "45758e5b-92e3-4ba3-9265-44f0d2531b34",
        "name": "kafka_1",
        "type": "kafka",
        "hosts": [
            "host:2566"
        ],
        "is_default": false,
        "is_default_monitoring": false,
        "config_yaml": "",
        "client_id": "Elastic",
        "version": "1.0.0",
        "compression": "gzip",
        "auth_type": "none",
        "connection_type": "plaintext",
        "partition": "random",
        "random": {
            "group_events": 1
        },
        "topics": [
            {
                "topic": "default"
            }
        ],
        "headers": [
            {
                "key": "",
                "value": ""
            }
        ],
        "timeout": 30,
        "broker_timeout": 30,
        "required_acks": 1,
        "username": null,
        "sasl": null,
        "compression_level": 4,
        "password": null
    }
}

Then I tried to change it to a logstash one filling out only the required fields:
. When saving, it fails with this error:

I also tried saving it as ES output and this time it fails with this error:

So I think there are generally issues with switching the type of output.

juliaElastic · 2024-09-20T13:55:37Z

@juliaElastic I'm testing the branch locally and found an issue related to outputs. This happens when trying to update an output from a type to another, in my case I had a Kafka output like this:

I also tried saving it as ES output and this time it fails with this error:

So I think there are generally issues with switching the type of output.

Thanks for testing, it seems when changing output type, the previous output type fields are set to null, not removed, and it fails schema validation. Checking how to fix the logic.

So the fields are set to null to reset them, because that's how we can reset them when updating an SO. (removing a field from the updated object would not modify the field in the doc when updating).
So there is a few ways to solve this:

allow unknown fields in the output response (less strict validation)
remove null fields from the response before returning (least risky probably)
add custom schema validation function that ignores null fields (still return them in the response)
change the SO update logic to do a full rewrite when updating an output, this way we would not need to set fields to null. This would probably be the best solution, but riskier and given this is an encrypted saved object too, I would avoid making a big change to it (at least should be done separately).

I think I'm going to go with the first solution to get this resolved, we can create a follow up to make a more involved solution.

{
  "id": "35e39ed3-3986-49e8-acb9-bb8f9951afed",
  "name": "kafka2",
  "type": "logstash",
  "hosts": [
    "localhost:9092"
  ],
  "is_default": false,
  "is_default_monitoring": false,
  "config_yaml": "",
  "client_id": null,
  "version": null,
  "compression": null,
  "auth_type": null,
  "connection_type": null,
  "partition": null,
  "random": null,
  "topics": null,
  "headers": null,
  "timeout": null,
  "broker_timeout": null,
  "required_acks": null,
  "username": null,
  "sasl": null,
  "key": null,
  "compression_level": null,
  "round_robin": null,
  "hash": null,
  "topic": null,
  "ca_trusted_fingerprint": null,
  "ca_sha256": null,
  "secrets": {
    "ssl": {
      "key": {
        "id": "Rpm0D5IBhno8hQCyjHEL"
      }
    }
  },
  "password": null,
  "ssl": {
    "certificate": "test",
    "certificate_authorities": []
  }
}

Should be fixed now in the latest commit.

criamico · 2024-09-20T14:46:51Z

allow unknown fields in the output response (less strict validation)

Yes I think it's fine to use this solution for now and have a follow up to reevaluate it in the future

criamico · 2024-09-20T15:07:02Z

@juliaElastic I tested locally again different cases of switching output type and works fine. Do you think it would be possible to add at least a test covering such a scenario?

juliaElastic · 2024-09-20T15:18:07Z

@juliaElastic I tested locally again different cases of switching output type and works fine. Do you think it would be possible to add at least a test covering such a scenario?

Added a null field to the unit test here, I could add a more complete test too.

criamico

LGTM 🚀 Thanks for adding the tests!

kibana-ci · 2024-09-23T10:20:48Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: c0f7e17

Failed CI Steps

Metrics [docs]

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.7MB	1.7MB	+76.0B

History

💛 Build #236011 was flaky e6616e0
💛 Build #235817 was flaky 99da201
💚 Build #235568 succeeded 861b6e3
💔 Build #235498 failed 2902996
💛 Build #235293 was flaky e31f501

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

cc @juliaElastic

* main: (176 commits) [ML][Rules] Fixes deletion in Check interval input for anomaly detection rule (elastic#193420) Bump maximum supported package spec version to 3.2 (elastic#193574) [ES|QL] new pattern for `SORT` autocomplete (elastic#193595) [Inventory][ECO] Entities page search bar (elastic#193546) [Synthetics] Remove extra overview route (elastic#192449) [Obs Alerts table] Fix error on clicking alert reason message (elastic#193693) [Migrations] Remove tests that are not applicable in 9.x (elastic#193699) [EDR Workflows] Set Agent Tamper Protection to false on policy unassignment (elastic#193017) [Inventory][ECO] Enable elastic entity model from inventory (elastic#193557) [EDR Workflows] The host isolation exception tab is hidden on the basic license if no artifacts (elastic#192562) [Entity Analytics] Ensuring definition transforms are managed (elastic#193408) [Automatic Import] Do not remove message field for unstructured logs (elastic#193678) [Fleet] Add missing permissions for connector package (elastic#193573) [Fleet] using @kbn/config-schema part 2 (outputs and other apis) (elastic#193326) [Migrations] Provide testing archives + tooling for migrations integration tests (elastic#193328) [ES|QL] Renames the textbased editor to esql editor (elastic#193521) [ES|QL] Update function metadata (elastic#193662) [Security Solution][Entity Analytics] Scoping the entity store to spaces (elastic#193303) [Docs] Update Sharing docs (elastic#190318) [ML] AIOps: Move Log Rate Analysis results callout to help popover. (elastic#192243) ... # Conflicts: # x-pack/plugins/search_inference_endpoints/public/components/all_inference_endpoints/render_table_columns/render_endpoint/endpoint_info.test.tsx # x-pack/plugins/search_inference_endpoints/public/components/all_inference_endpoints/render_table_columns/render_endpoint/endpoint_info.tsx

part2

94e947c

juliaElastic added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Sep 18, 2024

juliaElastic self-assigned this Sep 18, 2024

juliaElastic added 2 commits September 18, 2024 17:13

missing output file

e31f501

more api validatino

2902996

juliaElastic marked this pull request as ready for review September 19, 2024 09:30

juliaElastic requested a review from a team as a code owner September 19, 2024 09:30

Merge branch 'main' into oas-part2-tests

0c51d5e

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 19, 2024

[CI] Auto-commit changed files from 'yarn openapi:bundle'

561ff93

elastic-vault-github-plugin-prod bot requested a review from a team as a code owner September 19, 2024 11:44

juliaElastic added 2 commits September 19, 2024 13:57

host_id optional in health check api

403816f

make statusCode and error optional in generic error

861b6e3

juliaElastic commented Sep 19, 2024

View reviewed changes

juliaElastic added v9.0.0 backport:skip This commit does not require backporting and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Sep 19, 2024

nchaulet self-requested a review September 19, 2024 16:36

PhilippeOberti approved these changes Sep 19, 2024

View reviewed changes

Merge branch 'main' into oas-part2-tests

99da201

allow unknowns (nulls) in output response schema for type change

e6616e0

juliaElastic and others added 2 commits September 23, 2024 10:15

Merge branch 'main' into oas-part2-tests

0ca7623

added api test for changing output type

c0f7e17

criamico approved these changes Sep 23, 2024

View reviewed changes

juliaElastic merged commit 46dbf51 into elastic:main Sep 23, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] using @kbn/config-schema part 2 (outputs and other apis) #193326

[Fleet] using @kbn/config-schema part 2 (outputs and other apis) #193326

juliaElastic commented Sep 18, 2024

obltmachine commented Sep 18, 2024

elasticmachine commented Sep 19, 2024

juliaElastic Sep 19, 2024

juliaElastic Sep 19, 2024

PhilippeOberti left a comment

criamico commented Sep 20, 2024 •

edited

Loading

juliaElastic commented Sep 20, 2024 •

edited

Loading

criamico commented Sep 20, 2024

criamico commented Sep 20, 2024

juliaElastic commented Sep 20, 2024

criamico left a comment

kibana-ci commented Sep 23, 2024

[Fleet] using @kbn/config-schema part 2 (outputs and other apis) #193326

[Fleet] using @kbn/config-schema part 2 (outputs and other apis) #193326

Conversation

juliaElastic commented Sep 18, 2024

Summary

obltmachine commented Sep 18, 2024

🤖 GitHub comments

elasticmachine commented Sep 19, 2024

juliaElastic Sep 19, 2024

Choose a reason for hiding this comment

juliaElastic Sep 19, 2024

Choose a reason for hiding this comment

PhilippeOberti left a comment

Choose a reason for hiding this comment

criamico commented Sep 20, 2024 • edited Loading

juliaElastic commented Sep 20, 2024 • edited Loading

criamico commented Sep 20, 2024

criamico commented Sep 20, 2024

juliaElastic commented Sep 20, 2024

criamico left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 23, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

History

criamico commented Sep 20, 2024 •

edited

Loading

juliaElastic commented Sep 20, 2024 •

edited

Loading