Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validation of properties and parameters based on mappings #2288

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Dec 18, 2024

Follows #2214

In this PR, it is ensured that:

  • subfields named as properties are validated and not considered as another parameter of the object. For instance, a mapping like:
    {
      "dynamic_templates": {},
      "properties": {
        "foo": {
          "properties": {
            "properties": {
              "type": "keyword"
            }
          }
        }
      }
    }
  • undefined parameters in the actual mapping are reported as failure
    • Preview mapping
      {
        "properties": {
          "foo": {
            "properties": {
              "type": "keyword",
            }
          }
        }
      }
    • Actual mapping
      {
        "properties": {
          "foo": {
            "properties": {
              "type": "keyword",
              "time_series_metric": "counter"
            }
          }
        }
      }

@mrodm mrodm self-assigned this Dec 18, 2024
@mrodm
Copy link
Contributor Author

mrodm commented Dec 18, 2024

test integrations

@elastic-vault-github-plugin-prod

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

@@ -647,7 +723,7 @@ func TestComparingMappings(t *testing.T) {
)
require.NoError(t, err)

errs := v.compareMappings("", c.preview, c.actual)
errs := v.compareMappings("", false, c.preview, c.actual)
Copy link
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 parameter here to know in which cases properties could be part of the parameters of a mapping or a subfield named properties.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm
Copy link
Contributor Author

mrodm commented Dec 18, 2024

test integrations

errs = append(errs, propertiesErrs...)
if len(errs) == 0 {
return nil
}
return errs.Unique()
}

func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview map[string]any) multierror.Error {
func (v *MappingValidator) validateObjectProperties(path string, couldBeParametersDefinition, containsMultifield bool, preview, actual map[string]any) multierror.Error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reordered preview and actual parameters to keep consistency with the other methods.

@elastic-vault-github-plugin-prod

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

@mrodm mrodm marked this pull request as ready for review December 19, 2024 12:15
@mrodm mrodm requested a review from a team December 19, 2024 12:15
@mrodm mrodm merged commit 79e380f into elastic:main Dec 19, 2024
3 checks passed
@mrodm mrodm deleted the fix-check-object-properties branch December 19, 2024 15:41
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