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

Dynamic Configuration Refactoring #4168

Merged
merged 1 commit into from Nov 14, 2018
Merged

Dynamic Configuration Refactoring #4168

merged 1 commit into from Nov 14, 2018

Conversation

ldez
Copy link
Member

@ldez ldez commented Nov 7, 2018

!!! Breaking changes !!!

What does this PR do

This PR introduces changes on the dynamic configuration.

  • frontend becomes router
  • backend becomes service
  • new middleware approach (order customization, reusable configuration, chains)
  • rules are now striclty matching the request without tweaking it (no more modifiers)

Tooling

  • introduces a context based logger

Motivation

A more flexible configuration file that gives additional possibilities to the end user
Logs that provide context to the end user

Additional Notes

The only provider that has been migrated is file, as a proof of concept. All other providers are broken.

This Pull Request is the result of the work of many, and more specifically (in alphabetical order) @geraldcroes, @jbdoumenjou, @juliens, and @ldez

@ldez ldez added kind/enhancement a new or improved feature. area/server labels Nov 7, 2018
@ldez ldez added this to the next milestone Nov 7, 2018
@ldez ldez requested review from a team as code owners November 7, 2018 11:44
@ldez ldez removed request for a team November 7, 2018 11:46
@ldez ldez force-pushed the refactor/server-config branch 2 times, most recently from 219427c to 48e2adb Compare November 7, 2018 12:05
@emilevauge
Copy link
Member

2lxjy5

Ullaakut
Ullaakut reviewed Nov 7, 2018
View reviewed changes
Copy link
Contributor

@Ullaakut Ullaakut left a comment

Choose a reason for hiding this comment

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

No time for more review so far, I guess I checked less than half of the PR, but here are my comments so far. Mostly just style/consistency/grammar issues! Excellent work.

api/dashboard.go Show resolved Hide resolved
api/debug.go Show resolved Hide resolved
cmd/convert/convert.go Show resolved Hide resolved
config/middlewares.go Show resolved Hide resolved
config/middlewares.go Show resolved Hide resolved
middlewares/addprefix/add_prefix_test.go Outdated Show resolved Hide resolved
middlewares/auth/auth.go Show resolved Hide resolved
log/log_test.go Outdated Show resolved Hide resolved
middlewares/auth/basic_auth_test.go Outdated Show resolved Hide resolved
middlewares/auth/digest_auth_test.go Outdated Show resolved Hide resolved
@ldez ldez force-pushed the refactor/server-config branch 3 times, most recently from 1e1666b to e5a25bc Compare November 8, 2018 06:25
@geraldcroes geraldcroes mentioned this pull request Nov 8, 2018
Closed
2 tasks
mmatur
mmatur approved these changes Nov 8, 2018
View reviewed changes
Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

After a review in live with @ldez.
I wanted to congratulate you folks for this amazing job!!! 🎉 🎉
This a huge first step
LGTM 👏 👏

juliens
juliens approved these changes Nov 14, 2018
View reviewed changes
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

jbdoumenjou
jbdoumenjou approved these changes Nov 14, 2018
View reviewed changes
Copy link
Member

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM

geraldcroes
geraldcroes approved these changes Nov 14, 2018
View reviewed changes
Copy link
Contributor

@geraldcroes geraldcroes left a comment

Choose a reason for hiding this comment

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

LGTM & LBTM

@ldez @juliens
@geraldcroes @jbdoumenjou
feature: Dynamic Configuration Refactoring
b57d401 
Co-authored-by: SALLEYRON Julien <julien.salleyron@gmail.com>
Co-authored-by: Gérald Croës <gerald@containo.us>
Co-authored-by: Jean-Baptiste Doumenjou <jb.doumenjou@gmail.com>
@traefiker traefiker merged commit a09dfa3 into traefik:master Nov 14, 2018
@ldez ldez deleted the refactor/server-config branch November 14, 2018 09:18
@ldez ldez mentioned this pull request Nov 17, 2018
Closed
2 tasks
@bboreham

This comment has been minimized.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ullaakut Ullaakut Ullaakut left review comments

@juliens juliens juliens approved these changes

@geraldcroes geraldcroes geraldcroes approved these changes

@jbdoumenjou jbdoumenjou jbdoumenjou approved these changes

@mmatur mmatur mmatur approved these changes

Assignees
No one assigned
Labels
area/server kind/enhancement a new or improved feature. size/L
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ldez @emilevauge @bboreham @juliens @geraldcroes @jbdoumenjou @mmatur @Ullaakut @traefiker