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

chore(*) update Cargo structure for Dependabot #475

Merged
merged 5 commits into from
Jan 3, 2024

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Dec 21, 2023

End goal is improved integration with GitHub's security features:

  1. Automated security updates of dependencies in our end-users Rust crates in lib/.
  2. Automated periodic updates of dependencies in all Rust crates in lib/ and t/lib/ (end-users vs. test suite crates).

Changes:

  • Split the single Cargo workspace we had in two: lib/ and t/lib. This helps separating production vs. development crates for Dependabot.
  • Minor updates to auto/cargo to force it to use the same lib/ngx-wasm-rs/target/ directory as before instead of lib/target/ with the new workspace change.
  • Update dependabot.yml to configure several jobs for automated version bumps (distinct from security bumps).
  • Clean work/lib/wasm on make cleanup.
  • Always copy SDK example filters to work/lib/wasm due to cleanup change.
  • Rename the ngx-wasm-rs crate purely for consistency.
  • New make update target for quick cargo update in all workspaces.

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Merging #475 (6a7f2ad) into main (e7769a0) will decrease coverage by 0.29437%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #475         +/-   ##
===================================================
- Coverage   90.43593%   90.14156%   -0.29438%     
===================================================
  Files             46          46                 
  Lines           9818        9890         +72     
===================================================
+ Hits            8879        8915         +36     
- Misses           939         975         +36     

see 9 files with indirect coverage changes

Flag Coverage Δ
unit 90.24390% <ø> (ø)
valgrind 77.84247% <ø> (-2.89672%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@thibaultcha thibaultcha force-pushed the chore/dependabot-update branch from d68814c to 2bf9e11 Compare December 21, 2023 23:19
@thibaultcha thibaultcha changed the title chore(dependabot) update 'dependabot.yml' settings chore(*) update Cargo structure for Dependabot Dec 21, 2023
@thibaultcha thibaultcha force-pushed the chore/dependabot-update branch 7 times, most recently from 2fb6898 to 18a3549 Compare December 22, 2023 00:46
echo "$0: warning: the following functionality is not available for $ngx_addon_name:"
echo " $ngx_wasm_cargo_defines"
echo
echo "$0: warning: the following functionality is not available for $ngx_addon_name: $ngx_wasm_cargo_defines"
Copy link
Member Author

@thibaultcha thibaultcha Dec 22, 2023

Choose a reason for hiding this comment

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

Not an exit, use same format as the above echo "$0: error: to keep ./configure output more consistent.

@thibaultcha thibaultcha force-pushed the chore/dependabot-update branch from 76ddab7 to d36e773 Compare December 22, 2023 02:58
@thibaultcha
Copy link
Member Author

Ping @hishamhm @casimiro for ack of operational changes in this PR.

Ping @locao and @flrgh because I believe this may break the Gateway bazel builds of the Rust modules but I have not tried it yet.

@thibaultcha
Copy link
Member Author

Building Kong Gateway with Wasmer or V8 is already broken as of today, so we will merge this without further consideration for the Bazel build system.

@thibaultcha thibaultcha force-pushed the chore/dependabot-update branch from 40521c9 to da6e0db Compare January 3, 2024 01:41
@@ -1,5 +1,5 @@
[package]
name = "ngx-wasm"
name = "ngx-wasm-rs"
Copy link
Contributor

Choose a reason for hiding this comment

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

The Rust API guidelines recommend against -rs for the actual crate names:

https://rust-lang.github.io/api-guidelines/naming.html

Crate names should not use -rs or -rust as a suffix or prefix. Every crate is Rust! It serves no purpose to remind users of this constantly.

So you see many projects in the wild where the repo is foo-rs but the crate is foo.

Copy link
Member Author

@thibaultcha thibaultcha Jan 3, 2024

Choose a reason for hiding this comment

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

Given we will not publish this create to crates.io I preferred going for consistency so it would help distinguish it from the different Rust crates we have: "ngx-wasm-rs is the .so lib with extra features we embed into our builds". I looked at ngx-rust and its crate name is ngx; I'll revert this for now but a rename of both of these crates (or even a merge) is incoming in any case because all these names are more confusing than anything (also we should align with the Lua lib and name it with wasmx instead of wasm).

To better manage automated dependencies and security updates, and help
distinguish high-priority vs low-priority alerts.
Add a new `--install` mode to sdk.sh for copying the compiled .wasm
filters, if any. Now test.sh always installs SDK test filters, if any.

This allows `make cleanup` to properly clean the work/lib/wasm directory
and subsequently run `make test` again.
Thanks to recent refactor making them optional tools for the test suite.
@thibaultcha thibaultcha force-pushed the chore/dependabot-update branch from da6e0db to 6a7f2ad Compare January 3, 2024 17:45
@thibaultcha thibaultcha merged commit d033338 into main Jan 3, 2024
37 checks passed
@thibaultcha thibaultcha deleted the chore/dependabot-update branch January 3, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants