run_on property should not be in config

[hbagdi]

The following commit adds a run_on property inside the config which should really be at the fields level (one level up).

7d6da5f#diff-2ba4b18ab47bc44de697a2d3649d3971R9

This is also an issue with Azure functions plugin (Kong/kong-plugin-azure-functions@6675eed)

cc @kikito @Tieske @james-callahan

[Tieske]

Iirc this same issue was caught in the Kong code base before merging, but apparently was missed in the external plugins code

[james-callahan]

How do we write a migration for this safely?

[hishamhm]

How do we write a migration for this safely?

The existing config.run_on is, as it stands, ignored and can be safely deleted. The correct toplevel run_on entry is being picked with the default value (first). A migration could scan all zipkin plugins and replace their run_on with all.

I assume the latter can be done with something like this (untested)

  postgres = {
    up = [[
    ]],

    teardown = function(connector, helpers)
      assert(connector:connect_migrations())

      for rows, err in connector:iterate([[SELECT * FROM "plugins" WHERE name = 'zipkin';]]) do
        if err then
          return nil, err
        end

        for i = 1, #rows do
          local row = rows[i]
          local sql = string.format([[
            UPDATE "plugins" SET "run_on" = 'all' WHERE "id" = '%s';
          ]], row.id)
          assert(connector:query(sql))
        end
      end
    end,
  },

  cassandra = {
    up = [[
    ]],

    teardown = function(connector, helpers)
      local cassandra = require "cassandra"
      local coordinator = assert(connector:connect_migrations())
      for rows, err in coordinator:iterate([[SELECT * FROM plugins]]) do
        if err then
          return nil, err
        end

        for i = 1, #rows do
          local row = rows[i]
          if row.name == "zipkin" then
            assert(connector:query("UPDATE plugins SET run_on = ? WHERE id = ?", {
              "all",
              cassandra.uuid(row.id),
            }))
        end
      end
    end,
  },
}

I adapted this from a migration of the acl plugin. I'm not sure if one actually needs to remove the run_on data from the config row or if it would just disappear when removed fro the schema. If it needs to be explicitly deleted, then the config field of the plugins rows can be adjusted accordingly in the loops above (JSON-decode, remove field, JSON-encode and update).