loosen definition of object specs in SecretsHelper #6654
Conversation
| "title": "SSH Tunnel Method", | ||
| "description": "Whether to initiate an SSH tunnel before connecting to the database, and if so, which kind of authentication to use.", | ||
| "oneOf": [ | ||
| { |
There was a problem hiding this comment.
Here is where the problem was. It was expecting a "type": "object" here, which is missing. However, it isn't technically required by JSONSchema, so my implementation was wrong.
jrhizor
changed the title
There was a problem hiding this comment.
is the preferred state that specs follow json schema more rigidly and always specify types? or is this a permanent loosening?
i think ideally we would lint specs and make them actual conforment jsonschema. so i'd vote for it being temporary. if we are going with temporary let's create a follow up issue.
|
I don't think this can be temporary. Technically not specifying |
| final ConfigPersistence writeToPersistence = new FileSystemConfigPersistence(TEST_ROOT); | ||
| final SecretsMigration migration = new SecretsMigration(readFromPersistence, writeToPersistence, false); | ||
|
|
||
| final Database database = new ConfigsDatabaseInstance( |
There was a problem hiding this comment.
I'll update this to a production version in a separate PR
* address missing secretshelper case * fix test * migration changes * fix build * format * switch secrets migration to platform-only build
Postgres SSH keys aren't being handled properly by
SecretsHelpertest cases because it didn't specify that its type was an object in the spec. This adds a test case using the actual Postgres spec and fixes the behavior ofSecretsHelpers(and also preemptively fixes a similar problem forarray).Before merging, we need to figure out how to migrate between the two again.