fix: remove nginx proxy router, fixes #5940 #6739
Conversation
github-actions
bot
added
bugfix
dependencies
|
Download the artifacts for this pull request:
See Testing a PR.
|
| @@ -318,7 +298,6 @@ func init() { | |||
| configGlobalCommand.Flags().String("xdebug-ide-location", "", "For less usual IDE locations specify where the IDE is running for Xdebug to reach it") | |||
| configGlobalCommand.Flags().Bool("use-traefik", true, "If true, use Traefik for ddev-router") | |||
| _ = configGlobalCommand.Flags().MarkDeprecated("use-traefik", "please use --router instead") | |||
| configGlobalCommand.Flags().String("router", globalconfigTypes.RouterTypeTraefik, fmt.Sprintf("Valid router types are %s, default is %s", strings.Join(globalconfigTypes.GetValidRouterTypes(), ", "), globalconfigTypes.RouterTypeDefault)) | |||
There was a problem hiding this comment.
I think we'd better mark the --router flag as deprecated, just like we did for --use-traefik above.
Or make it accept traefik only.
I was reluctant to just remove it right away. Do you disagree? It would be reasonable either way. It's obviously there in the source code. |
rfay
force-pushed
the
20241118_rfay_remove_nginx_proxy
branch
from
2a0e7b7
to
5683c16
Compare
I'm okay with it, just asked if this was intentional to leave it there. |
rfay
force-pushed
the
20241118_rfay_remove_nginx_proxy
branch
from
5683c16
to
609634e
Compare
|
I agree. |
rfay
force-pushed
the
20241118_rfay_remove_nginx_proxy
branch
2 times, most recently
from
c0f5aa9
to
43b3bb3
Compare
pkg/globalconfig/global_config.go
Outdated
| return fmt.Errorf("invalid router: %s, valid router types are %v", DdevGlobalConfig.Router, types.GetValidRouterTypes()) | ||
| output.UserOut.Printf("\nThe only valid router type is %s, but you have router: %s in your global configuration, using %s instead.\n", types.RouterTypeTraefik, DdevGlobalConfig.Router, types.RouterTypeTraefik) | ||
| DdevGlobalConfig.Router = types.RouterTypeTraefik | ||
| return nil |
There was a problem hiding this comment.
Return was added here to show a warning for the user each time, but when I tested it with:
router: nginx-proxy
xdebug_ide_location: "blablabla"DDEV didn't complain about wrong xdebug_ide_location.
I don't think it's worth skipping all the further validation checks here.
There was a problem hiding this comment.
Fixed, thanks for catching the erroneous return there.
Co-authored-by: Stanislav Zhuk <[email protected]>
rfay
force-pushed
the
20241118_rfay_remove_nginx_proxy
branch
from
3c3669a
to
abd08ec
Compare
rfay
force-pushed
the
20241118_rfay_remove_nginx_proxy
branch
from
abd08ec
to
f6c9506
Compare
There was a problem hiding this comment.
Thank you.
I tested upgrade/downgrade. DDEV did a great job in all cases.
Regarding the code in containers/ddev-nginx-proxy-router:
We need to keep it because we still use this image for versions 1.23.5 and earlier, and we are required to provide the source code (as requested by Docker-Sponsored Open Source Program). So, until the Docker image is available, we should keep it.
The Issue
Remove the deprecated nginx proxy router
Manual Testing Instructions
Try setting router to nginx-proxy router
Make sure ordinary things work