Make SQLite KIM default #570
Conversation
Making the SQLite KIM default for testing and configuration examples. The change to testing involves replacing the on-disk KIM for configuration tests, enabling cross tests and multitenancy tests for the SQLite KIM, and making SQLite tests the default for `all-providers`. Changes were needed for cross-provider tests now that key names cannot be reused across providers, to generate unique key names at the test level. Signed-off-by: Ionut Mihalcea <[email protected]>
|
|
||
| [[provider]] | ||
| provider_type = "MbedCrypto" | ||
| key_info_manager = "on-disk-manager" | ||
| key_info_manager = "sqlite-manager" |
There was a problem hiding this comment.
Not related to any of your changes specifically but food for thought around SQLite config files.
[[provider]]
name="mbed-crypto-provider"
I know we originally made the provider name globally optional so that existing configs would not fail with new versions of Parsec. Would it be better for it to be optional for OnDisk KIM implementations (to ensure config stability), and required for SQLite KIM configs? This way it forces the user to edit the config and set the provider names with their "preferred" naming scheme; preventing the (probably common) case of a user changing from the OnDisk to SQLite KIM without reading any of the provider naming warnings (quoted below).
In my mind, by going from OnDisk -> SQLite the user is "opting-in" to a new feature-set and its requirements. The only concern from the stability requirements set out is New options should be optional. I would argue that, transitively, provider name in this case is optional as the manager_type="OnDisk" would have to be mutated to manager_type="SQLite" in order for provider name to become a required field.
Parsec Config Stability:
Old configuration files should still work with future stable Parsec versions, with the same default for optional options.
Configuration options should not disappear in future stable Parsec versions. Configuration defaults should remain the same. New options should be optional.
Current provider name warnings:
⚠ WARNING: Provider name cannot change.
⚠ WARNING: Choose a suitable naming scheme for your providers now.
⚠ WARNING: Provider name defaults to "mbed-crypto-provider" if not provided, you will not be able to change the provider's name from this if you decide to use the default.
⚠ WARNING: Changing provider name after use will lead to loss of existing keys.
On the note of these warnings. WARNING: Provider name cannot change. would probably read nicer as WARNING: Provider name should not change once set.. And maybe loss of existing keys to LOSS OF EXISTING KEYS to really drive the point home and draw the attention of skim readers.
There was a problem hiding this comment.
Matt's point here makes me think that maybe our rules around config file stability need some refinement, because I would agree that there are cases where explicitly opting in to a new feature (and hence editing the config file anyway) might prompt the need for other required changes. The current rule of "new options should be optional" is perhaps rather too vague. The real intention here is simply to ensure that existing config files always continue working, but that's not the same as saying that it should be possible to edit one thing without changing anything else.
But then there is the question about what should be done in this specific case - should a switch to the SQL lite provider prompt the addition of names for existing providers? I'm in two minds about this. It might be enough simply to ensure that our out-of-box example config includes provider names. At the moment, a working deployment cannot be "switched" from on-disk to sql anyway, because there is no migration facility being offered as yet. If and when we decide to offer a migration path, we could possibly make it a requirement of that process to inject provider names into the config.
There was a problem hiding this comment.
I have moved this conversation over to #571 !
Removing the space between '#' and 'name' (for providers) to make it aligned with how the other options are presented. Signed-off-by: Ionut Mihalcea <[email protected]>
|
I noticed that, while we did have |
Making the SQLite KIM default for testing and configuration examples.
The change to testing involves replacing the on-disk KIM for
configuration tests, enabling cross tests and multitenancy tests for the
SQLite KIM, and making SQLite tests the default for
all-providers.Changes were needed for cross-provider tests now that key names cannot
be reused across providers, to generate unique key names at the test
level.
Fixes #531