Database reconciler extracted to a separate library

[mflendrich]

Problem Statement

deck started out as a "database reconciler" and a definition for the declarative config format (aka "kong yaml" or "the deck format") that is now the API standard for Kong configuration. The scope of the deck tool has greatly expanded since then, however the "database reconciler" core is used as a library in certain contexts, particularly by KIC. This drift (between the broad scope of the deck CLI and the "database reconciler" library) has led to structural problems like the dependency cycle in #1050 (comment).

Let's fix this structural problem - by making the incremental step of decomposing deck into the "top level CLI that relies on functionality implemented in libraries" and "the database reconciler library"

Proposed Solution

Create a new library (@rainest coined the term "database reconciler" for this purpose) called go-database-reconciler and move the deck diff/sync/dump/reset implementation thereto.

The exact interface of the "database reconciler" is up to the implementer. The existing implementation suffers from tight coupling between the reconciliation logic and the filesystem R/W operations - whether this decoupling happens as part of this task or not is up to the person picking this up, whatever leads to better ROI in their view.

Additional Information

The go-database-reconciler name is open to change by the implementer.

Acceptance Criteria

• a new go-database-reconciler repo/library exists and has a public programmatic interface exposing at least the dump, sync and diff operations
• deck's cobra commands dump, sync diff and reset switched to using go-database-reconciler
• KIC no longer has a go.mod dependency on github.com/Kong/deck - it instead depends on go-kong and go-database-reconciler

[mflendrich]

Updated the AC to also include the reset command per @GGabriele's suggestion.

[rainest]

KIC doesn't use commands per se, as it's supplanting the deck UI, and all of what deck does really boils down to variants on its dump and sync commands (diff is sync without actually issuing the change reqests; reset is sync with an empty target state).

KIC also needs the file package for stuffing things into a deck manifest (and rendering it into JSON to send direct to Kong, in DB-less mode) and state and utils packages for some types. I've prepared a long and poorly-edited overview of the de facto interfaces.

We could maybe refactor to remove those type dependencies in KIC, but it probably wouldn't win us much in the short term. Those same packages are critical for the dump and sync implementations, so if we move dump and sync we're moving the rest anyway lest we end up with a silly circular dependency train that still leaves us with an indirect dependency.

https://github.com/rainest/decklib is an example library repo, and rainest/kubernetes-ingress-controller#218 and rainest#123 show what this looks like in the projects that would depend on it.

@GGabriele does that split look reasonable to you? IDK if there's anything ya'll would prefer to keep in the main deck repo or anything else you think should go in the lib.

• convert and validate I consider part of the UI. Nothing KIC needs depends on them, so no reason to move them.
• konnect is not used by KIC, but currently dump requires it. Not sure if there's a good way to refactor that, and I didn't really have much case for leaving or moving it beyond that KIC doesn't use it and has its own Konnect interface code.
• cprint we can move back if we go ahead and refactor Syncers to accept a logger or something else. It doesn't have much reason to go in the library, but diff does require it without a refactor.

Note: KIC still has a dependency on deck through KTF, as KTF has its own built-in miniature version of what KIC has to dump Kong config in diagnostics, AFAICT. While removing all tests does clear the deck dependency, I'm not sure how we actually want to go about that in KTF. We could refactor it to use the same library this would create.

[GGabriele]

@rainest thanks for putting together this plan and providing these options!

I think the option you are going with (keeping in decK only the CLI aspect of it) makes sense to me.

• convert and validate can definitely remain here
• konnect needs a separate refactor, with most of its code being deprecated and good to be removed. In reality, the client part could potentially live in go-kong, but that's for another time
• if you envision removing the cprint implementation from decklib later on, another option may be to both keep it in decK and temporarily copy it in the decklib as well, where it will be ready to be removed at any time without impacting decK.

In an ideal world we would have extracted (from file) and centralized only the types that make sense to share, while keeping in decK anything that's CLI-operations related. But this is not the ideal world. :)

I'd request that @Tieske also take a look at this, since his work with APIOps may be potentially impacted too.

cc: @aboudreault @mikefero

[rainest]

Kong/go-database-reconciler#8 and Kong/go-database-reconciler#9 as tentative first steps in the actual new repo. https://github.com/Kong/go-database-reconciler/milestone/1 to track overall progress.

Using multiple cprint copies is probably inadvisable because its verbosity is configured at the package level rather than through a struct instance, so I think we probably need to move that and then move it back after refactoring the diff bits that use it.

[rainest]

#1109 makes the changes in deck against the Kong/go-database-reconciler#9 draft. Assuming the GDR pull is merged as-is, the deck#1109 draft should just need to be updated with an actual version tag.

Kong/kubernetes-ingress-controller#5156 makes the changes in KIC; ditto the version stuff.

Kong/go-database-reconciler#10 is a basic test setup in the new repo that just runs the same tests as before by pulling in the deck CLI. Dunno if I can think of something more elegant--to actually do anything with the library (and thus test it) you need some equivalent of the deck CLI to call the library functions and provide them with input. We could make a non-CLI version that does so directly without cobra or whatever, but it wouldn't have much purpose other than avoiding the dependency ouroboros. That may be a valid reason in itself, but reusing the existing tests as a stopgap is simpler and achieves the same functional goal.

Not sure if CI is caching results or what--it appears the tests generally worked except codecov, which fails due to a missing token. Not too big a concern.