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(ci): use public url for fetching ngx_wasm_module #11240

Merged
merged 1 commit into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,6 @@ jobs:
- name: Checkout Kong source code
uses: actions/checkout@v3

- name: Set ngx_wasm_module path/branch vars
run: |
grep ^NGX_WASM_MODULE_BRANCH= .requirements >> $GITHUB_ENV || {
echo "ERROR: NGX_WASM_MODULE_BRANCH is not defined in .requirements"
exit 1
}
echo "NGX_WASM_MODULE_REMOTE=$PWD/ngx_wasm_module" >> $GITHUB_ENV

- name: Checkout ngx_wasm_module
uses: actions/checkout@v3
with:
repository: Kong/ngx_wasm_module
path: ${{ env.NGX_WASM_MODULE_REMOTE }}
ref: ${{ env.NGX_WASM_MODULE_BRANCH }}
token: ${{ secrets.GHA_KONG_BOT_READ_TOKEN }}

# these aren't necessarily used by all tests, but building them here will
# ensures that we have a warm cache when other tests _do_ need to build the
# filters
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ jobs:
uses: ./.github/workflows/build.yml
with:
relative-build-root: bazel-bin/build
secrets: inherit

lint-doc-and-unit-tests:
name: Lint, Doc and Unit tests
Expand Down
16 changes: 0 additions & 16 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -214,22 +214,6 @@ jobs:
# required for openssl 3.x config
cpanm IPC/Cmd.pm

- name: Set ngx_wasm_module path/branch vars
run: |
grep ^NGX_WASM_MODULE_BRANCH= .requirements >> $GITHUB_ENV || {
echo "ERROR: NGX_WASM_MODULE_BRANCH is not defined in .requirements"
exit 1
}
echo "NGX_WASM_MODULE_REMOTE=$PWD/ngx_wasm_module" >> $GITHUB_ENV

- name: Checkout ngx_wasm_module
uses: actions/checkout@v3
with:
repository: Kong/ngx_wasm_module
path: ${{ env.NGX_WASM_MODULE_REMOTE }}
ref: ${{ env.NGX_WASM_MODULE_BRANCH }}
token: ${{ secrets.GHA_KONG_BOT_READ_TOKEN }}

- name: Build Kong dependencies
if: steps.cache-deps.outputs.cache-hit != 'true'
env:
Expand Down
2 changes: 1 addition & 1 deletion build/openresty/wasmx/wasmx_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def wasmx_repositories():
new_git_repository,
name = "ngx_wasm_module",
branch = ngx_wasm_module_branch,
remote = KONG_VAR.get("NGX_WASM_MODULE_REMOTE", "git@github.com:Kong/ngx_wasm_module.git"),
Copy link
Member

Choose a reason for hiding this comment

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

Won't this impact the current wasmx team development / testing workflow - ie, how to easily test local changes of a nginx wasm module clone? We could leave it here for that, and add it to this list so that a change in it causes the invalidation of kong_bindings (I think @hishamhm brought this invalidation issue up few days ago : )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, good call. I'll rework this tomorrow/later this week with that in mind.

Copy link
Member

Choose a reason for hiding this comment

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

Yes we have many Bazel improvements we would like to make or else debugging ngx_wasm_module in the Gateway is going to be extremely impractical. All these needed updates are listed on page 3 of the integration document, if you're curious: https://docs.google.com/document/d/1Y-OL0QNb2CYVRfkEYuzLCc9dnbTVzb9mBiKL-fO7jUw/edit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the pull request with the NGX_WASM_MODULE_REMOTE handling intact. The only change made here is switching the remote to http to be more inline with how we are fetching other git dependencies.

remote = KONG_VAR.get("NGX_WASM_MODULE_REMOTE", "https://github.com/Kong/ngx_wasm_module.git"),
build_file_content = """
filegroup(
name = "all_srcs",
Expand Down
Loading