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

[aws] Bump package-spec version to 3.3.1 #11893

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zmoog
Copy link
Contributor

@zmoog zmoog commented Nov 27, 2024

Proposed commit message

Bump package-spec version to 3.3.1.

We want to get all the latest validations and features available from the latest package-spec version. Upgrading from package spec 3.0.0 to 3.3.1 required the following changes:

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.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Related issues

@zmoog
Copy link
Contributor Author

zmoog commented Nov 27, 2024

/test

@zmoog zmoog self-assigned this Nov 27, 2024
@zmoog zmoog added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team [elastic/obs-ds-hosted-services] label Nov 27, 2024
@zmoog
Copy link
Contributor Author

zmoog commented Nov 27, 2024

There's a pipeline test that's failing:

$ elastic-package test pipeline -v -g -d waf

2024/11/27 15:31:26 DEBUG Enable verbose logging
2024/11/27 15:31:26 DEBUG latest version (cached): v0.107.2. Download from: https://github.com/elastic/elastic-package/releases/tag/v0.107.2 (Timestamp 2024-11-27 15:05:35.306206 +0100 CET)
Run pipeline tests for the package
2024/11/27 15:31:26 DEBUG output command: /usr/local/bin/docker ps -a --filter label=com.docker.compose.project=elastic-package-stack --format {{.ID}}
2024/11/27 15:31:26 DEBUG output command: /usr/local/bin/docker inspect 26d10450efc2 c938e8167ef5 4e302d384676 25399154b62d 1bd06f0e5097 6b4172b6d166 8cde670d589a a6f8b175f74e e3d933ab0fe4 c7175d8cb22c
2024/11/27 15:31:26 DEBUG Connecting with Elasticsearch host from current profile (profile: default, host: "https://127.0.0.1:9200")
2024/11/27 15:31:26 DEBUG Running tests sequentially
2024/11/27 15:31:27 DEBUG Imported ECS fields definition from external schema for validation (embedded in package: false, stack uses ecs@mappings template: true)
2024/11/27 15:31:27 DEBUG Dump Elastic stack data
2024/11/27 15:31:27 DEBUG Dump stack logs (location: )
2024/11/27 15:31:27 DEBUG output command: /usr/local/bin/docker ps -a --filter label=com.docker.compose.project=elastic-package-stack --format {{.ID}}
2024/11/27 15:31:27 DEBUG output command: /usr/local/bin/docker inspect 26d10450efc2 c938e8167ef5 4e302d384676 25399154b62d 1bd06f0e5097 6b4172b6d166 8cde670d589a a6f8b175f74e e3d933ab0fe4 c7175d8cb22c
2024/11/27 15:31:27 DEBUG Dump stack logs for elasticsearch
2024/11/27 15:31:27 DEBUG running command: /usr/local/bin/docker compose version --short
2024/11/27 15:31:27 DEBUG Determined Docker Compose version: 2.29.7-desktop.1
2024/11/27 15:31:27 DEBUG running command: /usr/local/bin/docker compose -f /Users/zmoog/.elastic-package/profiles/default/stack/docker-compose.yml -p elastic-package-stack logs --since 2024-11-27T14:31:26Z elasticsearch
--- Test results for package: aws - START ---
FAILURE DETAILS:
aws/waf test-waf.log:
[0] parsing field value failed: [0] field "aws.waf.non_terminating_matching_rules.action" is undefined
[1] field "aws.waf.non_terminating_matching_rules.ruleId" is undefined
[2] field "aws.waf.non_terminating_matching_rules.ruleMatchDetails" is used as array of objects, expected explicit definition with type group or nested
[1] parsing field value failed: [0] field "aws.waf.rule_group_list.terminatingRule.ruleId" is undefined
[1] field "aws.waf.rule_group_list.terminatingRule.action" is undefined
[2] field "aws.waf.rule_group_list.nonTerminatingMatchingRules" is used as array of objects, expected explicit definition with type group or nested
[3] field "aws.waf.rule_group_list.ruleGroupId" is undefined
[2] parsing field value failed: [0] field "aws.waf.terminating_rule_match_details.conditionType" is undefined
[1] field "aws.waf.terminating_rule_match_details.location" is undefined
[2] field "aws.waf.terminating_rule_match_details.matchedData" is undefined
[3] parsing field value failed: [0] field "aws.waf.terminating_rule_match_details.matchedData" is undefined
[1] field "aws.waf.terminating_rule_match_details.conditionType" is undefined
[2] field "aws.waf.terminating_rule_match_details.location" is undefined


╭─────────┬─────────────┬───────────┬─────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────┬──────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME                               │ RESULT                                                                      │ TIME ELAPSED │
├─────────┼─────────────┼───────────┼─────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┼──────────────┤
│ aws     │ waf         │ pipeline  │ (ingest pipeline warnings test-waf.log) │ PASS                                                                        │ 533.444292ms │
│ aws     │ waf         │ pipeline  │ test-waf.log                            │ FAIL: test case failed: one or more problems with fields found in documents │ 150.739417ms │
╰─────────┴─────────────┴───────────┴─────────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────────┴──────────────╯
--- Test results for package: aws - END   ---
Done
Error: one or more test cases failed

The waf integration is unchanged, so I wonder where this comes from (is this package-spec related?)

@zmoog
Copy link
Contributor Author

zmoog commented Nov 27, 2024

Yes, it's related to package-spec 3.3.0, probably there are new validation rules.

@botelastic
Copy link

botelastic bot commented Dec 27, 2024

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Dec 27, 2024
@zmoog zmoog force-pushed the zmoog/aws-package-spec-3.3.0 branch from c2be1b3 to 223ced5 Compare December 30, 2024 10:06
@botelastic botelastic bot removed the Stalled label Dec 30, 2024
@zmoog zmoog changed the title [aws] Bump package-spec version to 3.3.0 [aws] Bump package-spec version to 3.3.1 Dec 30, 2024
@zmoog
Copy link
Contributor Author

zmoog commented Dec 30, 2024

Starting from package-spec version 3.0.1, I get the following validation errors on nested fields:

CleanShot 2024-12-30 at 11 27 13@2x

The 3.0.1 changelog doesn’t seem to mention changes related to this type

@elastic-vault-github-plugin-prod

🚀 Benchmarks report

Package aws 👍(10) 💚(8) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
securityhub_insights 942.51 798.08 -144.43 (-15.32%) 💔

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @zmoog

@zmoog
Copy link
Contributor Author

zmoog commented Dec 30, 2024

Starting from package-spec version 3.0.1, I get the following validation errors on nested fields:

The 3.0.1 changelog doesn’t seem to mention changes related to this type

The nested fields now must define the sub-fields. elastic-package added new validation rules with elastic/elastic-package#2090

@zmoog zmoog marked this pull request as ready for review December 30, 2024 18:42
@zmoog zmoog requested review from a team as code owners December 30, 2024 18:42
@andrewkroh andrewkroh added the Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations] label Dec 30, 2024
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

Comment on lines +18 to +22
- remove:
field: message
ignore_missing: true
if: 'ctx.event?.original != null'
description: 'The `message` field is no longer required if the document has an `event.original` field.'
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic of the rename processor above and the added remove processor is not straightforward to read. It looks like they may influence each other (but they don't):

Rename message to event.original (if we only have message).
Remove message (if we have both message and event.original, which is only true if both were present in the pipeline input).

I think it's slightly better if the remove is before the rename:

Remove message (if we have both message and event.original)
Rename message to event.original (if we only have message)

Copy link
Contributor

@tetianakravchenko tetianakravchenko left a comment

Choose a reason for hiding this comment

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

Please add a changelog entry and package version change

@@ -77,13 +77,27 @@ Please refer to the following [document](https://www.elastic.co/guide/en/ecs/cur
| aws.s3.object.key | The AWS S3 Object key. | keyword |
| aws.waf.arn | AWS ARN of ACL | keyword |
| aws.waf.id | ID of ACL | keyword |
| aws.waf.non_terminating_matching_rules | The list of non-terminating rules in the rule group that match the request. These are always COUNT rules (non-terminating rules that match) | nested |
| aws.waf.non_terminating_matching_rules.action | | keyword |
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is a public documentation - we shouldn't leave those fields without description
https://docs.aws.amazon.com/waf/latest/developerguide/logging-fields.html

wondering if it is maybe better to split version bump and waf PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:aws AWS Team:obs-ds-hosted-services Label for the Observability Hosted Services team [elastic/obs-ds-hosted-services] Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants