cloudflare / certmgr

Automated certificate management using a CFSSL CA.
BSD 2-Clause "Simplified" License
218 stars 40 forks source link

Convert to go mods, Separate certmgrd internals from certmgr lib code, build a usable API. #94

Closed ferringb closed 4 years ago

ferringb commented 4 years ago

TL;DR: this splits the codebase areas of concern into a certmgr/* tool pathing, and a library pathing (cert/*), and then moves everything back into the actual spot it belonged in. In the process, we wind up with a simplified 'cert.Spec' object that isn't creatable only via parsing files from disk, and also make the individual source actually readable now.

For review, I'd suggest just pulling the files in cert/* rather than looking at diff's- it's easier to read. Note this is a usable first 'split', but I expect there to be further iterations as the code becomes more controlled/managed.

Relevant commit message meanwhile:

The goal here is to break out a usable API for codebases not using
certmgr spec files, while also decoupling the internals from the
certmgrd implementation.  The code was far to intertwined and overloading
of concerns- and in turn, complicating the heck out of cert.Spec.

While I'm restructuring to support this, the parsing related code,
the CLI tool, etc, all are moved into a new certmgr subpackage and
were realigned to normal standards (cobra layout for cmd's for example).

The supportive code for cert/*- primarily file/* and util/* were moved
into the areas of the codebase that makes sense, now that things are
separated out.

Renaming and cleanup is needed within the certmgr/*, but going forward
the parsing code should never be w/in the same package space as Spec
to ensure that separtion of concerns is maintained (in the process,
ensuring that certmgr is usabled as a general lib).  Note that there
are no semver API guarantees that will be made for the certmgr utility.

Use the library code instead; semver will be maintained for that.

On disk spec parsing has no compatibility breakage, but I have
moved the CA file definition out of 'authority'.

Backwards compatibility is retained, it'll just warn if you're using
the old pathing.

Backwards compatibility wasn't retained for the following:
* CHANGE_TYPE is no longer exposed for command notifications; things shouldn't
  care if it's the CA or cert that changed, it should just reimport both.
* metrics that were action orientated were dropped; they weren't particularly
  useful and they directly conflict w/ the storage interface that has been
  introduce to allow using cert.spec as a proper API.
ferringb commented 4 years ago

I'll push an update in a bit addressing the test complaints (not sure why I didn't trigger that locally, but oh well, that's what travis is for- a backstop at worst).