Make tags.yml print a warning instead of failing

[marc-gr]

What does this PR do?

Changes validation error for Kibana version when using tags.yaml to a warning.

Why is it important?

If we want to add this file to all integrations, this would require to bump minimum kibana version for all of them.

Since having this file, even when not used, when targeting lower stack versions is not harmful, removing the error allows us to add tags.yml to all integrations. Users on stacks >= 8.10 would benefit from it automatically, and for the rest nothing would change.

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

• Relates Add asset tags to security integrations integrations#7483

[jsoriano]

Since having this file, even when not used, when targeting lower stack versions is not harmful

The problem can be, that users may want to add the saved object tags, but they would silently be ignored in older versions, and this may be unexpected. I think this was the main reason to add this check.

[marc-gr]

Since having this file, even when not used, when targeting lower stack versions is not harmful

The problem can be, that users may want to add the saved object tags, but they would silently be ignored in older versions, and this may be unexpected. I think this was the main reason to add this check.

I think this was our initial assumption of how this would work based on elastic/integrations#7483. In any case it is for sure up for discussion cc @narph

[P1llus]

Since having this file, even when not used, when targeting lower stack versions is not harmful

The problem can be, that users may want to add the saved object tags, but they would silently be ignored in older versions, and this may be unexpected. I think this was the main reason to add this check.

I think since the tags never existed in the old versions, and the creation of those tags is managed by fleet, they are free to create tags with the same names manually, but they should not really be deleted or taken over by fleet either way.

I was hoping to not have the version restriction, but let's see if there is any other opinions as well 👍

[mrodm]

Since having this file, even when not used, when targeting lower stack versions is not harmful

The problem can be, that users may want to add the saved object tags, but they would silently be ignored in older versions, and this may be unexpected. I think this was the main reason to add this check.

I agree with @jsoriano . This validation was added to avoid users/developers adding those tag files (kibana/tags.yaml) targeting Kibana versions that do not support that. I think that it could be misleading for users/developers that expect to have those tags in those assets when installing the package (since those are defined in the package), but they are not going to be created by Fleet in those Kibana versions < 8.10. There will not be neither any error or warning, just ignored those tags.

[P1llus]

@jsoriano @mrodm Unfortunately this one is a fairly significant blocker for us. We are planning on deploying this on every integration owned by SEI, but it would be fairly significant impact for us to bump the version on all our integrations, seeing that we cannot provide hotfixes anymore to older versions then.

These tags are also required for our UI in Serverless.
Is there nothing else that can be done? For example we could easily add a lint/check warning, checking for tags.yml and comparing it to current version in manifest.yml?
Or can we add a undocumented override that we can add somewhere, that is meant only for these niche cases? As this feature is fairly important to us, its hard to completely agree, while I do totally understand the reasoning around it.

[mrodm]

seeing that we cannot provide hotfixes anymore to older versions then.

There is a way of doing hotfixes in-place for integrations repository, here are the docs:
https://github.com/elastic/integrations/blob/e32c0233a0bac0201ef6687b60c0624ca571d35c/docs/developer_workflow_bug_fix_older_package_version.md

An option it would be to change this to just a warning in package-spec, but this warning would be just shown while the package is built or checked using elastic-package. Just as a
note, this warning would not appear when the users install this integration.

If it is done so, probably description of this tags file should be also updated to include something similar to "They are just installed when Kibana >= 8.10". And it could be added the warning, as it was done in this validation rule to be able to test it:

package-spec/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go

Line 31 in 037631b

func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors {

Here it is the test to validate warnings:

package-spec/code/go/pkg/validator/validator_test.go

Line 501 in 037631b

if err := common.EnableWarningsAsErrors(); err != nil {

@jsoriano WDYT ?

[P1llus]

Would be really appreciated if this could be possible! A bit unsure why the end users would be the main focus there, as there is no UI saying that these tags will be added right? Or documentation that mentions the tags are created?

[jsoriano]

@jsoriano WDYT ?

Sounds good.

A bit unsure why the end users would be the main focus there, as there is no UI saying that these tags will be added right?

It would be users, package developers, support... these tags will be silently ignored on installation on older versions, what can be unexpected when the tags are actually defined in the packages. Probably not a big issue in any case to miss some tags, but unexpected.

[P1llus]

@jsoriano WDYT ?

Sounds good.

A bit unsure why the end users would be the main focus there, as there is no UI saying that these tags will be added right?

It would be users, package developers, support... these tags will be silently ignored on installation on older versions, what can be unexpected when the tags are actually defined in the packages. Probably not a big issue in any case to miss some tags, but unexpected.

@jsoriano @marc-gr let's add a comment at the top of the tags.yml file explaining that these will be applied to 8.10+ users only, just to prevent users seeing our tag files and might get confused?

[mrodm]

@jsoriano @marc-gr let's add a comment at the top of the tags.yml file explaining that these will be applied to 8.10+ users only, just to prevent users seeing our tag files and might get confused?

It would be nice to also change the errors by warnings following the example of the validation rule shown in #577 (comment)

package-spec/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go

Lines 31 to 32 in 037631b

	func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors {
	warningsAsErrors := common.IsDefinedWarningsAsErrors()

package-spec/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go

Lines 61 to 66 in 037631b

	message := fmt.Sprintf("Warning: references found in dashboard %s: %s", filePath, s)
	if warningsAsErrors {
	errs = append(errs, errors.New(message))
	} else {
	log.Printf(message)
	}

That validation rule will just show a warning message when building time, but when testing package-spec (running make test in this directory) they would be raised an errors so they can be tested

[marc-gr]

I will update the PR to include that then 👍

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 3e96440

History

• 💔 Build #143 failed 5723465
• 💚 Build #138 succeeded 444cb8c
• 💚 Build #134 succeeded a6bf34b
• 💔 Build #133 failed 4dae82a

[mrodm]

👍
Just as a note, packages will need to be updated to a spec version >= 2.11.0 (to be released) in order to be able to define kibana/tags.yml