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

Set traefik as default router, fixes #4679 #4983

Merged
merged 33 commits into from
Jun 25, 2023
Merged

Set traefik as default router, fixes #4679 #4983

merged 33 commits into from
Jun 25, 2023

Conversation

rfay
Copy link
Member

@rfay rfay commented Jun 14, 2023 •
edited

The Issue

How This PR Solves The Issue

  • Traefik is default
  • New global config router: traefik or router: nginx-proxy
  • New ddev config global --router=traefik

TODO

  • Re-enable and merge TestCmdConfigGlobal - it's pending changes in Add global router_http(s)_port config, fixes #3240 #4640
  • Add test coverage for router into TestCmdConfigGlobal
  • ddev describe currently shows something other than "traditional" or whatever. Use the correct value.
  • Implement @mattstein 's new diagram
  • Implement @gilbertsoft suggestion of typed strings.
  • Add IsTraefik() instead of awkward if globalconfig.DdevGlobalConfig.Router == nodeps.TraefikRouter && status == SiteRunning
  • Use constants in initializations in globalconfig.New()

Manual Testing Instructions

  • Try switching between traefik and traditional, verify behavior in each.
  • Make sure that existing use-traefix configuration in global_config.yaml doesn't break anything.

Automated Testing Overview

  • Updated tests

Release/Deployment Notes

  • Info about this change goes into release notes

@github-actions
Copy link

github-actions bot commented Jun 14, 2023 •
edited

Download the artifacts for this pull request:

See Testing a PR

@rfay rfay force-pushed the 20230614_traefik_default branch from 271dc5b to df67c68 Compare June 15, 2023 20:44
@rfay rfay marked this pull request as ready for review June 15, 2023 21:27
@rfay rfay requested review from a team as code owners June 15, 2023 21:27
@rfay rfay requested review from gilbertsoft and tyler36 June 15, 2023 21:27
@rfay rfay mentioned this pull request Jun 16, 2023
Merged
8 tasks
@rfay rfay mentioned this pull request Jun 17, 2023
Merged
1 task
gilbertsoft
gilbertsoft requested changes Jun 18, 2023
View reviewed changes
Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

Had a frist look and found some minor conceptual issues which we should agree how to handle. Maybe also something to discuss on Tuesday.

pkg/nodeps/values.go Outdated Show resolved Hide resolved
pkg/nodeps/values.go Outdated Show resolved Hide resolved
pkg/nodeps/values.go Outdated Show resolved Hide resolved
pkg/globalconfig/values.go Outdated Show resolved Hide resolved
pkg/globalconfig/values.go Outdated Show resolved Hide resolved
pkg/globalconfig/global_config.go Outdated Show resolved Hide resolved
pkg/globalconfig/global_config.go Outdated Show resolved Hide resolved
pkg/ddevapp/ddevapp.go Outdated Show resolved Hide resolved
pkg/ddevapp/config.go Outdated Show resolved Hide resolved
mattstein
mattstein requested changes Jun 19, 2023
View reviewed changes
docs/content/users/configuration/config.md Outdated Show resolved Hide resolved
docs/content/users/configuration/config.md Outdated Show resolved Hide resolved
docs/content/users/configuration/traefik-router.md Outdated Show resolved Hide resolved
docs/content/users/configuration/traefik-router.md Outdated Show resolved Hide resolved
docs/content/users/configuration/traefik-router.md Outdated Show resolved Hide resolved
@mattstein
Copy link
Sponsor Collaborator

Here’s an updated file for /images/container-diagram.png:

container-diagram

@rfay rfay mentioned this pull request Jun 21, 2023
Merged
@rfay rfay force-pushed the 20230614_traefik_default branch from 52b5154 to 737258b Compare June 22, 2023 23:31
@rfay rfay requested a review from gilbertsoft June 23, 2023 21:27
gilbertsoft
gilbertsoft requested changes Jun 24, 2023
View reviewed changes
Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

Had a quick look a made some more suggestions.

pkg/globalconfig/globalconfigtypes/globalconfig_types.go Outdated Show resolved Hide resolved
pkg/globalconfig/globalconfigtypes/globalconfig_types.go Outdated Show resolved Hide resolved
pkg/globalconfig/globalconfigtypes/globalconfig_types.go Outdated Show resolved Hide resolved
cmd/ddev/cmd/config-global.go Outdated Show resolved Hide resolved
cmd/ddev/cmd/config-global.go Outdated Show resolved Hide resolved
cmd/ddev/cmd/config-global_test.go Show resolved Hide resolved
cmd/ddev/cmd/config-global_test.go Show resolved Hide resolved
cmd/ddev/cmd/debug-router-nginx-config.go Outdated Show resolved Hide resolved
pkg/ddevapp/amplitude_project.go Outdated Show resolved Hide resolved
pkg/globalconfig/global_config.go Outdated Show resolved Hide resolved
@rfay rfay force-pushed the 20230614_traefik_default branch from ebe22d9 to dd2ae54 Compare June 24, 2023 20:45
@rfay rfay requested a review from gilbertsoft June 24, 2023 20:45
gilbertsoft
gilbertsoft approved these changes Jun 24, 2023
View reviewed changes
Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

Looks good!

@rfay
Copy link
Member Author

rfay commented Jun 24, 2023

Thanks, I'll wait until upstream/master tests look mostly done.

rfay and others added 27 commits June 24, 2023 20:47
@rfay
Re-enable TestCmdGlobalConfig
55d443f
@rfay
Update TestCmdGlobalConfig
2c7f44f
@rfay
Add router= testing in TestCmdGlobalConfig
81cd3f2
@rfay
Update docs in comments etc
3f6346d
@rfay
Improve TestConfigValidate
47aa974
@rfay
First attempt at using constants for traefik things
1038dd3
@rfay
Regenerate ampli.go
b2f4a30
@rfay
Fix amplitude_project after new Router type added
d22782f
@rfay
Change 'traditional' to 'nginx-proxy' type
a10c751
@rfay
In ddev list, change router name from original to nginx-proxy
a1d597e
@rfay
Update container-diagram.png - thanks @mattstein
e5cb3c6
@rfay
Implement @mattstein docs suggestions
770eb2d
@rfay
More naming from traditional -> nginx-proxy
a2aca19
@rfay
Remove some == true for @gilbertsoft
6d1a7a8
@rfay
Fix up TestCmdGlobalConfig
8f4d9f4
@rfay
Introduce IsTraefikRouter()
1bab9d6
@rfay
Use constants for global config New() init
543866a
@rfay
Move traefik constants to typed constants
02312bd
@rfay @mattstein
[docs] Capitalize Traefik [skip ci]
99d0128 
Co-authored-by: Matt Stein <m@ttste.in>
@rfay
Make validRouterTypes private per @gilbertsoft
b13fa94
@rfay
Rename globalconfig_types -> types
57e39ff
@rfay
Use GetValidRouterTypes for error message
3d2ef50
@rfay
Change to use explicit comparison to nginx-proxy in debug command
64fe928
@rfay
Handle router type as optional property for amplitude
c08c2a0
@rfay
Change signature of IsTraefikRouter
6898b2f
@rfay
Change validRouterTypes to slice so predictable order
42aac2a
@rfay
3 places still had direct comparison for router value, change to IsTr…
5fe7f7d 
…aefikRouter()
@rfay rfay force-pushed the 20230614_traefik_default branch from 8678699 to 5fe7f7d Compare June 25, 2023 02:47
@rfay rfay merged commit 8ce4d1e into ddev:master Jun 25, 2023
20 checks passed
@rfay rfay deleted the 20230614_traefik_default branch June 25, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattstein mattstein mattstein requested changes

@gilbertsoft gilbertsoft gilbertsoft approved these changes

@tyler36 tyler36 Awaiting requested review from tyler36

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rfay @mattstein @gilbertsoft