run_on is a top level field, not part of config #38
Conversation
| @@ -3,10 +3,10 @@ local typedefs = require "kong.db.schema.typedefs" | |||
| return { | |||
| name = "zipkin", | |||
| fields = { | |||
| { run_on = typedefs.run_on { default = "all" } }, | |||
There was a problem hiding this comment.
The original code included one_of = { "all" }, to force that only "all" would be valid for this plugin. Was this removed deliberately?
There was a problem hiding this comment.
Yes. There's no reason to disallow someone logging to zipkin from only one side.
There was a problem hiding this comment.
To be honest, I think it's not worth it to write a migration here. The change here modifies a default, but this is very low-impact: if a user tried to use this on a mesh scenario the inappropriate default would probably have been evident from the get-go.
Plus, more generally speaking, writing a migration to modify values given a change of default is not even correct: there's no way to know the default value hasn't been set by the user on purpose.
Of course, in this case if we want the old default to be now invalid (with the one_of annotation) it would make sense to force the change via migration — I guess that's why the one_of annotation was removed?
Given all that, I think it's fine to make the change as-is.
Fix #34
I'm trying to figure out if we need migrations for this and how to write them.