-
Notifications
You must be signed in to change notification settings - Fork 459
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
[Cloud Security] release Wiz version 2.0.0 #11414
Changes from 4 commits
03a32bd
6dde700
5ca6264
4d557e6
82f8834
c5599fa
b7cc277
25632c2
9f8506d
a2cbc19
e086309
1623967
1184a6b
15734c8
e7616b0
93271df
1560059
3f11a0f
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 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,7 +11,5 @@ | |||||||||||||
type: text | ||||||||||||||
- name: remediation | ||||||||||||||
type: text | ||||||||||||||
- name: references | ||||||||||||||
type: text | ||||||||||||||
- name: reference | ||||||||||||||
type: text | ||||||||||||||
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. The This is what it would look like to use However, after doing that clean-up it becomes easier to spot other problems (not new in this PR)
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. @andrewkroh thanks for the detailed feedback! I updated the mapping according to your suggestion. A couple of questions:
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 don't know that elastic-package has this capability yet. I have an unofficial means that I occasionally use to audit the mappings.
It's hard to prescribe one solution because it depends on how the data is being used and what problem they encounter (types of queries or aggregations they are performing). One solution for conflicts mentioned in this blog post is to apply a runtime field to the old index to make the field a keyword. 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. Since we are adhering to ECS, should we even define the rule fields that are of 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. @kcreddy @andrewkroh Looking at the value of the
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. AFAIU the idea of In our case, following are
Comparing the above dynamic mappings with ECS rule fields, it matches correctly.
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. @kcreddy now I see how |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,4 +29,4 @@ | |
- name: event.type | ||
external: ecs | ||
- name: observer.vendor | ||
external: ecs | ||
external: ecs |
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 don't think this is the correct way to do this; the versions that have been released should continue to exist.
Also, what is it that makes this a 2.0.0? What is breaking here?
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.
what would be the correct way to do that? From a user's point of view if they never enabled "show beta integrations", they would upgrade from 1.8.1 (latest released non-preview version) to 2.0.0 and would get all the changes from
1.9.0-preview*
with the change, and as a user I would expect to see this as a part of 2.0.0 changelogthis is reflected in the changelog
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.
The change here inserts a bunch of breaking changes before a bugfix that had already been released as a 1.9.0 preview. If correct chronological order had been used and they were added as a 1.9.0, the full set of 1.9.0 changes would not have been breaking; there would have been a single new version entry looking like this:
(leaving all the other already published versions in tact)
then you could have added this breaking enhancement:
This is not an academic issue; there is a bugfix that is being hidden here from people who may not want to upgrade across a break, and also blocks them from getting non-breaking enhancements. Admittedly, there is a fix in 1.8.something hidden in a backport branch, so the bugfix is less of a concern, but the non-breaking enhancements are still an issue.
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.
The problem is that 1.9.0-preview1 and 1.9.0-preview3 introduce the latest transforms, which we plan to release as part of the CDR initiative. These changes require the 8.16.x version of the stack; that's why we were asked to hold them in an open PR until closer to the 8.16 release date, as the flow with preview is not advised to be used. So, I can't release 1.9.0 just yet. Additionally, there are problems that we found while testing the CDR approach, together with the product folks, which require breaking changes before we can release transforms. That's why I'm skipping the 1.9.0 release and moving to 2.0.0, as all the changes there are bundled under CDR. We don't want users to be able to install the version with the transforms before the breaking changes are made. I can move the breaking change entries to the top of the changelog if you think that this would help.
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 is deeply dissatisfying. I have removed my block.