Skip to content

Add JSON patch to remove SLO assets for spec versions less than 3.4.0#884

Merged
mrodm merged 14 commits intoelastic:mainfrom
mrodm:patch_remove_slo_folder_content_packages
Mar 24, 2025
Merged

Add JSON patch to remove SLO assets for spec versions less than 3.4.0#884
mrodm merged 14 commits intoelastic:mainfrom
mrodm:patch_remove_slo_folder_content_packages

Conversation

@mrodm
Copy link
Copy Markdown
Contributor

@mrodm mrodm commented Mar 21, 2025

What does this PR do?

Remove SLO assets in content package for spec versions < 3.4.0, as it is applied in the integration packages.

Ensure that all the kibana validation rules are also executed in content packages.

Why is it important?

Keep the same behavior in content and integration packages.

Checklist

Related issues

@mrodm mrodm self-assigned this Mar 21, 2025
Comment on lines +1 to +3
errors:
exclude_checks:
- PSR00001 # Allow to use unreleased features in GA package.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added since 3.4.0 is not released yet (-next prerelease tag)

@@ -1,4 +1,4 @@
format_version: 3.3.1
format_version: 3.4.0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be updated the spec version for the compliance test here ?

@3.3.0
Scenario: Content package can be installed
Given the "good_content" package is installed
#Then there are no errors.

That test is running successfully even if that spec version is not updated.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the package has SLOs it should be updated, yes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that spec version is updated to be @3.4.0, I guess it should be needed to add a new compliance test for 3.4.0.

Or should it be updated the one with stack 9.1.0-SNAPSHOT to 3.4.0 ? @jsoriano

compliance_test 9.1.0-SNAPSHOT 3.3.3

Would it be better to create a new content package without SLO , so it can be tested that scenario with previous spec versions 3.3.0 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new basic_content test package , to be able to keep testing content packages with spec version 3.3.0 (and previous Elastic stacks). Added in 6250bd1

@mrodm mrodm marked this pull request as ready for review March 21, 2025 14:44
@mrodm mrodm requested a review from a team as a code owner March 21, 2025 14:44
@mrodm
Copy link
Copy Markdown
Contributor Author

mrodm commented Mar 21, 2025

test integrations

@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

Created or updated PR in integrations repository to test this version. Check elastic/integrations#13242

jsoriano
jsoriano previously approved these changes Mar 21, 2025
@@ -1,4 +1,4 @@
format_version: 3.3.1
format_version: 3.4.0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the package has SLOs it should be updated, yes.

{fn: semantic.ValidateKibanaNoDanglingObjectIDs, since: semver.MustParse("3.0.0")},
{fn: semantic.ValidateKibanaFilterPresent, since: semver.MustParse("3.0.0")},
{fn: semantic.ValidateKibanaNoLegacyVisualizations, types: []string{"integration"}, since: semver.MustParse("3.0.0")},
{fn: semantic.ValidateKibanaNoLegacyVisualizations, types: []string{"integration", "content"}, since: semver.MustParse("3.0.0")},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these changes be mentioned in the changelog too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new changelog entry for these changes in dd33139

@mrodm
Copy link
Copy Markdown
Contributor Author

mrodm commented Mar 21, 2025

Validated in elastic/integrations#13242 that testing of the current content packages (kubernetes_otel and nginx_ingress_controller_otel) defined in the integrations repository is successful.

Buildkite build: https://buildkite.com/elastic/integrations/builds/23702

EOF

# Generate each test we want to do.
compliance_test 9.1.0-SNAPSHOT 3.4.0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would allow to test the installation of good_content package that contains SLO assets.

  @3.4.0
  Scenario: Content package can be installed
   Given the "good_content" package is installed
   #Then there are no errors.

Does it make sense?

Copy link
Copy Markdown
Contributor Author

@mrodm mrodm Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe, just use the new basic_content package for compliance testing and remove the testing of good_content (this contains SLO assets)?

If good_content package is removed from these tests, there is no need to add this new compliance test with spec 3.4.0.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compliance tests for 3.4.0 in 9.1.0 should fail, because 3.4.0 is not implemented yet. Or is it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not failing the current compliance tests, but I don't see any SLO assets created when I tested locally to install in Elastic stack 9.1.0-SNAPSHOT (related PR not merged yet elastic/kibana#186974).

Copy link
Copy Markdown
Contributor Author

@mrodm mrodm Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been updated the compliance test using the good_content package to validate that there is a specific SLO asset installed. Currently, it's failing when using spec 3.4.0 and latest Elastic stack 9.1.0-SNAPSHOT.

https://buildkite.com/elastic/package-spec/builds/1053#0195c88d-3a38-44db-a668-f1c14f0cf477

Therefore, I will remove the compliance test for that scenario (9.1.0-SNAPSHOT and spec 3.4.0): 5b59414

@mrodm mrodm requested a review from jsoriano March 21, 2025 17:40
@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm merged commit c5acb1c into elastic:main Mar 24, 2025
3 checks passed
@mrodm mrodm deleted the patch_remove_slo_folder_content_packages branch March 24, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants