Closed mrajashree closed 3 years ago
The EncryptionConfigName
field, is the name of a kubernetes Secret, that contains the encryptionConfig file. This same encryptionFile is needed while restoring from a backup. It is up to the user to save their encryptionConfig file contents properly, as it will not be stored by the backup-restore-operator. So if user looses this encryptionConfig file, restore will fail. We should add a warning above this field in the UI indicating that user is responsible for saving contents of the encryptionConfig file, and must use the same file when restoring from this backup.
NOTE: The secret that contains this encryption config file can be named something else, the contents must stay same
Can we remove the "Description" Box, and "Labels and Annotations" section for this CRD too? The fields we want to display for sure are:
In the "Schedule" field, either in it or below it, can we show examples of what is allowed? So can the schedule field have '@midnight'
and '* * * * ?'
as examples?
S3 details are not a required field. The way this operator works is, user can configure a global backup location while launching/upgrading the operator (helm chart). OR User can specify a particular s3 bucket to restore from/backup to. So, after the first set of fields, this is what I think we should have
Select Storage Location (Radio buttons)
Option 1: Use the Backup location configured at the operator level. (Note: Right now there is no way of displaying the exact value in UI)
Option 2: Specify details of the s3 compatible storage provider where the backup file should get stored (s3/minio) If this option is selected, only then we show the fields to configure s3.
On master-head 6a6f13cec
The "Schedule" field should not have the increment/decrement arrows, because if I click on it it just sets schedule to an integer, thus setting it to an invalid cron value which the operator can't parse
The "Schedule" field is not able to accept the cron values, I cannot type in any special characters such as *
or /
or space, or in the case of Descriptor cron values such as @every 1h
I cannot type in @
. I think this and the first issue is because the type of input for schedule should be a string and maybe right now its an integer?
"Encrypt backups using an encryption config secret" does not show a dropdown for secrets. I also see this error in the console
If you select the first option under "Storage Location", to "Use the the default storage location configured during installation", UI still unsets the "storageLocation" and "s3" fields in yaml as seen in the pic below:
This is incorrect, it should only set the "storageLocation" and/or "s3" field if the second option is selected. Since the two fields are set with no other details for s3, operator throws an error for incorrect s3 configuration
These are some changes I'd like to request, that aren't causing bugs but would be good to have/are typos:
Create Page
Detail Page
Since schedule is optional, the schedule concept should convert to an option for one time backup and scheduled backups. Is a one time backup an empty schedule
yes, if the schedule field is not set at all then its a one-time backup
What is supposed to happen if I select encrypt backups? Does the encryption configuration need to already exist? Maybe we should add (Recommended) to the encrypt option.
yes the secret containing encryption config should already exist, and a dropdown to select such secrets should be presented
on master-head sept 21
encryptionConfigSecretName
and not encryptionConfigName
. Right now enabling encryption config sets encryptionConfigName
but it should set encryptionConfigSecretName
Encryption Config Secret
does not open the k8s docs page that the link
Rest all fields and changes look good
Encrypting backups works now. But while configuring s3, the field "Endpoint CA" "Read from File" does not get the contents of the file. I can directly copy-paste and that works @mantis-toboggan-md
I was able to perform backups using the form. And then after sometime whenever I go to Create Backup page I see this, although the default resource-set does exist. And when I go to create backups page, the api call to get resource-sets also returns the existing resource-set
On master-head- commit id: 2229f5cd
Issues seen:
Issue#1
We should add a warning above this field in the UI indicating that user is responsible for saving contents of the encryptionConfig file, and must use the same file when restoring from this backup.
Currently this is NOT available
Logged issue -- https://github.com/rancher/dashboard/issues/1393Issue#2
Encryption Config Secret
to be listed in the dropdown, the secret has to be created with key “encryption-provider-config.yaml”. This should be made obvious on the UI. Can we add a note in the UI ?
Logged issue - https://github.com/rancher/dashboard/issues/1395
Issue#3
Issue#4
Issue#5
issue#6
Encryption Config Secret
option - enabled but do NOT give in a filenameEncryption
on the backupExpected: Backup creation should throw an error saying the file/input value is missing.
Logged issue -- https://github.com/rancher/dashboard/issues/1398
Closing this in favor of the individual issues logged
In order to perform backup, user needs to create an instance/CR of the Backup CRD. When a backup CR is created, the backup controller will start processing it and create a backup by gathering all resources specified in the backup CR's ResourceSet.
The
EncryptionConfigName
field on Backup spec, is the name of a kubernetes Secret, that contains the encryptionConfig file. This same encryptionFile is needed while restoring from a backup. It is up to the user to save their encryptionConfig file contents properly, as it will not be stored by the backup-restore-operator. So if user looses this encryptionConfig file, restore will fail. We should add a warning above this field in the UI indicating that user is responsible for saving contents of the encryptionConfig file, and must use the same file when restoring from this backup. NOTE: The secret that contains this encryption config file can be named something else, the contents must stay same