Better user experience for BackupStorageLocation.Spec.Config

[mateusoliveira43]

Describe the problem/challenge you have
Today, we do not have yaml validation for BackupStorageLocation.Spec.Config

velero/pkg/apis/velero/v1/backupstoragelocation_types.go

Lines 30 to 32 in cb5ffe2

	// Config is for provider-specific configuration fields.
	// +optional
	Config map[string]string `json:"config,omitempty"`

And this is on purpose, right? Since Velero has supported and unsupported plugins (and yaml validation does not allow a map with specified and non specified fields).

But I think would be awesome to have yaml validation for the supported ones.

Describe the solution you'd like

I think one way to try to not change much code in Velero, would be adding two fields to BackupStorageLocation:

• Spec.SupportedByVelero: Boolean that indicates if the BSL plugin is supported or not by Velero
• Spec.SupportedConfig: The fields for supported BSL plugins

I searched the code, maybe only changing here

velero/pkg/persistence/object_store.go

Lines 145 to 150 in cb5ffe2

	objectStoreConfig := make(map[string]string)
	if location.Spec.Config != nil {
	for key, val := range location.Spec.Config {
	objectStoreConfig[key] = val
	}
	}

to something like this

	objectStoreConfig := make(map[string]string)
	if location.Spec.SupportedByVelero {
		if location.Spec.SupportedConfig != nil {
			for key, val := range location.Spec.SupportedConfig {
				objectStoreConfig[key] = val
			}
		}
	} else {
		if location.Spec.Config != nil {
			for key, val := range location.Spec.Config {
				objectStoreConfig[key] = val
			}
		}
	}

could be enough (but this would also change the way users create BSL today).

And another good enhancement, if changing code is not possible at the moment, would be to better point those options in docs. @shubham-pampattiwar explained to me that they are specified here:

• https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/main/backupstoragelocation.md
• https://github.com/vmware-tanzu/velero-plugin-for-microsoft-azure/blob/main/backupstoragelocation.md
• https://github.com/vmware-tanzu/velero-plugin-for-gcp/blob/main/backupstoragelocation.md

But I think this is not mentioned in Velero docs.

Anything else you would like to add:

Not at the moment.

Environment:

Not applicable.

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

• 👍 for "The project would be better with this feature added"
• 👎 for "This feature will not enhance the project in a meaningful way"

[ywk253100]

@mateusoliveira43 I don't fully understand. Could you clarify what you want to validate and how your solution fixes that?

[mateusoliveira43]

The validation of BSL config fields happens in Velero plugin code (example https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/main/velero-plugin-for-aws/object_store.go#L83).

I think would be better to have this validation in yaml (creating a type for BSL CRD). The main advantage would be the ability to users to see the field options with kubectl describe, for example.

[reasonerjt]

I don't quite understand how this could work.
The BSL is to be created by user, how could we rely on user to specify whether it's supported by velero or not?
Could you give a more concrete example?

[mateusoliveira43]

We should say in the docs, and the doc examples (but maybe Spec.SupportedByVelero boolean would not be necessary).

From today, sported BSL plugins are aws, gcp and azure, right? These ones should have the fields specified in Spec.SupportedConfig (the non supported plugins would not change their use case, and also supported but non BSL plugins).

So, for example: All fields from here https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/main/backupstoragelocation.md should be in Spec.SupportedConfig (and also gcp and azure fields) and user would use that field instead of Spec.Config. This way, if user forgets a field name (what would prevent typo being accepted in object creation) or wants to see all options for the cloud provider, kubectl command can be used for that purpose.

[sseago]

@mateusoliveira43 one problem with this approach is that it would require any changes to BSL config in a plugin to be coordinated with a core velero PR, and the whole purpose of moving these plugins out of velero main repo (where they were originally located) was to decouple this sort of thing.

[mateusoliveira43]

Yes, that would be a problem 😬

[mateusoliveira43]

Talked to @kaovilai @sseago and others, this will be faced as doc issue. Just adding entry in docs to where users can find all options for each specific BSL plugin config.

[reasonerjt]

@mateusoliveira43
Thanks， so I'll make sure the "backuplocation.md“ file of AWS/Azure/GCP plugin is referenced in velero's doc.

[yanggangtony]

1

2

3

4

Follow above steps, can found the config's parameter.

---

Is that still need to told user the details url in plugin repo like

AWS:
https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/main/backupstoragelocation.md
GCP:
https://github.com/vmware-tanzu/velero-plugin-for-gcp/blob/main/backupstoragelocation.md
AZURE:
https://github.com/vmware-tanzu/velero-plugin-for-microsoft-azure/blob/main/backupstoragelocation.md

[yanggangtony]

Now , there are only GCP has the referenced url.

I can add ohters, like gcp's referenced if that is need 😇😇

[mateusoliveira43]

Thanks @reasonerjt
So, as @yanggangtony pointed (thanks!!!), its already there, but did not see before

On the table you added in the steps, its already in the last column (parameters). My suggestion for this would be:

• remove link from GCP object store and volume snapshotter (just because the other column already have the same links)
• change "parameters" column to "config options"
• in the examples in the docs, tell user that the full list of options for each BSL plugin can be found in that table (for example, in here https://velero.io/docs/v1.12/contributions/minio/#set-up-server)

[yanggangtony]

@reasonerjt @mateusoliveira43
Sound goods.
I will update as that post .

1 remove link from GCP object store and volume snapshotter (just because the other column already have the same links)

2 change "parameters" column to "config options"

3 supplement the docs in https://velero.io/docs/v1.12/contributions/minio/#set-up-server , tell user that the full list of options for each BSL plugin .