ssm-diff icon indicating copy to clipboard operation
ssm-diff copied to clipboard

Plugin system for `Diff*` engine

Open ambsw-technology opened this issue 5 years ago • 0 comments

This PR builds on refactor, adding a plugin system for Diff* engines. To make the plugin system work, I needed to simplify the interface between ParameterStore (formerly RemoteState) and DiffResolver (formerly FlatDictDiffer). My proposed interface is:

  • DiffBase.configure(args) is a standard way for plugins to pre-configure themselves using CLI args. This method would normally use functools.partial to pre-populate constructor kwargs.
    • (backwards incompatible) I also tweaked the CLI arg for --force (to --diffresolver-force) since it's specific to the DiffResolver plugin.
  • DiffBase.__init__(remote, local, **kwargs) initializes the plugin with the local and remote data. The constructor kwargs can be populated directly by programmatic users.
  • <instance>.plan (an @property) returns a dict of operations that will be applied to Parameter Store:
    {
        'add': {'key': '<value>', ...},
        'change': {
            'key': {'old': '<value>', 'new': '<value>'},
            ...
        },
        'delete': {'key': '<value>', ...}
    }
    
  • DiffBase.describe_diff(plan) will provide a print-friendly display of the elements in plan. Since the possible operations in plan are finite and well-described (i.e. what you can do on the AWS API), the default implementation should work for 99% of implementations.

By consolidating all API operations in a plan dict, the Diff* plugin can support an arbitrary amount of complexity. For example, a concurrency-sensitive plugin (like the one proposed in #16) could use additional methods to change the behavior of plans:

> diff = ImaginaryDiff(remote, local)
> diff.plan
Exception:  Conflicts Detected:
`/a/b/c/` has changed on both the remote and local storage
> diff.resolve('/a/b/c', use=ImaginaryDiff.REMOTE)
> diff.plan
{'add': {}, 'change': {}, 'delete': {}}
> diff.resolve('/a/b/c', use=ImaginaryDiff.LOCAL)
> diff.plan
{'add': {}, 'change': {'/a/b/c': {'old': '<remote>', 'new': '<local>'}}, 'delete': {}}

ImaginaryDiff would need local_changes() and remote_changes() instead of simply changed(). The result of these methods would be unchanged by resolve() but the plan would be updated accordingly.

I'm not married to the API if you can think of a situation that isn't addressed by it. I needed to get something working and this has been adequate so-far.

ambsw-technology avatar Apr 30 '19 19:04 ambsw-technology