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.

Sign up for GitHub

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

Jump to bottom

[Automatic Import] Safely output the package manifest #192316

Merged
merged 8 commits into from
Sep 9, 2024
Merged

[Automatic Import] Safely output the package manifest #192316

merged 8 commits into from
Sep 9, 2024

Conversation

ilyannn
Copy link
Contributor

@ilyannn ilyannn commented Sep 7, 2024 •
edited by kibanamachine
Loading

Release note

Fixes issues with rendering the package manifest in Automatic Import.

Summary

Previously the multiline output or special symbols in the user-provided strings, like description, were breaking YAML structure of the package manifest. The user would be confronted with a message like this, during the last step, after all the work of generating the integration was completed:

image

The incorrect behavior can be observed in detail with a failing test in the first commit.

In this PR, we change the manifest construction logic from template rendering into TypeScript code. As a result, all user-provided strings are correctly serialized. We keep as close as possible to the original manifest structure, also keeping the parameter names.

Example

An example generated manifest:

format_version: 3.1.4
name: ai_teleport_3f1a4a4_2
title: Teleport-3f1a4a4-2
version: 1.0.0
description: |2-
    Multiline string
  Commit 3f1a4a4cb11239f7031359b4b432a92e21a76399
    Weird   spacing 
  No icon
type: integration
categories:
  - security
  - iam
conditions:
  kibana:
    version: ^8.13.0
policy_templates:
  - name: ai_teleport_3f1a4a4_2
    title: Teleport-3f1a4a4-2
    description: |2-
        Multiline string
      Commit 3f1a4a4cb11239f7031359b4b432a92e21a76399
        Weird   spacing 
      No icon
    inputs:
      - type: filestream
        title: 'Teleport : filestream'
        description: Teleport Audit Events Generated
owner:
  github: '@elastic/custom-integrations'
  type: elastic

Checklist

Risk Matrix

Risk Probability Severity Mitigation/Notes
It will be harder to support adding changes to the package manifest Medium Low We can update to a more advanced templating engine if we often need changes in the package manifest.

@ilyannn ilyannn self-assigned this Sep 7, 2024
@ilyannn ilyannn added Team:Security-Scalability Team label for Security Integrations Scalability Team release_note:fix labels Sep 7, 2024
@ilyannn ilyannn mentioned this pull request Sep 7, 2024
Closed
10 tasks
@ilyannn ilyannn added bug Fixes for quality problems that affect the customer experience backport:skip This commit does not require backporting labels Sep 7, 2024
@ilyannn ilyannn removed their assignment Sep 7, 2024
@ilyannn
Copy link
Contributor Author

ilyannn commented Sep 7, 2024

@elasticmachine merge upstream

@ilyannn ilyannn marked this pull request as ready for review September 7, 2024 13:28
@ilyannn ilyannn requested a review from a team as a code owner September 7, 2024 13:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-scalability (Team:Security-Scalability)

@ilyannn ilyannn enabled auto-merge (squash) September 7, 2024 13:30
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
integrationAssistant 4 5 +1

Total ESLint disabled count

id before after diff
integrationAssistant 11 12 +1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

bhapas
bhapas approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@bhapas bhapas left a comment

Choose a reason for hiding this comment

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

Tested locally... LGTM..!

@ilyannn ilyannn merged commit 521b6ee into elastic:main Sep 9, 2024
19 checks passed
@kgeller kgeller added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Sep 10, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 10, 2024
@ilyannn
[Automatic Import] Safely output the package manifest (elastic#192316)
e5bd9e8 
## Release note

Fixes issues with rendering the package manifest in Automatic Import.

## Summary

Previously the multiline output or special symbols in the user-provided
strings, like description, were breaking YAML structure of the package
manifest. The user would be confronted with a message like this, during
the last step, after all the work of generating the integration was
completed.

The incorrect behavior can be observed in detail with a failing test in
the first commit of the PR.

In this PR, we change the manifest construction logic from template
rendering into TypeScript code. As a result, all user-provided strings
are correctly serialized. We keep as close as possible to the original
manifest structure, also keeping the parameter names.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 521b6ee)
@kibanamachine kibanamachine mentioned this pull request Sep 10, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.15

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 10, 2024
@kibanamachine @ilyannn
[8.15] [Automatic Import] Safely output the package manifest (#192316) (
8457287 
#192524)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Automatic Import] Safely output the package manifest
(#192316)](#192316)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ilya
Nikokoshev","email":"ilya.nikokoshev@elastic.co"},"sourceCommit":{"committedDate":"2024-09-09T13:52:07Z","message":"[Automatic
Import] Safely output the package manifest (#192316)\n\n## Release
note\r\n\r\nFixes issues with rendering the package manifest in
Automatic Import.\r\n\r\n## Summary\r\n\r\nPreviously the multiline
output or special symbols in the user-provided\r\nstrings, like
description, were breaking YAML structure of the package\r\nmanifest.
The user would be confronted with a message like this, during\r\nthe
last step, after all the work of generating the integration
was\r\ncompleted.\r\n\r\nThe incorrect behavior can be observed in
detail with a failing test in\r\nthe first commit of the PR.\r\n\r\nIn
this PR, we change the manifest construction logic from
template\r\nrendering into TypeScript code. As a result, all
user-provided strings\r\nare correctly serialized. We keep as close as
possible to the original\r\nmanifest structure, also keeping the
parameter names.\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic
Machine
<elasticmachine@users.noreply.github.com>","sha":"521b6eec30cb5f8e4a81307110760527d21fd154","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","backport:prev-minor","v8.16.0","Team:Security-Scalability"],"title":"[Automatic
Import] Safely output the package
manifest","number":192316,"url":"https://github.com/elastic/kibana/pull/192316","mergeCommit":{"message":"[Automatic
Import] Safely output the package manifest (#192316)\n\n## Release
note\r\n\r\nFixes issues with rendering the package manifest in
Automatic Import.\r\n\r\n## Summary\r\n\r\nPreviously the multiline
output or special symbols in the user-provided\r\nstrings, like
description, were breaking YAML structure of the package\r\nmanifest.
The user would be confronted with a message like this, during\r\nthe
last step, after all the work of generating the integration
was\r\ncompleted.\r\n\r\nThe incorrect behavior can be observed in
detail with a failing test in\r\nthe first commit of the PR.\r\n\r\nIn
this PR, we change the manifest construction logic from
template\r\nrendering into TypeScript code. As a result, all
user-provided strings\r\nare correctly serialized. We keep as close as
possible to the original\r\nmanifest structure, also keeping the
parameter names.\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic
Machine
<elasticmachine@users.noreply.github.com>","sha":"521b6eec30cb5f8e4a81307110760527d21fd154"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192316","number":192316,"mergeCommit":{"message":"[Automatic
Import] Safely output the package manifest (#192316)\n\n## Release
note\r\n\r\nFixes issues with rendering the package manifest in
Automatic Import.\r\n\r\n## Summary\r\n\r\nPreviously the multiline
output or special symbols in the user-provided\r\nstrings, like
description, were breaking YAML structure of the package\r\nmanifest.
The user would be confronted with a message like this, during\r\nthe
last step, after all the work of generating the integration
was\r\ncompleted.\r\n\r\nThe incorrect behavior can be observed in
detail with a failing test in\r\nthe first commit of the PR.\r\n\r\nIn
this PR, we change the manifest construction logic from
template\r\nrendering into TypeScript code. As a result, all
user-provided strings\r\nare correctly serialized. We keep as close as
possible to the original\r\nmanifest structure, also keeping the
parameter names.\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic
Machine
<elasticmachine@users.noreply.github.com>","sha":"521b6eec30cb5f8e4a81307110760527d21fd154"}}]}]
BACKPORT-->

Co-authored-by: Ilya Nikokoshev <ilya.nikokoshev@elastic.co>
@szaffarano szaffarano mentioned this pull request Sep 13, 2024
Merged
1 task
@ilyannn ilyannn mentioned this pull request Oct 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhapas bhapas bhapas approved these changes

Assignees

@ilyannn ilyannn

Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience release_note:fix Team:Security-Scalability Team label for Security Integrations Scalability Team v8.15.2 v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ilyannn @elasticmachine @kibana-ci @kibanamachine @bhapas @kgeller