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

Jk/cumulus 3701 fix null rules (#3641) #3645

Merged
merged 1 commit into from
May 7, 2024

Conversation

Jkovarik
Copy link
Member

@Jkovarik Jkovarik commented May 6, 2024


Summary: Summary of changes

Addresses CUMULUS-3701

Changes

PR Checklist

  • Update CHANGELOG
  • Unit tests
  • Ad-hoc testing - Deploy changes and test manually
  • Integration tests

* Update core deps to v20.12.2

* Update unit test to rmove .sort

.sort is intermittently failing to order as expected, causing this
test to fail in local test at an alarming failure rate.    Updating
test to be more explicit/remove sort, however this needs to be
investigated prior to closing thie branch/PR/ticket

* Update kinesis test to node v20

JSON.parse now throws a different error for the merged test case.

* Update knex test to handle new subdependency throwing an Aggregate
error

* Revert "Update unit test to rmove .sort"

This reverts commit adbad84.

* Update got

* Revert "Update got"

This reverts commit a19f9d1.

* Upgrade ava/nock, fix ingest provider tests

* Reapply "Update got"

This reverts commit e652516.

* Update ingest module for node 20

This update has a couple of controversial changes in it:

Updating got to v14 means we're using a pure ESM module given sindre's
stance on not supporting common exports.  That's being done due to
incompatible changes in node streams requires at least got v12

Additionally there's a probable outstanding issue in got
sindresorhus/got#2341 related to node v20/fs
readstreams/nock and/or msw incompatibility (as well as a possible
open source contrib)

* Remove httpClient test mock/fix

Updating the existing apache server to return 200 on an existing
endpoint is far better than the prior commit approach in that it's a
valid/useful unit test as a result, with the technical/less tidy
downside of requiring the unit test stack to be working.

* Update local test stack configuration to restrict connection to localhost

* Update send-pan test/dependencies

* Update lambdas/async operation to deploy with node 20

* Update common to export a helper to import ESM got module

* Add docstring to importGot

* Remove unneeded imports

* Minor PR review fixes

* Update CHANGELOG

* Update CHANGELOG

* Reconfigure CI to not use unsafe perms modification

* Fix broken package.json

* Fix extran. dep issue

* Make error message test less specific

* Update test to not rely on aggregate internal errors

* Update packages/ingest/test/test-HttpProviderClient.js

Co-authored-by: etcart <37375117+etcart@users.noreply.github.com>

* Move importGot -> importEsm, introduce static import definition

* Fix inadvertant test commit

* Update rules helpers to remove all null key values from rule JSON

* Update CHANGELOG

* Update helper test titles for consistency

* Minor refactor/rename

* Fix typing errors in original code, update PR accordingly

* Fix inadvertant move of rule validation

* Update packages/api/endpoints/rules.js

Co-authored-by: etcart <37375117+etcart@users.noreply.github.com>

* Remove custom null removal method in favor of omitDeepBy

* Remove duplicative call to omitDeepBy

* Update packages/api/lib/rulesHelpers.js

Co-authored-by: etcart <37375117+etcart@users.noreply.github.com>

---------

Co-authored-by: etcart <37375117+etcart@users.noreply.github.com>
Copy link
Contributor

@npauzenga npauzenga left a comment

Choose a reason for hiding this comment

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

The backport commit looks good but I think we need the version bump in this minor version branch.

@Jkovarik
Copy link
Member Author

Jkovarik commented May 7, 2024

@npauzenga I believe that should be part of the release 18.2.x ticket

@Jkovarik Jkovarik merged commit 76f6824 into release-18.2.x May 7, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants