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

fix(clustering) localize config_version to avoid race condition from #8818

Merged
merged 1 commit into from
May 19, 2022
Merged

fix(clustering) localize config_version to avoid race condition from #8818

merged 1 commit into from
May 19, 2022

Conversation

fffonion
Copy link
Contributor

@fffonion fffonion commented May 19, 2022 •
edited
Loading

yield

Summary

update_config can yield, so we need to cache fields in self, otherwise it might got updated (on L89) before we read it (on L208), and causes L202 become false next time, thus prevent next ConfigSync being executed. This probably become more visiable as we introduce more yield from #8800.

Full changelog

  • localize config_version to avoid race condition from yield

@fffonion fffonion requested a review from a team as a code owner May 19, 2022 08:54
@fffonion
Copy link
Contributor Author

Other issus observed, but out of scope of this PR:

  • All protocols run export_deflate_config_payload regardless of there’s clients connected or not, we can shortcut it so it doesn't unnecessarilu double CPU load.
  • Version negotiation not working sometimes, in e2e test json protocol might end up using wrpc protocol.

@StarlightIbuki
Copy link
Contributor

Not sure if we should include it but: the data_plane.lua has different handling of self.next_hashes and self.next_hash compare to wrpc_data_plane.lua.

@fffonion fffonion merged commit f84bdde into master May 19, 2022
@fffonion fffonion deleted the fix/flaky-wrpc branch May 19, 2022 10:23
ADD-SP added a commit that referenced this pull request May 19, 2022
@ADD-SP
Squashed commit of the following:
9329427 
commit f84bdde
Author: Wangchong Zhou <fffonion@gmail.com>
Date:   Thu May 19 18:23:05 2022 +0800

    fix(clustering) localize config_version to avoid race condition from (#8818)

    yield

    `update_config` can yield, so we need to cache fields in `self`, otherwise it might got updated (on L89) before we read it (on L208), and causes L202 become false next time, thus prevent next ConfigSync being executed. This probably become more visiable as we introduce more `yield` from #8800.

commit 6558e99
Author: Thijs Schreijer <thijs@thijsschreijer.nl>
Date:   Thu May 19 09:39:12 2022 +0200

    feat(api) report plugin versions on server (#8810)

    * feat(api) report plugin versions on server
    * add changelog entry
    * update to object containing version: this allows for easier extension later, without breaking changes

commit fba1993
Author: Vinicius Mignot <vinicius.mignot@gmail.com>
Date:   Thu May 12 16:17:00 2022 -0300

    fix(balancer) set target status using hostname

    When target is added using hostname, it was not possible to update its
    health status using only the hostname. This change fixes that issue.

commit ce5e6a2
Author: Xumin <100666470+Suika-Kong@users.noreply.github.com>
Date:   Wed May 18 21:50:21 2022 +0800

    perf(pdk) faster request.get_header (#8716)

    * perf(pdk) faster reqeust.get_header
    * typo
    * cache ngx.var
    * more effecient implementation of string process
    * no need to lower
    * bug fix
    * try to archive better performance
    * style
    * bug fix

commit 6a0a579
Author: Qi <add_sp@outlook.com>
Date:   Wed May 18 20:38:14 2022 +0800

    tests(*) make the `host` and `port` of `grpcbin` configurable (#8625)

commit 9da3dee
Author: Isa Farnik <isa@konghq.com>
Date:   Tue May 17 10:26:23 2022 -0700

    [ENGEN-450] chore(changelog) debian 8 deprecation notice (#8807)

commit e80acd0
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri May 13 11:36:48 2022 +0300

    chore(db) small cleanups on off strategy dao

commit 337122e
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri May 13 10:22:53 2022 +0300

    perf(schema) no deep copy on select on process auto fields

    It is inefficient to create deep copies of tables when e.g. looping through
    database rows. In my testing with uploading some 64k routes with dbless
    (which calls process auto fields twice), this can cut the time looping the
    data by 1/4th. It also generates much less garbage.

    I searched our code bases where we use "select" context, and could not
    find anything that might break because of this.

commit 88e60a2
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Tue May 17 09:47:07 2022 +0300

    perf(db) yield on DB-less daos methods

    DBless doesn't naturally yield (not with `shared dict`, nor with `lmdb`). This may cause
    latency spikes when iterating over bigger lists, e.g. `kong.db.routes:each()`. This commit
    adds some yields so that iterating doesn't fully block the worker from doing other work
    too, so this is about cooperative multitasking.

commit 86f67e5
Author: Qirui(Keery) Nie <windmgc@gmail.com>
Date:   Tue May 17 14:05:25 2022 +0800

    feat(plugins/aws-lambda) accept string type `statusCode` under proxy integration mode

    Co-authored-by: Datong Sun <dndx@idndx.com>

commit 46eeef8
Author: Qi <add_sp@outlook.com>
Date:   Tue May 17 00:46:26 2022 +0800

    tests(helpers) make `host` and `port` of `Zipkin` configurable (#8626)

    Move zipkin default host and port into spec/helpers and update plugin test to use those.

commit 82fa99d
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Mon May 16 19:31:05 2022 +0300

    chore(migrations) remove deprecated Cassandra migrations helpers (#8781)

    This library had only one function left uncommented and that
    was about `Cassandra`. As `Cassandra` is deprecated with upcoming
    `3.0.0`, we don't use / need this anymore, thus removing it.

commit dd7f298
Author: Michael Martin <flrgh@protonmail.com>
Date:   Mon May 16 09:24:07 2022 -0700

    tests(rockspec) improve validation script for rockspec file (#8801)

commit 4759a29
Author: Datong Sun <datong.sun@konghq.com>
Date:   Tue May 10 02:57:55 2022 -0700

    chore(rockspec) add missing rockspec entry for
    `kong.db.migrations.core.016_280_to_300` which caused the packaging
    failure before

    Thanks @hutchic @flrgh for pointing out.

commit 49c3ae7
Author: Datong Sun <datong.sun@konghq.com>
Date:   Thu Apr 28 01:42:48 2022 -0700

    Revert "Revert "feat(dao) use `cache_key` for target uniqueness detection" (#8705)"

    This reverts commit 579537b.

commit 1439f9c
Author: Thijs Schreijer <thijs@thijsschreijer.nl>
Date:   Thu May 12 12:16:38 2022 +0200

    chore(httplog) bump version for breaking change (#8792)

commit b440737
Author: Thijs Schreijer <thijs@thijsschreijer.nl>
Date:   Wed May 11 18:15:26 2022 +0200

    fix(http-log) set headers with single value, not array (#6992)

commit 22f4cb2
Author: Colin Hutchinson <hutchic@users.noreply.github.com>
Date:   Wed May 11 13:07:12 2022 +0000

    chore(ci) adjust the version string format for our nightlies

commit d09c66b
Author: Michael Martin <flrgh@protonmail.com>
Date:   Wed May 11 06:04:17 2022 -0700

    docs(changelog) add entry for http-stream API improvements (#8750)

commit 1265280
Author: Mayo <i@shoujo.io>
Date:   Wed May 11 21:00:49 2022 +0800

    perf(clustering) log `push_config` duration to help debugging config export performance

    Co-authored-by: Datong Sun <datong.sun@konghq.com>
    Co-authored-by: Harry <harrybagdi@gmail.com>

commit 78fa687
Author: Michael Martin <3277009+flrgh@users.noreply.github.com>
Date:   Tue May 10 18:09:26 2022 -0700

    tests(rockspec) add a script to validate rockspec file

commit d569af6
Author: Javier <javier.guerra@konghq.com>
Date:   Tue May 10 07:13:56 2022 -0500

    chore(plugins) remove deprecated BasePlugin (#7961)

    * chore(plugins) remove deprecated BasePlugin

    It has been removed from the docs for some time, but the core still
    has to check each method to see if it's actually reimplemented or just
    inherited.

    Updated several fixture plugins that still used it.

    Co-authored-by: Alan Boudreault <alan@alanb.ca>

commit 23f50ea
Author: Chrono <chrono_cpp@me.com>
Date:   Sat May 7 15:42:05 2022 +0800

    chore(tools) small optimization for grpc (#8763)

commit c5fd723
Author: Zachary Hu <huzachary@icloud.com>
Date:   Thu May 5 15:25:16 2022 +0800

    feat(pdk) support HTTP/2 on Admin API server by adding new PDK function `nginx.get_statistics()` for fetching Nginx statistics

    Retired the `/nginx_status` location block and use LuaJIT FFI to fetch the counters directly instead.

    Co-authored-by: Datong Sun <datong.sun@konghq.com>

commit 0b000d2
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Mon May 2 14:50:05 2022 +0300

    fix(cmd) check db connection on reload

    Database connection may be changed on `kong reload`, so it is good that we
    check that before we signal to actual Kong server process to `reload`.

commit bc2879d
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Sun Apr 24 18:41:55 2022 +0300

    fix(conf) remove sensitive turns resolved configuration values back to references

    When Vault references are used in `kong.conf` settings, these may be displayed in
    plain in Kong Admin API, e.g. in `http :8001` (except the `pg_password,
    `cassandra_password`, and `pg_ro_password` which were masked properly).
    This commit turns configuration values back to references as part of removing
    sensitive values (`conf_loader.remove_sensitive`).

commit 9fa4647
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri Apr 22 11:21:19 2022 +0300

    fix(conf) allow only the enabled vaults

    There was some logic that allowed the default bundled vaults
    even in case they were not enabled. This commit fixes that.

commit d55d33b
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Tue Apr 19 20:46:20 2022 +0300

    fix(conf) ngx.socket.tcp not available on init

    Kong's initialization parses Kong configuration twice:

    1. first the config is parsed in timer context by kong start (cli)
    2. then the config is parsed again in init context when kong server is started

    Many Vault implementations will require the availability of `ngx.socket.tcp`
    to be able to fetch secrets. Unfortunately the `ngx.socket.tcp` is not available
    on init or init worker phases, but it works on timer context.

    Alternative approach would be to fetch secrets using `LuaSocket` on init phase,
    but that would mean that each secret is fetched twice (added latency + possible
    costs of accessing Vaults). Also, it is impossible to make `LuaSocket` and
    `ngx.socket.tcp` fully compatible, but there is one project that at least tried it:
    https://github.com/thibaultcha/lua-resty-socket

    This commit takes yet another approach:
    1. it fetches secret on CLI
    2. it passes secrets to server via environment variable
    3. except on `kong reload` it passes it to server via file `.kong_process_secrets`

    The error on current master branch looks like this:
    ```
    Error: ./kong/cmd/start.lua:64: nginx: [error] init_by_lua error: ./kong/globalpatches.lua:396: no request found
    ```

    This commit fixes it.

commit 79ad5fc
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed Apr 13 12:43:47 2022 +0300

    fix(conf) infer vault references

    Kong escapes e.g. `"#"` with `"\#"`, and these need to be removed before
    passing them to vault functions as otherwise reference like:

    ```
    {vault://env/pg-password#1}
    ```

    Doesn't work as it gets past as:

    ```
    {vault://env/pg-password\#1}
    ```

    This fixes that.

commit 2554988
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed May 4 19:04:50 2022 +0300

    chore(deps) bump resty.healthcheck from 1.5.0 to 1.5.1 (#8755)

    * Fix: avoid breaking active health checks when adding or removing targets.

commit fb00d31
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed May 4 16:41:18 2022 +0300

    chore(deps) bump luacheck (dev dep) from 0.26.0 to 0.26.1 (#8756)

    - Exempt special builtin \_ENV from 214 warning

    - In case of no home environment, default to caching in CWD (#60)
    - Add multi-thread support to container (#59)

    - Tweak warning message for 214 to be more explicit

commit 9376948
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed May 4 16:40:49 2022 +0300

    chore(deps) bump luasec from 1.0.2 to 1.1.0 (#8754)

    * Fix missing DANE flag
    * Remove unused parameter in https.lua

commit 54d46b9
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed May 4 16:00:37 2022 +0300

    chore(deps) bump resty.openssl from 0.8.7 to 0.8.8 (#8753)
    - **ctx:** use global ctx where request is unavailable [e3590cf](fffonion/lua-resty-openssl@e3590cf)
    - **x509.extension:** correct X509V3_CTX size for OpenSSL 3.0 [0946c59](fffonion/lua-resty-openssl@0946c59)
    - **x509.extension:** add X509V3_set_issuer_pkey in OpenSSL 3.0 [dbd3f74](fffonion/lua-resty-openssl@dbd3f74)
    - **x509.store:** add set_purpose and verify_method parameter [b7500fe](fffonion/lua-resty-openssl@b7500fe)

commit 6163945
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Wed May 4 15:19:43 2022 +0300

    chore(deps) bump openssl from 1.1.1n to 1.1.1o (#8752)

    Fixed a bug in the c_rehash script which was not properly sanitising shell
    metacharacters to prevent command injection
    ([CVE-2022-1292](https://www.openssl.org/news/vulnerabilities.html#CVE-2022-1292)).

commit 0973036
Author: Colin Hutchinson <hutchic@users.noreply.github.com>
Date:   Thu Apr 28 17:05:38 2022 +0000

    chore(ci): fix the test packaging (#8723)

    * chore(ci): dummy commit to test ci
    * fix(ci): use relative path
    * Delete touchfile

commit 1bfdf97
Author: Colin Hutchinson <hutchic@users.noreply.github.com>
Date:   Mon Apr 25 19:58:49 2022 +0000

    test(packaging): do a quick validation that Kong can viably be packaged, installed and used (#8707)

commit 86de704
Author: Murillo <103451714+gruceo@users.noreply.github.com>
Date:   Wed Apr 20 20:18:14 2022 -0300

    fix(cp) proper error handling for export_deflated_reconfigure_payload

commit 612648c
Author: Murillo <103451714+gruceo@users.noreply.github.com>
Date:   Wed Apr 13 11:28:29 2022 -0300

    fix(wrpc) do a pcall for all export_deflated_reconfigure_payload calls

    We are already wrapping some calls to
    `export_deflated_reconfigure_payload()` inside a pcall in the
    `wrpc_control_plane.lua` file. This change is doing a pcall in all the
    remaining calls to `export_deflated_reconfigure_payload()` in this file
    to avoid the CP crash whenever we find errors during initialization of
    modules for example.

commit 3c89fa1
Author: Murillo <103451714+gruceo@users.noreply.github.com>
Date:   Mon Apr 11 16:05:09 2022 -0300

    fix(cp) do a pcall for all calls to export_deflated_reconfigure_payload

    We are already wrapping some calls to
    `export_deflated_reconfigure_payload()` inside a pcall in the
    `control_plane.lua` file. This change is doing a pcall in all the
    remaining calls to `export_deflated_reconfigure_payload()` in this file
    to avoid the CP crash whenever we find errors during initialization of
    modules for example.

commit 6f20f2f
Author: Enrique García Cota <kikito@gmail.com>
Date:   Fri Apr 22 15:18:24 2022 +0200

    tests(hybrid) mark test as flaky (#8713)

commit fb8aa2d
Author: Suika <100666470+Suika-Kong@users.noreply.github.com>
Date:   Fri Apr 22 01:24:15 2022 +0800

    fix(pdk) ignore user set Tranfer-Encoding (#8698)

commit 31ca6ea
Author: Colin Hutchinson <hutchic@users.noreply.github.com>
Date:   Thu Apr 21 11:33:23 2022 +0000

    chore(release): cleanup the Jenkins release logic (#8706)

commit 39dd728
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Thu Apr 21 13:32:50 2022 +0300

    feat(clustering) atomic export of declarative config with Postgres

    This minimizes the possibilities of inconsistencies in exported config, especially under high Admin API update traffic.

commit 579537b
Author: Colin Hutchinson <hutchic@users.noreply.github.com>
Date:   Wed Apr 20 18:00:04 2022 +0000

    Revert "feat(dao) use `cache_key` for target uniqueness detection" (#8705)

    This reverts commit 9eba2a1.

commit a05cc4c
Author: Vinicius Mignot <vinicius.mignot@gmail.com>
Date:   Tue Apr 19 16:42:12 2022 -0300

    docs(CHANGELOG) added fix entry

commit 9a65902
Author: Vinicius Mignot <vinicius.mignot@gmail.com>
Date:   Tue Apr 19 15:41:43 2022 -0300

    fix(balancer) do not reschedule resolve timer when reloading

commit f6aae6f
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Tue Apr 19 17:56:27 2022 +0300

    chore(deps) bump luarocks 3.8.0 to 3.9.0 (#8700)

    * `builtin` build mode now always respects CC, CFLAGS and LDFLAGS
    * Check that lua.h version matches the desired Lua version
    * Check that the version of the Lua C library matches the desired Lua version
    * Fixed deployment of non-wrapped binaries
    * Fixed crash when `--lua-version` option is malformed
    * Fixed help message for `--pin` option
    * Unix: use native methods and don't always rely on $USER to determine user
    * Windows: use native CLI tooling more
    * macOS: support .tbd extension when checking for libraries
    * macOS: add XCode SDK path to search paths
    * macOS: add best-effort heuristic for library search using Homebrew paths
    * macOS: avoid quoting issues with LIBFLAG
    * macOS: deployment target is now 11.0 on macOS 11+
    * added DragonFly BSD support
    * LuaRocks test suite now runs on Lua 5.4 and LuaJIT
    * Internal dependencies of standalone LuaRocks executable were bumped

commit eb9a8ba
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Mon Apr 11 16:35:08 2022 +0300

    perf(conf) localize variables needed for configuration parsing

    Just localizes some variable for a faster configuration parsing, and tidier
    code.

commit 951b93f
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Mon Apr 11 15:57:22 2022 +0300

    fix(conf) properly support vault configurations with process secrets

    Default vault configurations can be configured with Kong configuration.

    For example using environment variables:

    - `KONG_VAULT_ENV_PREFIX=vault_`
    - `KONG_VAULT_HCV_TOKEN=xxx`

    Previously these settings were not honoured when kong configuration references
    were dereferenced. This fixes that issue.

commit 3d583c8
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Mon Apr 11 12:49:35 2022 +0300

    refactor(pdk) vault pdk to be more like rest of the pdk modules

    Refactor Vault PDK to follow other Kong PDK modules. This means that functions
    are created inside `.new` function. This has benefit of being able to access
    up-value `self`, which means that no direct references to global `kong` is needed.

    In general, it makes testing and mocking easier too.

    I need this so I can pass some initial configuration very early on when Kong does
    process secrets resolving of Kong configuration references.

commit 5156596
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri Apr 8 16:33:33 2022 +0300

    feat(vaults) store dao references in $refs (needed for rotation)

    When there are references used in dao fields with `referenceable=true`,
    Kong replaces the references with values when the data is read
    (excluding admin api and control planes). When Kong replaces the reference,
    it is basically lost, and thus the automatic secret rotation cannot be
    implemented. This commit stores the references on returned entities to
    `"$refs"` property:

    ```
    local certificate = kong.db.certificates:select(...)
    -- the possible reference can be found here:
    print(certificate["$refs"].key)
    ```

    There will be helper functions so `"$refs"` property is not intended to
    end users.

commit ac69743
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri Apr 8 16:00:49 2022 +0300

    fix(vaults) do not leak resolved vault references to .kong_env file

    When Kong prepares a `prefix` directory, it also stores current environment
    related to Kong in file called `.kong_env`. As Kong resolves the Vault
    references when it starts, the resolved values got leaked to `.kong_env`
    file. This was partly because for `vaults-beta` we didn't yet implement
    secret rotation, and we decided to also not keep the references around
    when they were resolved. Not that we have added the `"$refs"` property
    to `kong.configuration`, we can replace the values of configuration with
    the references before we write the `.kong_env` file.

    This commit fixes that.

commit 7f13cbc
Author: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Date:   Fri Apr 8 15:53:29 2022 +0300

    feat(vaults) store configuration references in $refs (needed for rotation and .kong_env cleanup)

    Kong vault references like `{vault://env/my-env-var}` when used in Kong configuration are replaced
    with actual secrets. This makes it hard to implement secret rotation as the reference is lost when
    it is replaced. This commit stores the original references on a side:

    ```lua
    kong.configuration[$refs][<key>] = <reference>
    ```

commit bffa4af
Author: Mayo <i@shoujo.io>
Date:   Tue Apr 19 17:57:40 2022 +0800

    chore(ci) changelog label

    Any PR includes a changelog will add a “core/docs” label which is unnecessary, this PR added an extra label 'changelog' to detect changelog file changes.

commit 9eba2a1
Author: yankun-li-kong <77371186+yankun-li-kong@users.noreply.github.com>
Date:   Tue Apr 19 19:27:23 2022 +0900

    feat(dao) use `cache_key` for target uniqueness detection

    Add new `cache_key(upstream, target)` in targets table for atomic
    uniqueness detection.
    Delete useless targets uniqueness detection functions.
    Targets API returns `409` when creating/updating delicate targets.
    Add migration functions to add `cache_key` column,
    delete duplicate targets and add `cache_key` for existing targets.

    Co-authored-by: Mayo <i@shoujo.io>

commit d7a8e66
Author: Mayo <i@shoujo.io>
Date:   Tue Apr 19 17:36:33 2022 +0800

    fix(ldap-auth) free internal pointer after covert to lua string (#8696)

commit d4bdae5
Author: Mayo <i@shoujo.io>
Date:   Tue Apr 19 12:08:09 2022 +0800

    refactor(ldap-auth) openssl ffi based asn1 parser/decoder (#8663)

    Replace asn1 parser/decoder with openssl ffi based functions.

commit 79f362d
Author: Wheeler Law <whelderwheels613@gmail.com>
Date:   Mon Apr 18 04:51:32 2022 -0500

    chore(CODEOWNERS) add `CODEOWNERS` file to the repo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StarlightIbuki StarlightIbuki StarlightIbuki approved these changes

@dndx dndx dndx approved these changes

@bungle bungle bungle approved these changes

Assignees
No one assigned
Labels
core/clustering size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fffonion @StarlightIbuki @bungle @dndx