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

fix(tests): don't use 2.x style path regex where requiring 3.x one #3489

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Feb 6, 2023

What this PR does / why we need it:

This fixes the following deck warning:

time="2023-02-03T13:45:21+01:00" level=debug msg="converting configuration to deck config" kong_url="http://172.18.1.1:8001/"
1 unsupported routes' paths format with Kong version 3.0
or above were detected. Some of these routes are (not an exhaustive list):

18a19edb-0b01-4d9a-988b-146c3c086e04

Please upgrade your configuration to account for 3.0
breaking changes using the following command:

deck convert --from kong-gateway-2.x --to kong-gateway-3.x

This command performs the following changes:
  - upgrade the `_format_version` value to `3.0`
  - add the `~` prefix to all routes' paths containing a regex-pattern

These changes may not be correct or exhaustive enough.
It is strongly recommended to perform a manual audit
of the updated configuration file before applying
the configuration in production. Incorrect changes will result in
unintended traffic routing by Kong Gateway.

For more information about this and related changes,
please visit: https://docs.konghq.com/deck/latest/3.0-upgrade

updating route 8ec040df-5096-467e-b571-da89a2803a09.regex-toggle.httpbin..80  {
   "https_redirect_status_code": 426,
   "id": "18a19edb-0b01-4d9a-988b-146c3c086e04",
   "name": "8ec040df-5096-467e-b571-da89a2803a09.regex-toggle.httpbin..80",
   "path_handling": "v0",
   "paths": [
-    "~/test_ingress_class_regex_toggle/\d+"
+    "/test_ingress_class_regex_toggle/\d+"
   ],
   "preserve_host": true,
   "protocols": [
     "http",
     "https"
   ],
   "regex_priority": 0,
   "request_buffering": true,
   "response_buffering": true,
   "service": {
     "id": "1f195e3d-4b76-4b27-b03b-14c69f84f02d",
     "name": "8ec040df-5096-467e-b571-da89a2803a09.regex-toggle.httpbin.80"
   },
   "strip_path": true,
   "tags": [
     "managed-by-ingress-controller"
   ]
 }

Sadly there's no way to check/assert this now (or is there?). Perhaps we could introduce event for deck conversion warning/errors? 🤔

@codecov
Copy link

codecov bot commented Feb 6, 2023

Codecov Report

Base: 73.9% // Head: 73.8% // Decreases project coverage by -0.1% ⚠️

Coverage data is based on head (30361d2) compared to base (65e07cf).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3489     +/-   ##
=======================================
- Coverage   73.9%   73.8%   -0.1%     
=======================================
  Files        118     118             
  Lines      13990   13990             
=======================================
- Hits       10341   10335      -6     
- Misses      2987    2992      +5     
- Partials     662     663      +1     
Impacted Files Coverage Δ
internal/dataplane/parser/translate_utils.go 88.9% <0.0%> (-5.6%) ⬇️
...nternal/controllers/gateway/tlsroute_controller.go 71.4% <0.0%> (-2.1%) ⬇️
...nternal/controllers/gateway/udproute_controller.go 71.6% <0.0%> (+2.1%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pmalek pmalek marked this pull request as ready for review February 6, 2023 12:43
@pmalek pmalek requested a review from a team as a code owner February 6, 2023 12:43
@pmalek pmalek enabled auto-merge (squash) February 6, 2023 12:43
@pmalek pmalek linked an issue Feb 6, 2023 that may be closed by this pull request
1 task
@pmalek
Copy link
Member Author

pmalek commented Feb 6, 2023

@czeslavo
Copy link
Contributor

czeslavo commented Feb 6, 2023

Sadly there's no way to check/assert this now (or is there?). Perhaps we could introduce event for deck conversion warning/errors? 🤔

That would be similar to #3205, but for deck. The mentioned issue explicitly states that adapting the deck is out of scope. I allowed myself to create an issue that will track deck's side: #3492.

@pmalek pmalek merged commit 53152a1 into main Feb 6, 2023
@pmalek pmalek deleted the fix-route-regex-in-TestIngressClassRegexToggle branch February 6, 2023 13:55
@pmalek pmalek added the fix label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestIngressClassRegexToggle outputs deck notice that it shouldn't(?)
2 participants