-
Notifications
You must be signed in to change notification settings - Fork 88
Fix template path validator for composable integration packages #1108
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -47,6 +47,9 @@ func TestValidateFile(t *testing.T) { | |||||||||
| "good_alert_rule_templates": {}, | ||||||||||
| "good_requires": {}, | ||||||||||
| "good_package_reference_policy_template": {}, | ||||||||||
| "good_composable_pt_inputs": {}, | ||||||||||
| "good_composable_ds_streams": {}, | ||||||||||
|
Comment on lines
+50
to
+51
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit.
Suggested change
|
||||||||||
| "good_composable_multi_package": {}, | ||||||||||
|
Comment on lines
+50
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could these cases be added to existing good_packages instead of adding a test package for each one?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, |
||||||||||
| "deploy_custom_agent": {}, | ||||||||||
| "deploy_custom_agent_multi_services": {}, | ||||||||||
| "deploy_docker": {}, | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,9 @@ | |
| - description: Add support for `profiles` policy template type in input packages | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/1092 | ||
| - description: Fix template path validator to skip policy template inputs and data stream streams that use package references. | ||
| type: bugfix | ||
| link: https://github.com/elastic/package-spec/pull/1108 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to add changelog for this as package references are also introduced in 3.6. |
||
| - version: 3.5.8 | ||
| changes: | ||
| - description: Add deployer option to system benchmark configuration. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| - version: 0.0.1 | ||
| changes: | ||
| - description: Initial release | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| - name: data_stream.type | ||
| type: constant_keyword | ||
| description: Data stream type. | ||
| - name: data_stream.dataset | ||
| type: constant_keyword | ||
| description: Data stream dataset. | ||
| - name: data_stream.namespace | ||
| type: constant_keyword | ||
| description: Data stream namespace. | ||
| - name: '@timestamp' | ||
| type: date | ||
| description: Event timestamp. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| title: Web server logs | ||
| type: logs | ||
| streams: | ||
| - package: filelog_otel | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this package correct in a real case? This input is not used on any policy template 🤔 Only
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont know if is a real-case, but is a possible case... |
||
| title: Web Server Access Logs | ||
| description: Collect web server access logs using the filelog OTel input package | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Composable Package - Data Stream Streams | ||
|
|
||
| This is a test package to validate that all data stream streams can reference an input package via the `package` field instead of the traditional `input` field. The `filelog_otel` package is declared in `requires.input` and referenced directly in the data stream stream entry. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| format_version: 3.6.0 | ||
| name: good_composable_ds_streams | ||
| title: Composable Package - Data Stream Streams | ||
| description: Integration package with all data stream streams using package references. | ||
| version: 0.0.1 | ||
| type: integration | ||
| source: | ||
| license: "Apache-2.0" | ||
| conditions: | ||
| kibana: | ||
| version: '^8.0.0' | ||
| requires: | ||
| input: | ||
| - package: filelog_otel | ||
| version: "1.0.0" | ||
| policy_templates: | ||
| - name: webserver | ||
| title: Web server monitoring | ||
| description: Collect logs from web servers | ||
| inputs: | ||
| - type: logfile | ||
| title: Collect web server logs | ||
| description: Collecting web server logs using logfile | ||
| owner: | ||
| github: elastic/foobar | ||
| type: community |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| - version: 0.0.1 | ||
| changes: | ||
| - description: Initial release | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| - name: data_stream.type | ||
| type: constant_keyword | ||
| description: Data stream type. | ||
| - name: data_stream.dataset | ||
| type: constant_keyword | ||
| description: Data stream dataset. | ||
| - name: data_stream.namespace | ||
| type: constant_keyword | ||
| description: Data stream namespace. | ||
| - name: '@timestamp' | ||
| type: date | ||
| description: Event timestamp. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| title: Database metrics | ||
| type: metrics | ||
| streams: | ||
| - package: sql_input | ||
| title: Database Metrics | ||
| description: Collect database metrics using the SQL input package |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| - name: data_stream.type | ||
| type: constant_keyword | ||
| description: Data stream type. | ||
| - name: data_stream.dataset | ||
| type: constant_keyword | ||
| description: Data stream dataset. | ||
| - name: data_stream.namespace | ||
| type: constant_keyword | ||
| description: Data stream namespace. | ||
| - name: '@timestamp' | ||
| type: date | ||
| description: Event timestamp. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| title: Web server logs | ||
| type: logs | ||
| streams: | ||
| - package: filelog_otel | ||
| title: Web Server Logs | ||
| description: Collect web server logs using the filelog OTel input package |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Composable Package - Multiple Input Packages | ||
|
|
||
| This is a test package to validate that multiple input packages can be declared in `requires.input` and referenced via the `package` field in both policy template inputs and data stream streams. It also demonstrates `requires.content` for content package dependencies, which accept version constraints and cannot be referenced via `package`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| format_version: 3.6.0 | ||
| name: good_composable_multi_package | ||
| title: Composable Package - Multiple Input Packages | ||
| description: Integration using multiple input packages in both policy templates and data streams, with content package dependencies. | ||
| version: 0.0.1 | ||
| type: integration | ||
| source: | ||
| license: "Apache-2.0" | ||
| conditions: | ||
| kibana: | ||
| version: '^8.0.0' | ||
| requires: | ||
| input: | ||
| - package: sql_input | ||
| version: "1.0.0" | ||
| - package: filelog_otel | ||
| version: "2.0.0" | ||
| content: | ||
| - package: elastic_agent | ||
| version: "^1.0.0" | ||
| - package: system | ||
| version: "~2.0.0" | ||
| policy_templates: | ||
| - name: database | ||
| title: Database monitoring | ||
| description: Collect metrics from databases | ||
| inputs: | ||
| - package: sql_input | ||
| title: Collect database metrics | ||
| description: Collecting database metrics using the SQL input package | ||
| - name: webserver | ||
| title: Web server monitoring | ||
| description: Collect logs from web servers | ||
| inputs: | ||
| - package: filelog_otel | ||
| title: Collect web server logs | ||
| description: Collecting web server logs using the filelog OTel input package | ||
| owner: | ||
| github: elastic/foobar | ||
| type: community |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| - version: 0.0.1 | ||
| changes: | ||
| - description: Initial release | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| data_stream: | ||
| {{#if data_stream.dataset}} | ||
| dataset: {{data_stream.dataset}} | ||
| {{/if}} | ||
| paths: | ||
| {{#each paths}} | ||
| - {{this}} | ||
| {{/each}} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| - name: data_stream.type | ||
| type: constant_keyword | ||
| description: Data stream type. | ||
| - name: data_stream.dataset | ||
| type: constant_keyword | ||
| description: Data stream dataset. | ||
| - name: data_stream.namespace | ||
| type: constant_keyword | ||
| description: Data stream namespace. | ||
| - name: '@timestamp' | ||
| type: date | ||
| description: Event timestamp. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| title: Database metrics | ||
| type: metrics | ||
| streams: | ||
| - input: logfile | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This input is not used on any policy template. Would this work as a real package?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as the other example, out of the spec this is possible for a package, i created them based on what possibilities a package can have. is there a requirement for data streams and inputs from policy templates to be "conected". i remember the conexion is between the input type (policy) and the input (Stream). they should appear allways? i have a gap here. if a policy template has a package, with a template, does it make sense to have a data stream with templates too? |
||
| title: Database Metrics | ||
| description: Collect database metrics using logfile input | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Composable Package - Policy Template Inputs | ||
|
|
||
| This is a test package to validate that all policy template inputs can reference an input package via the `package` field instead of the traditional `type` field. The `sql_input` package is declared in `requires.input` and referenced directly in the policy template input. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| format_version: 3.6.0 | ||
| name: good_composable_pt_inputs | ||
| title: Composable Package - Policy Template Inputs | ||
| description: Integration package with all policy template inputs using package references. | ||
| version: 0.0.1 | ||
| type: integration | ||
| source: | ||
| license: "Apache-2.0" | ||
| conditions: | ||
| kibana: | ||
| version: '^8.0.0' | ||
| requires: | ||
| input: | ||
| - package: sql_input | ||
| version: "1.0.0" | ||
| policy_templates: | ||
| - name: database | ||
| title: Database monitoring | ||
| description: Collect metrics and logs from databases | ||
| inputs: | ||
| - package: sql_input | ||
| title: Collect database metrics | ||
| description: Collecting database metrics using the SQL input package | ||
| owner: | ||
| github: elastic/foobar | ||
| type: community |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be still validated on the built package, right?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont understand the question :S
Before building, this validation takes place. Here we are looking for the template_path so we check if exist. Data streams and policy templates are "linked" by the input.type -> stream.input;
when using package, the validation does not find template path, and injects empty "type" into the stream validation, to eventually find the template path.. when in the stream template validation, it finds that package is used, so input is also ""; empty == empty; it looks for the default stream.yml.hbs and it does not find it.
Shall this "connection" be fixed? perhaps i took a wrong assumption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the code is fine, I was only asking for confirmation.
elastic-package buildvalidates the built package after building it. In this case built packages will have streams here even if their source code didn't have them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm, actually I see that streams merge is not described on any issue, apart from variables and template paths. I will create one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elastic/elastic-package#3380