Add validation rule to disallow dangling object IDs #589

mrodm · 2023-09-11T14:50:27Z

What does this PR do?

This PR adds a new validation rule to be applied in v3 to check dangling object IDs.

Why is it important?

This new validation rule ensures that all references listed in dashboards or other assets are valid ones. That means that there is an asset file with the same ID and type in the package.

Example of the error reported using elastic-package:

 $ elastic-package check 
Format the package
Done
Lint the package
2023/09/11 16:40:52 Warning: package using an unreleased version of the spec (3.0.0-next)
Error: checking package failed: linting package failed: found 1 validation error:
   1. file "/home/user/Coding/work/package-spec/test/packages/bad_dangling_object_ids/kibana/dashboard/bad_dangling_object_ids-82273ffe-6acc-4f2f-bbee-c1004abba63d.json" is invalid: dangling reference found: bad_dangling_object_ids-8287a5d5-1576-4f3a-83c4-444e9058439c (search)

IMPORTANT: Excluded index-pattern references from this check.

Errors found if this validation rule would be applied to all packages (mainly dangling reference of tags):

Package gcp:

 1. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-1ae960c0-f9f8-11eb-bc38-79936db7c106.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
 2. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-1ae960c0-f9f8-11eb-bc38-79936db7c106.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)
 3. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-1ae960c0-f9f8-11eb-bc38-79936db7c106.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
 4. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-1ae960c0-f9f8-11eb-bc38-79936db7c106.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)
 5. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-2b0fd7b0-feac-11ea-b032-d59f894a5072.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
 6. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-2b0fd7b0-feac-11ea-b032-d59f894a5072.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)
 7. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-ca401040-8e52-11ea-9fa6-4d675d5290dc.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
 8. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-ca401040-8e52-11ea-9fa6-4d675d5290dc.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)
 9. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-f40ee870-5e4a-11ea-a4f6-717338406083.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
10. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-f40ee870-5e4a-11ea-a4f6-717338406083.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)
11. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-f40ee870-5e4a-11ea-a4f6-717338406083.json" is invalid: dangling reference found: gcp-fleet-managed-default (tag)
12. file "/home/mariorodriguez/Coding/work/integrations/packages/gcp/kibana/dashboard/gcp-f40ee870-5e4a-11ea-a4f6-717338406083.json" is invalid: dangling reference found: gcp-fleet-pkg-gcp-default (tag)

Package istio:

1. file "/home/mariorodriguez/Coding/work/integrations/packages/istio/kibana/dashboard/istio-fed6f500-5941-11ed-bfb8-bbfe5b338339.json" is invalid: dangling reference found: fleet-managed-default (tag)
2. file "/home/mariorodriguez/Coding/work/integrations/packages/istio/kibana/dashboard/istio-fed6f500-5941-11ed-bfb8-bbfe5b338339.json" is invalid: dangling reference found: fleet-pkg-istio-default (tag)

Package system:

 1. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-0d3f2380-fa78-11e6-ae9b-81e5311e8cab.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 2. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-277876d0-fa2c-11e6-bbd3-29c986c96e5a.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 4. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-5517a150-f9ce-11e6-8115-a7c18106d86a.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 5. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-79ffd6e0-faa0-11e6-947f-177f697178b8.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 6. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-Logs-syslog-dashboard.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 7. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/dashboard/system-Metrics-system-overview.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 8. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/search/system-62439dc0-f9c9-11e6-a747-6121780e0414.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
 9. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/search/system-8030c1b0-fa77-11e6-ae9b-81e5311e8cab.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
10. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/search/system-Syslog-system-logs.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
11. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/search/system-b6f321e0-fa25-11e6-bbd3-29c986c96e5a.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)
12. file "/home/mariorodriguez/Coding/work/integrations/packages/system/kibana/search/system-eb0039f0-fa7f-11e6-a1df-a78bd7504d38.json" is invalid: dangling reference found: system-fleet-pkg-system-default (tag)

Package airflow:

1. file "/home/mariorodriguez/Coding/work/integrations/packages/airflow/kibana/dashboard/airflow-a3aa42d0-a465-11ed-9ff0-ab4dd59e4c75.json" is invalid: dangling reference found: airflow-fleet-managed-default (tag)
2. file "/home/mariorodriguez/Coding/work/integrations/packages/airflow/kibana/dashboard/airflow-a3aa42d0-a465-11ed-9ff0-ab4dd59e4c75.json" is invalid: dangling reference found: airflow-fleet-pkg-airflow-default (tag)

Checklist

I have added test packages to test/packages that prove my change is effective.
I have added an entry in spec/changelog.yml.

Related issues

Part of Package Spec v3 #539

Update error message to follow the same pattern as other validation errors

mrodm · 2023-09-11T14:59:24Z

test integrations

elasticmachine · 2023-09-11T15:02:39Z

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#7758

jsoriano · 2023-09-11T17:48:15Z

Is it possible that it is ok to reference these fleet managed tags?

mrodm · 2023-09-11T17:50:18Z

code/go/internal/validator/spec.go

@@ -133,6 +133,7 @@ func (s Spec) rules(pkgType string, rootSpec spectypes.ItemSpec) validationRules
 		{fn: semantic.ValidateProfilingNonGA, types: []string{"integration"}},
 		{fn: semantic.ValidateKibanaObjectIDs, types: []string{"integration"}},
 		{fn: semantic.ValidateRoutingRulesAndDataset, types: []string{"integration"}, since: semver.MustParse("2.9.0")},
+		{fn: semantic.ValidateKibanaNoDanglingObjectIDs, since: semver.MustParse("3.0.0")},


Just to be applied since v3.0.0

mrodm · 2023-09-12T10:07:58Z

Is it possible that it is ok to reference these fleet managed tags?

@jsoriano Not sure about this, in case of elastic-package when it exports dashboards it was set to remove those references from the dashboard too in this PR elastic/elastic-package#1206.

Currently, if it is tried to export those dashboards (for instance from airflow package), it is raised this error (as a note the installation does not fail):

 $ elastic-package-v0.86.1 export dashboards --allow-snapshot
Export Kibana dashboards
Warning: exporting dashboards from a SNAPSHOT version of Kibana (8.10.0-SNAPSHOT) is discouraged. It could lead to invalid dashboards (for example if they use features that are reverted or modified before the final release)
? Which dashboards would you like to export? [Metrics Airflow] Overview (ID: airflow-a3aa42d0-a465-11ed-9ff0-ab4dd59e4c75)
Error: dashboards export failed: exporting dashboards using Kibana client failed: at least Kibana object returned an error: [0] Saved object [tag/airflow-fleet-managed-default] not found
[1] Saved object [tag/airflow-fleet-pkg-airflow-default] not found

Probably because elastic-package also adds the package name as part of one of their transform operations: https://github.com/elastic/elastic-package/blob/e1157add2447f28f135fdb055aa396fce8f9c3b5/internal/export/transform_standardize.go#L128-L141

Those fleet tags are always created when the package is installed ("managed" and "<package_name>").

jsoriano

👍

jsoriano · 2023-09-12T12:25:30Z

code/go/internal/validator/semantic/validate_kibana_no_dangling_object_ids.go

+		found := false
+		for _, installed := range installedIDs {
+			if reference.objectID != installed.objectID {
+				continue
+			}
+			if reference.objectType != installed.objectType {
+				continue
+			}
+
+			found = true
+		}


Nit, as the struct seems to be comparable, maybe you can use slices.Contains.

Suggested change

found := false

for _, installed := range installedIDs {

if reference.objectID != installed.objectID {

continue

}

if reference.objectType != installed.objectType {

continue

}

found = true

}

found := slices.Contains(installedIDs, reference)

I'll check it, but there is a third parameter filePath in the struct that has not the same value between installedIDs and referencedIDs

Changed to use slice.ContainsFunc to use those specific conditions.

Using slice.Conditions make the tests fail.

jsoriano · 2023-09-12T12:27:22Z

code/go/internal/validator/semantic/validate_kibana_no_dangling_object_ids.go

+	}
+
+	var references []reference
+	for _, reference := range allReferences {


Nit. Also here maybe you can use some slices helper.

Changed to use slices.Contains 👍

jsoriano · 2023-09-12T12:28:38Z

test/packages/good/kibana/dashboard/good-dashboard-abc-1.json

@@ -1,3 +1,4 @@
 {
-  "id": "good-dashboard-abc-1"
+  "id": "good-dashboard-abc-1",
+  "type": "dashboard"


Why these files didn't have types? Were they crafted files for tests?

Yes I think those assets were crafted with the minimum values for the tests. At that moment, it was just the id field.

jsoriano

👍

code/go/internal/validator/semantic/validate_kibana_no_dangling_object_ids.go

elasticmachine · 2023-09-12T14:59:50Z

💚 Build Succeeded

Buildkite Build
Commit: b917cb7

History

💚 Build #171 succeeded 1f4e35b
💚 Build #169 succeeded 945b5cd
💚 Build #158 succeeded 34bbf0c
💚 Build #156 succeeded ea17720

cc @mrodm

mrodm added 3 commits September 11, 2023 16:28

Add validation rule to check dangling object IDs

3d9a5e9

Add type field into assets

c40c086

Add test package

315ba9e

Update error message to follow the same pattern as other validation errors

mrodm self-assigned this Sep 11, 2023

Add changelog entry

ea17720

elasticmachine mentioned this pull request Sep 11, 2023

Test branch package-spec#589 - DO NOT MERGE elastic/integrations#7758

Closed

Merge remote upstream/main into add_validation_dangling_objects

34bbf0c

mrodm marked this pull request as ready for review September 11, 2023 17:40

mrodm requested a review from a team as a code owner September 11, 2023 17:40

mrodm changed the title ~~Add validation dangling objects~~ Add validation rule to disallow dangling object IDs Sep 11, 2023

mrodm commented Sep 11, 2023

View reviewed changes

mrodm mentioned this pull request Sep 12, 2023

Package Spec v3 #539

Closed

jsoriano previously approved these changes Sep 12, 2023

View reviewed changes

Usage of slices package

4720e03

mrodm dismissed jsoriano’s stale review via 4720e03 September 12, 2023 13:59

Replace containsFunc by contains

945b5cd

mrodm requested a review from jsoriano September 12, 2023 14:34

Merge remote upstream/main into add_validation_dangling_objects

1f4e35b

jsoriano previously approved these changes Sep 12, 2023

View reviewed changes

code/go/internal/validator/semantic/validate_kibana_no_dangling_object_ids.go Outdated Show resolved Hide resolved

Re-order imports

b917cb7

mrodm dismissed jsoriano’s stale review via b917cb7 September 12, 2023 14:53

jsoriano approved these changes Sep 12, 2023

View reviewed changes

mrodm merged commit 0839924 into elastic:main Sep 12, 2023

mrodm deleted the add_validation_dangling_objects branch September 12, 2023 15:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation rule to disallow dangling object IDs #589

Add validation rule to disallow dangling object IDs #589

mrodm commented Sep 11, 2023 •

edited

Loading

mrodm commented Sep 11, 2023

elasticmachine commented Sep 11, 2023

jsoriano commented Sep 11, 2023

mrodm Sep 11, 2023

mrodm commented Sep 12, 2023

jsoriano left a comment

jsoriano Sep 12, 2023

mrodm Sep 12, 2023

mrodm Sep 12, 2023

jsoriano Sep 12, 2023

mrodm Sep 12, 2023

jsoriano Sep 12, 2023

mrodm Sep 12, 2023

jsoriano left a comment

elasticmachine commented Sep 12, 2023

Add validation rule to disallow dangling object IDs #589

Add validation rule to disallow dangling object IDs #589

Conversation

mrodm commented Sep 11, 2023 • edited Loading

What does this PR do?

Why is it important?

Checklist

Related issues

mrodm commented Sep 11, 2023

elasticmachine commented Sep 11, 2023

jsoriano commented Sep 11, 2023

Choose a reason for hiding this comment

mrodm commented Sep 12, 2023

jsoriano left a comment

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

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

jsoriano left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 12, 2023

💚 Build Succeeded

History

mrodm commented Sep 11, 2023 •

edited

Loading