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

Bump openresty to 1.21.4.3 #1461

Merged
merged 4 commits into from
May 2, 2024
Merged

Conversation

tkan145
Copy link
Contributor

@tkan145 tkan145 commented Apr 29, 2024

What

Upgrade openresty to 1.21.4.3. https://issues.redhat.com/browse/THREESCALE-10601

Verification steps

No code changes and all tests pass so I'll keep the verification steps simple:

  • Create an apicast-config.json file with the following content
cat <<EOF >apicast-config.json
{
  "services": [
    {
      "backend_version": "1",
      "id": "1",
      "proxy": {
         "hosts": ["one"],
         "api_backend": "https://echo-api.3scale.net:443",
         "authentication_method":"2",
        "backend": {
          "endpoint": "http://127.0.0.1:8081",
          "host": "backend"
        },
        "policy_chain": [
          {
            "name": "apicast.policy.apicast"
          }
        ],
        "proxy_rules": [
          {
            "http_method": "GET",
            "pattern": "/",
            "metric_system_name": "hits",
            "delta": 1,
            "parameters": [],
            "querystring_parameters": {}
          }
        ]
      }
    }
  ]
}
EOF
  • Checkout this branch and start dev environment
make development
make dependencies
  • Verify the openresty version
bash-4.4$ openresty -v

It should output the following nginx version: openresty/1.21.4.3

  • Run apicast locally
THREESCALE_DEPLOYMENT_ENV=staging APICAST_LOG_LEVEL=warn APICAST_WORKER=1 APICAST_CONFIGURATION_LOADER=lazy APICAST_CONFIGURATION_CACHE=0 THREESCALE_CONFIG_FILE=apicast-config.json ./bin/apicast
  • Capture apicast IP
APICAST_IP=$(docker inspect apicast_build_0-development-1 | yq e -P '.[0].NetworkSettings.Networks.apicast_build_0_default.IPAddress' -)
  • Send a request
curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/test?user_key="

The response should be HTTP/1.1 200

< HTTP/1.1 200 OK
< Server: openresty
< Date: Wed, 01 May 2024 02:44:05 GMT
< Content-Type: application/json
< Content-Length: 568
< Connection: keep-alive
< x-3scale-echo-api: echo-api/1.0.3
< vary: Origin
< x-content-type-options: nosniff
< x-envoy-upstream-service-time: 1

@tkan145 tkan145 force-pushed the openresty-1.21.4.3 branch 2 times, most recently from 810a002 to a8d3a6c Compare April 30, 2024 06:21
tkan145 added 3 commits May 1, 2024 11:42
Previously, header contains space or control character were
considered invalid and were ignore by default. But in nginx
1.21.1, it now always returns an error instead if spaces or
control characters are used in a header
In 1.21.4 openresty sock:handshake return cdata type instead of
userdata

Reference: openresty/lua-nginx-module#1602
Previously context.apply() is set in PR#1038. Due to ctx_ref
is nil in the test, the context also has nil value. However in
openresty 1.21.4 set ngx.ctx to a non-table value is considered
harmful and will return error. Therefore, we need to remove
context.apply() from the test.

Ref: openresty/lua-resty-core#333
@tkan145 tkan145 force-pushed the openresty-1.21.4.3 branch from a8d3a6c to f8f8017 Compare May 1, 2024 01:48
@tkan145 tkan145 marked this pull request as ready for review May 1, 2024 02:47
@tkan145 tkan145 requested a review from a team as a code owner May 1, 2024 02:47
Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

🎉

@tkan145 tkan145 merged commit a5e47ed into 3scale:master May 2, 2024
12 checks passed
@tkan145 tkan145 deleted the openresty-1.21.4.3 branch May 2, 2024 23:33
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.

None yet

2 participants