[AWS] Migrate AWS package to ecs@mappings

[harnish-elastic]

• Enhancement

Proposed commit message

Command

  go run github.com/andrewkroh/go-examples/ecs-update@014b35dfe4c9832b51e7c909a39a48257d6a005d \
    -ecs-version=8.11.0 \
    -ecs-git-ref=v8.11.0 \
    -fields-yml-drop-ecs \
    -kibana-version=^8.13.0 \
    -drop-import-mappings \
    packages/aws

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.

[ishleenk17]

@efd6, @andrewkroh : As part of the migration of AWS to ECS@mappings.
There is one conflict in WAF datastream, which is owned by security team.

In the AWS waf data stream, the pipeline test fails with the below error. And that's correct, for the web category the allowed list is as seen in the error message.

aws/waf test-waf.log:
[0] parsing field value failed: field "event.type" value "allowed" is not one of the expected values (access, error, info) for any of the values of "event.category" (web)
[1] parsing field value failed: field "event.type" value "denied" is not one of the expected values (access, error, info) for any of the values of "event.category" (web)

AWS WAF pipeline default.yml

- append:
    field: event.type
    value: allowed
    if: ctx.event.action == "ALLOW"
- append:
    field: event.type
    value: denied
    if: ctx.event.action == "BLOCK"

Would it be ok to stick to the allowed list ?
There is a possibility of this being a breaking change if users might be using these event.type values of denied and allowed .

[efd6]

@ishleenk17 I think that would a case of the tail wagging the dog; the list there is the expected term not the allowed term, but EP is being unreasonably strict. We had the same issue with ti_opencti (note in particular this line which differs from the stricter option) and in that case we were able to allow them by adding the expected values to the fields definition. It does not appear to be possible to do this in this case; this is because event.type is treated specially here using logic here

/cc @jsoriano

[jsoriano]

Would it be ok to stick to the allowed list ?
There is a possibility of this being a breaking change if users might be using these event.type values of denied and allowed .

event.category is expected to be an array, and can contain different values. Adding a value reduces the chance of making it a breaking change. In this case, appending to event.category a value that has allowed and denied as expected event types would solve the issue, for example:

- set:
    field: event.category
    value: [web, api]

Or if wanted only when event.type would be set to these values, add something like this:

- append:
    field: event.category
    value: api
    if: ctx.event.action == "ALLOW" || ctx.event.action == "BLOCK"

Another option would be to override the definition of the web event.category, with something like this:

- name: event.category
  external: ecs
  allowed_values:
  - name: web
    expected_event_types:
    - access
    - denied
    - allowed

EP is being unreasonably strict

EP is more strict than it could be in this and many other cases. We do this to encourage consistency and reduce the chances of having problematic situations.

For the interpretation of expected vs allowed, we could ignore expected_* by default, but I think they provide guidance to package developers in order to have consistent values, a consistency that can be useful in some important use cases.

We could add some toggle to disable these validations, but as mentioned above, there seem to be also reasonable options to fix this specific issue, or circumvent the validations by overriding the definitions of these fields.

[ishleenk17]

@efd6 :

I would go ahead with this option, as we don't add a new category, we just add the expected values to this category.

- name: event.category
  external: ecs
  allowed_values:
  - name: web
    expected_event_types:
    - access
    - denied
    - allowed
    - error
    - info

[efd6]

Thanks @ishleenk17 I'm OK with that.

[harnish-elastic]

@efd6 :

I would go ahead with this option, as we don't add a new category, we just add the expected values to this category.

- name: event.category
  external: ecs
  allowed_values:
  - name: web
    expected_event_types:
    - access
    - denied
    - allowed
    - error
    - info

@ishleenk17 @jsoriano Getting this error from elastic-package while doing the suggested changes in ecs.yml file /integrations/build/packages/aws-2.16.0.zip/data_stream/waf/fields/ecs.yml" is invalid: field 0: Additional property allowed_values is not allowed

[jsoriano]

@ishleenk17 @jsoriano Getting this error from elastic-package while doing the suggested changes in ecs.yml file /integrations/build/packages/aws-2.16.0.zip/data_stream/waf/fields/ecs.yml" is invalid: field 0: Additional property allowed_values is not allowed

@harnish-elastic what version of elastic-package are you using?

I cannot reproduce this failure locally, and in CI I only see these errors:

one or more errors found in document: [0] field "event.category" is not normalized as expected: expected array, found "web" (string)

This would need to be addressed too, by converting the field to array.

And given that this needs to be converted to array, I would consider the first option in #10223 (comment) instead of overriding the definition. But as you prefer.

@ishleenk17 why do you consider an issue to add a category?

[harnish-elastic]

@ishleenk17 @jsoriano Getting this error from elastic-package while doing the suggested changes in ecs.yml file /integrations/build/packages/aws-2.16.0.zip/data_stream/waf/fields/ecs.yml" is invalid: field 0: Additional property allowed_values is not allowed

@harnish-elastic what version of elastic-package are you using?

@jsoriano I am currently using elastic-package v0.100.0 version-hash 2632478

I cannot reproduce this failure locally, and in CI I only see these errors:

one or more errors found in document: [0] field "event.category" is not normalized as expected: expected array, found "web" (string)

This would need to be addressed too, by converting the field to array.
And given that this needs to be converted to array, I would consider the first option in #10223 (comment) instead of overriding the definition. But as you prefer.

Mb. I have resolved those errors related to expected array, found string.

Let me push the suggested changes.

- name: event.category
  external: ecs
  allowed_values:
  - name: web
    expected_event_types:
    - access
    - denied
    - allowed
    - error
    - info

[jsoriano]

@harnish-elastic you are right, this field is not in the package-spec, I think I had tried it but I had surely done something wrong, sorry for the confusion. We would need to make some changes to support this.

What do you think about the option of adding api to event.category? I think this change would make sense, would be more consistent with ECS values, and would unblock your progress here.

[harnish-elastic]

@harnish-elastic you are right, this field is not in the package-spec, I think I had tried it but I had surely done something wrong, sorry for the confusion. We would need to make some changes to support this.

What do you think about the option of adding api to event.category? I think this change would make sense, would be more consistent with ECS values, and would unblock your progress here.

Sure, let me give a try to your proposal!

[jsoriano]

I have opened an issue to support the override of expected_event_types elastic/package-spec#770, but I cannot guarantee we will implement and release it soon.

[ishleenk17]

@ishleenk17 why do you consider an issue to add a category?

Since, event.category doesn't support overriding of expected values, lets go ahead with addition of API to the category.

[elasticmachine]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[efd6]

What do you think about the option of adding api to event.category? I think this change would make sense, would be more consistent with ECS values, and would unblock your progress here.

I don't think "api" matches the semantics of a WAF, but "web" only matches it just barely, so I guess this is something that needs to happen.

[jsoriano]

I don't think "api" matches the semantics of a WAF, but "web" only matches it just barely, so I guess this is something that needs to happen.

The network category also seems to have the needed expected values for event type, maybe this category fits better for WAF?

[harnish-elastic]

I don't think "api" matches the semantics of a WAF, but "web" only matches it just barely, so I guess this is something that needs to happen.

The network category also seems to have the needed expected values for event type, maybe this category fits better for WAF?

Updated, thanks!

[harnish-elastic]

/test

[harnish-elastic]

/test

[harnish-elastic]

/test

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 9c7d408

History

• 💔 Build #13185 failed 9c7d408
• 💔 Build #13184 failed 9c7d408
• 💔 Build #13118 failed 9c7d408
• 💔 Build #13115 failed 9efd781
• 💔 Build #13052 failed e3188ed

cc @harnish-elastic

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[ishleenk17]

Changes look good!

[harnish-elastic]

Hey @elastic/obs-ds-hosted-services, Can someone please have a look and provide Codeowner approval so that we can proceed to merge this PR as soon as possible!

[zmoog]

LGTM

[elasticmachine]

Package aws - 2.17.0 containing this change is available at https://epr.elastic.co/search?package=aws