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

Add proper dependency resolution for dns-packet #456

Merged
merged 1 commit into from
Jun 16, 2021
Merged

Add proper dependency resolution for dns-packet #456

merged 1 commit into from
Jun 16, 2021

Conversation

tmarkley
Copy link
Contributor

@tmarkley tmarkley commented Jun 8, 2021 •
edited
Loading

Description

Dependabot bumped the dependency from 1.3.1 to 1.3.4 in #381, but it edited the yarn.lock file directly. This is not the recommended short- or long-term solution.

Signed-off-by: Tommy Markley [email protected]

$ yarn why dns-packet
yarn why v1.22.10
[1/4] Why do we have the module "dns-packet"...?
[2/4] Initialising dependency graph...
warning Resolution field "[email protected]" is incompatible with requested version "typescript@~3.7.2"
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#@osd#ui-framework#webpack-dev-server#bonjour#multicast-dns" depends on it
   - Hoisted from "_project_#@osd#ui-framework#webpack-dev-server#bonjour#multicast-dns#dns-packet"
info Disk size without dependencies: "44KB"
info Disk size with unique dependencies: "152KB"
info Disk size with transitive dependencies: "152KB"
info Number of shared dependencies: 2
Done in 1.48s.

The longer-term solution involves upgrading webpack and webpack-dev-server.

Testing

Changes for #456, #457, #458, #460, #461, and #476 were merged into the same branch and tested together. All tests passed.

Unit

$ yarn test:jest
...
Test Suites: 23 skipped, 1411 passed, 1411 of 1434 total
Tests:       256 skipped, 9 todo, 10364 passed, 10629 total
Snapshots:   2363 passed, 2363 total
Time:        156.76 s
Ran all test suites.
Done in 159.62s.

Integration

$ yarn test:jest_integration
...
Test Suites: 2 skipped, 48 passed, 48 of 50 total
Tests:       19 skipped, 418 passed, 437 total
Snapshots:   73 passed, 73 total
Time:        222.992 s
Ran all test suites.
Done in 225.23s.

Functional

Screen Shot 2021-06-10 at 10 31 39 PM

Issues Resolved

N/A

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

Add proper dependency resolution for dns-packet
1ae09d3 
Dependabot bumped the dependency from 1.3.1 to 1.3.4 in opensearch-project#381,
but it edited the yarn.lock file directly. This is not the
recommended short- or long-term solution.

Signed-off-by: Tommy Markley <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 1ae09d3

@tmarkley tmarkley closed this Jun 11, 2021
@tmarkley tmarkley reopened this Jun 11, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 1ae09d3

This was referenced Jun 11, 2021
Merged
Merged
Merged
Merged
@tmarkley tmarkley added dependencies Pull requests that update a dependency file v1.0.0 labels Jun 11, 2021
@tmarkley tmarkley marked this pull request as ready for review June 11, 2021 04:33
@tmarkley tmarkley requested review from kavilla, boktorbb and ananzh June 11, 2021 04:33
@tmarkley tmarkley mentioned this pull request Jun 11, 2021
Merged
5 tasks
ananzh
ananzh approved these changes Jun 11, 2021
View reviewed changes
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

mihirsoni
mihirsoni approved these changes Jun 15, 2021
View reviewed changes
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !! Thanks for the change.

@kavilla
Copy link
Member

kavilla commented Jun 15, 2021

Will this be back-ported to 1.x and 1.0?

boktorbb
boktorbb approved these changes Jun 15, 2021
View reviewed changes
Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

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

Looks good

@tmarkley
Copy link
Contributor Author

Will this be back-ported to 1.x and 1.0?

@kavilla Yes, we need these changes merged to 1.0 for GA to address security vulnerabilities.

@tmarkley tmarkley merged commit f9e47e9 into opensearch-project:main Jun 16, 2021
@tmarkley tmarkley deleted the deps-dns-packet branch June 16, 2021 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ananzh ananzh ananzh approved these changes

@boktorbb boktorbb boktorbb approved these changes

@kavilla kavilla kavilla approved these changes

@mihirsoni mihirsoni mihirsoni approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file v1.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tmarkley @opensearch-ci-bot @kavilla @mihirsoni @boktorbb @ananzh