Refactor vault handling - #642
Conversation
68bf1d7 to
80bd471
Compare
80bd471 to
cde3db2
Compare
joka134
left a comment
joka134
left a comment
There was a problem hiding this comment.
Some minor questions. looks nice in general
| type PlainFileVault struct{ options FileOptions } | ||
|
|
||
| // NewPlainFileVault returns a vault handler for interacting with a plain (unencrypted) file | ||
| func NewPlainFileVault(opts FileOptions) (*PlainFileVault, error) { |
There was a problem hiding this comment.
| func NewPlainFileVault(opts FileOptions) (*PlainFileVault, error) { | |
| func NewPlainFileVault(opts FileOptions) (*Vault, error) { |
You can return the interface type here
| } | ||
|
|
||
| // NewSOPSVault creates a new vault handler for interacting with SOPS-encrypted vault files | ||
| func NewSOPSVault(opts SOPSOptions) (*SOPSVault, error) { |
There was a problem hiding this comment.
| func NewSOPSVault(opts SOPSOptions) (*SOPSVault, error) { | |
| func NewSOPSVault(opts SOPSOptions) (*Vault, error) { |
There was a problem hiding this comment.
I think in general it's preferable to return the struct type instead of the interface, makes navigation easier. The caller can use the concrete type and you can jump to that instead of the interface directly.
There was a problem hiding this comment.
That is true, but with this there is no actual verification if this is really implementing the interface. Or am I wrong?
| type PlainFileVault struct{ options FileOptions } | ||
|
|
||
| // NewPlainFileVault returns a vault handler for interacting with a plain (unencrypted) file | ||
| func NewPlainFileVault(opts FileOptions) (*PlainFileVault, error) { |
There was a problem hiding this comment.
we could move the implementations of the interface into sub packages to import them like vault.sops.New
| } | ||
|
|
||
| // NewFromString parses vaultType and creates the matching implementation. | ||
| func NewFromString(vaultType string, opts Options) (Vault, error) { |
There was a problem hiding this comment.
Why not fore the usage of the custom type?
2 participants
Refactor vault handling to abstract the encryption handling. This will unify handling of vaults and allow to add new vault types in the future.
The TS installer requires an encrypted SOPS vault, so
oms install codesphereonly supports thesopstype.oms init install-configrespects the vault type so allows to output an encrypted vault directly.