Azure Blob Storage state store strips the prefix

[ItalyPaleAle]

Reported by a user on Discord.

keyPrefix is ineffective with the Azure Blob Storage state store. It appears that the component explicitly removes the prefix from the file key:

components-contrib/state/azure/blobstorage/blobstorage.go

Lines 221 to 228 in c2dbb03

	func getFileName(key string) string {
	pr := strings.Split(key, keyDelimiter)
	if len(pr) != 2 {
	return pr[0]
	}
	return pr[1]
	}

Looking at the "git blame", it appears that this behavior has been present since the very first version of the component: #348

However, this is not something we can fix because it would break backwards-compatibility for all existing users. It may require a v2 of the component.

Note that it appears that || is a valid character in Azure Blob Storage file paths.

[berndverst]

Most folks do not use the special runtime provided prefixes like appID, podID, namespace etc - of course they are very powerful and useful.

I suggest introducing a new metadata property like enableRuntimePrefixSupport or similar (which defaults to false for this component), which basically will additionally allow the use of {appID} and such.

[ItalyPaleAle]

Most folks do not use the special runtime provided prefixes like appID, podID, namespace etc - of course they are very powerful and useful.

The problem is that appID is the default prefix. Right now the component always acts as if "none" is the prefix. The consequence is that if 2 apps write the same key, and users don't set a prefix, with ABS they read the same data, but with all other components they don't.

As for solutions, I agree there are 2 options, and I'm not a fan of either.

1. The v2 of the component, which does add overhead
2. The metadata option, which is more lightweight. However this needs to be "off" by default to preserve backwards-compat, so we'd continue to have a component that misbehaves compared with all other components. It does seem odd to have a metadata option saying "by default there's a bug but we give you the option to not have the bug"

[olitomlinson]

As for solutions, I agree there are 2 options, and I'm not a fan of either.

• The v2 of the component, which does add overhead
• The metadata option, which is more lightweight. However this needs to be "off" by default to preserve backwards-compat, so we'd continue to have a component that misbehaves compared with all other components. It does seem odd to have a metadata option saying "by default there's a bug but we give you the option to not have the bug"```

It really is difficult one. Just to add my opinions :

From an end-user perspective, not introducing breaking changes in behaviour is more important than having a oddly named metadata option. Speaking from experience, breaking changes really hurts dev teams that are trying to adopt, embrace, and build confidence in a particular tech.

That said, given this particular bug is related to the access of data, you could argue that the fix creates a more secure mode of operation with a narrower scope, and so as an end-user, I could see it as reasonable breaking change to make.

Hypothetically, If the fix was creating a less secure / greater scope to the data, this could be disastrous for an end-user scenario i.e. data compromise / leakage.

[ItalyPaleAle]

@olitomlinson thanks for sharing your opinion!

To be clear, we are not talking about making a breaking change in the current component. A Component YAML that defines state.azure.blobstorage which works today, will continue to work tomorrow without any change.

The two options are both backwards-compatible as they require users to opt into the new, fixed behavior:

• A v2 component must be explicitly named in the Component YAML (omitting the version defaults to v1 in Dapr)
• A metadata option would have to be added explicitly

[olitomlinson]

In that case, purely from an end-user perspective, I would bias towards preferring a v2 component.

There would need to be some explicit documentation on the change in behaviour, targeted directly at V2 users who are upgrading from V1.

[iamdrewhayes]

I would recommend taking the v2 component approach. The dapr documentation indicates appid is the default keyprefix behavior.

A version upgrade indicates a breaking change. The breaking change in this case would be fixing the default keyprefix behavior. The documentation for the version upgrade could indicate the default keyprefix behavior will be appid and if already using the component then the keyprefix should be updated to none when upgrading the component version.

[ItalyPaleAle]

I may be leaning towards a "v2" myself. Although it's uncomfortable, it probably is the most correct choice.

@dapr/maintainers-components-contrib what are your thoughts?

[berndverst]

In general, for maintenance reasons, I am against the v2 component for something so little. However, if you can come up with a clean and elegant way that is both easy to understand and minimizes code duplication I am ok with the v2 approach.

I do not want to set a precedent where we introduces v2....v100 for tiny changes. That's a big maintenance burden.

As a maintainer I would personally choose to do the following:

• add a warning message to this component which triggers if someone has the keyPrefix specified
• add a new metadata property that honors the keyprefix if enabled (default is not enabled)
• at some point in the future (say dapr 1.14) change this default, but allowing existing users to retain the existing behavior (by explicitly turning off the honoring of the keyprefix)

Compatibility between v1 and v2 components will not necessarily be guaranteed. We reserve the right to change components significantly between versions. We continue maintaining all component versions unless we announce a deprecation of an existing version in accordance with our deprecation policy.
A v2 component would initially not be considered Stable and not receive support in the release it is being introduced in by the way.

[ItalyPaleAle]

add a warning message to this component which triggers if someone has the keyPrefix specified

Note that the default keyPrefix (if unset) is "appid". So we should show a warning unless the keyPrefix is explicitly set to "none", because the component behaves as if it were set to "none".

This can have important consequences as users could expect the state to be namespaced per app (keyPrefix=appId), while in this case it's not (acts as keyPrefix=none)

[berndverst]

add a warning message to this component which triggers if someone has the keyPrefix specified

Note that the default keyPrefix (if unset) is "appid". So we should show a warning unless the keyPrefix is explicitly set to "none", because the component behaves as if it were set to "none".

This can have important consequences as users could expect the state to be namespaced per app (keyPrefix=appId), while in this case it's not (acts as keyPrefix=none)

Yes we can take a look at those exact details - I am commenting from memory here and forgot some of the info.

I think we can use a metadata property + warning, and eventually switch the default to what it should be all along with a backwards compatibility path.

[berndverst]

Thinking about this more, in this case we can go with a V2 components. Note that there is no upgrade path from V1 to V2. You'd have to manually update the name of all your blobs to prepend the right app ID.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.