-
Notifications
You must be signed in to change notification settings - Fork 393
Description
These are very minor but:
The configuration
key feels arbitrary and is really leaking deployment vs other objects rather than representing a conceptual difference for a user. This is all configuration. Also non-idiomatic. I'd propose either removing configuration
and moving the subkeys to the root level or renaming it velero
. The latter is a bit confusing when using this chart as a requirement/subchart as it leads to velero.velero.property
so I'd recommend the former.
deployRestic
should be restic.enabled
. In addition to being a standard pattern this helps signal that the configuration under the restic
key is only relevant if enabled.
useSecret
could be removed, if either existingSecret
or secretContents
are set we can infer that we should run with the secret if neither is set we can assume we should behave as if useSecret
was false
.
Consider combining existingSecret
and the newly added existingSecretKey
to a single object with name and key props. This is less common specifically for these props but aligns with common patterns in other objects, I'd probably not recommend this but it's worth raising.
This combined with the existing PR would make a simple deployment on GCP with Restic and a schedule defined:
provider: gcp
backupStorageLocation:
bucket: <a_gcp_bucket>
credentials:
existingSecret: gcp-storage-credentials
restic:
enabled: true
schedules:
- name: daily
schedule: "@every 24h"