IReadOnlyDictionary<string, IReadonlyDictionary<string, string>> would be the most raw way to expose the bulk data, but I'm not sure if that'd be the best way, as it doesn't easily allow for changes to the API in the future. (E.g. suppose the bulk request returns both values and other metadata for each individual secret). It'd probably be better to expose something like IReadOnlyDictionary<string, GetSecretResponse> Data (which is actually closer to the .proto definition anyway).

I think it's fine to try to simplify some of the .proto APIs where it makes sense (e.g. where it contains unnecessary layers), but we also want to walk that fine line of being true to the .proto as well so that the .NET APIs are still conceptually similar to the other SDKs.

I think we can use the SecretResponse() constructor from the proto defintion.

What's the Dapr semantics for getting a secret key that doesn't exist? Is that an error or should a "does not exist" value be returned? In the case of state stores, we return null in that case.

in case of env secret store we return empty value like {somsecret: ""}. I think it can depend on specific implementation.

Not sure what happened here...

I think the XML editor is getting too aggressive about escaping characters; -> is what it should be.

Remove the nullability (?) from the response argument? We would always expect the implementation to give us one, right?

We'll want to defer committing this PR until (at least) a v1.12.0 tag exists in dapr/dapr, as we don't want builds picking up whatever happens to be in the master branch at the time.

C# allows Add() methods to be inlined:

var resp = new Dictionary<string, string>
{
  { request.SecretName, data }
}

Nit: var response = new SecretStoreGetResponse allows you to eliminate the nullable response created above.

Nit: should be string? data = ...?

I'd expect a build warning about nullability otherwise (as GetEnvironmentVariable() returns string?).

GetEnvironmentVariable returns string when environment variable is not found or set.

GetEnvironmentVariable() returns string? (a potentially null string). An alternative would be:

string data = Environment.GetEnvironmentVariable() ?? String.Empty;

...which assigns "" only if the return value is null.

GetEnvironmentVariables() returns a dictionary of both names and values; there shouldn't be a reason to later call GetEnvironmentVariable() for each variable name.

It might be good to add <remarks> that describe the expected behavior for secrets that don't exist as well as differences between stores that support multiple values per secret and those that don't.

This should only happen once, right, outside the foreach and for the item.Key key?

response should be non-null (by nullability contract). While the implementor could still technically return null at runtime, I'd say that's their problem and not something to explicitly account for.

response should be non-null (by nullability contract). While the implementor could still technically return null at runtime, I'd say that's their problem and not something to explicitly account for.