ObjectStorage Prefix specified in BackupStorageLocation is ignored

[jacksgt]

Hello,

Velero's BackupStorageLocation (.spec.objectStorage.prefix) as well as OADP's DataProtectionApplication (.spec.backupLocations[].velero.objectStorage.prefix) allow setting a (optional) prefix for files uploaded into the S3 bucket:

Velero assumes it has control over the location you provide so you should use a dedicated bucket or prefix. If you provide a prefix, then the rest of the bucket is safe to use for multiple purposes.

apiVersion: velero.io/v1
kind: BackupStorageLocation
metadata:
  name: default
spec:
  provider: aws
  objectStorage:
    bucket: myBucket
    prefix: myPrefix

https://velero.io/docs/v1.13/api-types/backupstoragelocation/

Our BackupStorageLocation is configured with such a prefix, but the plugin currently ignores this. This leads to conflicts in the S3 bucket since ImageStream data is always uploaded to docker/registry/v2/... (when using the PluginRegistry). This appears to be the problematic part of the code: https://github.com/openshift/openshift-velero-plugin/blob/95ab2b038f0170ab6b0fa70bcb2d28268b312739/velero-plugins/imagestream/registry.go#L133

An additional environment variable REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY=${BSL.spec.objectStorage.prefix} should be added to address this (see example in the (registry configuration documentation](https://distribution.github.io/distribution/about/configuration/))

[sseago]

Supporting the registry here was one of the reasons we added the prefix in the first place -- the registry can't be in the same prefix as the velero backup, or velero will complain about the docker dir. To avoid the issue you're hitting, we'd probably need to add a separate prefix for the registry location outside of the BSL config.

[jacksgt]

Supporting the registry here was one of the reasons we added the prefix in the first place

You mean adding the prefix to BackupStorageLocation.spec.objectStorage.prefix, correct?

the registry can't be in the same prefix as the velero backup, or velero will complain about the docker dir.

Ah, that's interesting. So if prefix is velero, then velero would complain if /velero/docker/registry/... appears in the S3 bucket?

UPDATE: Indeed, I found the relevant code snippet here: https://github.com/vmware-tanzu/velero/blob/6c2b66b480f1f58e08f62c8a0e8ae820a9cd02c6/pkg/persistence/object_store.go#L203

But plugins are allowed to (supposed to?) write arbitrary data to the plugins directory, so maybe this data should actually go to <PREFIX>/plugins/openshift-velero-plugin/docker/registry/...:

• https://github.com/vmware-tanzu/velero/issues/2344
• https://github.com/vmware-tanzu/velero/pull/2350/files
• https://github.com/vmware-tanzu/velero/blob/6c2b66b480f1f58e08f62c8a0e8ae820a9cd02c6/pkg/persistence/object_store_layout.go#L42

[kaovilai]

so maybe this data should actually go to <PREFIX>/plugins/openshift-velero-plugin/docker/registry/...:

Sure. But it would be a breaking change. or require some migration effort.

[sseago]

Supporting the registry here was one of the reasons we added the prefix in the first place

You mean adding the prefix to BackupStorageLocation.spec.objectStorage.prefix, correct?

Yes, exactly.

the registry can't be in the same prefix as the velero backup, or velero will complain about the docker dir.

Ah, that's interesting. So if prefix is velero, then velero would complain if /velero/docker/registry/... appears in the S3 bucket?

Yes -- this is actually why I implemented the prefix feature upstream in the BSL in the first place.

UPDATE: Indeed, I found the relevant code snippet here: https://github.com/vmware-tanzu/velero/blob/6c2b66b480f1f58e08f62c8a0e8ae820a9cd02c6/pkg/persistence/object_store.go#L203

But plugins are allowed to (supposed to?) write arbitrary data to the plugins directory, so maybe this data should actually go to <PREFIX>/plugins/openshift-velero-plugin/docker/registry/...:

• allow plugins to write arbitrary data to 'plugins/' directory inside backup storage location vmware-tanzu/velero#2344
• https://github.com/vmware-tanzu/velero/pull/2350/files
• https://github.com/vmware-tanzu/velero/blob/6c2b66b480f1f58e08f62c8a0e8ae820a9cd02c6/pkg/persistence/object_store_layout.go#L42

[sseago]

so maybe this data should actually go to /plugins/openshift-velero-plugin/docker/registry/...:

Sure. But it would be a breaking change. or require some migration effort.

And actually, any change here is potentially a breaking change, even my earlier suggestion for a registry-specific prefix. One possibility to fix would be to have the registry creation code configure the new location under plugins but if an existing registry exists at the top level, velero could copy the full set of registry files from top level to plugins if there are no files yet under plugins.

[jacksgt]

so maybe this data should actually go to /plugins/openshift-velero-plugin/docker/registry/...:

Sure. But it would be a breaking change. or require some migration effort.

And actually, any change here is potentially a breaking change, even my earlier suggestion for a registry-specific prefix. One possibility to fix would be to have the registry creation code configure the new location under plugins but if an existing registry exists at the top level, velero could copy the full set of registry files from top level to plugins if there are no files yet under plugins.

Very true. I think this requirement / limitation should at least be very clearly documented.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[kaovilai]

/lifecycle frozen