Update azure storage extra data

[abmussani]

Description:

Blob service of Azure storage returns containers name in response. From that, containers name is added in extra data.

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[rgmz]

This should probably be inside the http.StatusOk block, as I doubt it would return the info for non-success responses.

[abmussani]

make sense. I have moved the parsing and unmarshaling code in StatusOk block.

[rgmz]

I would personally invert the conditions so if err != nil returns the error, rather than discarding it.

[abmussani]

I was in doubt that in case, parsing or unmarshaling get failed, should I return the error and mark verification as fail ? even we get the 200 OK response.

[rgmz]

The decoding failing is notable because it's reasonable to expect that a 200 OK response means the key is valid.

Personally, I might return the error AND set verified = true because it seems highly unlikely that this would occur. However, per @rosecodym's comment you should return the error and mark verification as failed.

[abmussani]

@rgmz I believe @rosecodym shared 2 choices. the second one seems to be like ignoring the additional metadata if error occures in parsing and reading the stream as verification is based on Status.OK. Please correct me if my understanding is wrong @rosecodym.

[rosecodym]

Yes, that's correct. There are clearly some general questions to resolve:

• If we can't decode the response body, is that a fatal error?
• How should we handle non-fatal errors?

We need to decide on general answers to these general questions, but until we do, I think it is acceptable to answer them with "no" and "ignore them" respectively, because that represents a minimal change from the current behavior.

(Another option we haven't discussed yet is to redact the errors and put them themselves into ExtraData.)