Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle invalid Azure object metadata keys #7258

Merged

Conversation

Neon-White
Copy link
Contributor

@Neon-White Neon-White commented Apr 3, 2023

Explain the changes

  1. The new code should adhere to azcopy's method of resolving invalid object metadata keys in Azure

The key of the newly added header is azure-metadata-handling, and its possible values are:

  • RenameIfInvalid - which will rename the key in order to preserve it (which is the default behavior if the header is not supplied)
  • ExcludeIfInvalid - which will exclude the invalid tag
  • FailIfInvalid - which will fail the upload

We support the same naming and recovery methodologies as Microsoft, meaning we won't be able to discern between tags whose renamed form cannot be restored (e.g. - !@# and #$% on the same object)

Issues: Fixed #xxx / Gap #xxx

  1. Fixes object IO when the object contains metadata keys that are invalid by Azure's standard

Testing Instructions:

  1. Upload an object with metadata keys containing non-alphanumeric characters (e.g. !@#-) to a bucket backed by an Azure namespacestore
  2. Download the object from the bucket
  3. Verify that the tags are the same as they were originally
    Bonus step:
  4. After 1, download the object directly from Azure, and verify that it now contains two new metadata keys related to the invalid one, starting with rename_ and rename_key_

@Neon-White Neon-White added the Compatibility-Azure Blob Compatibility and Namespace over Azure label Apr 3, 2023
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 4, 2023
@dannyzaken dannyzaken requested review from a team, tamireran and alphaprinz and removed request for a team and tamireran April 4, 2023 12:17
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
src/sdk/namespace_blob.js Outdated Show resolved Hide resolved
@@ -49,7 +49,8 @@ async function put_object(req, res) {
tagging,
tagging_copy: s3_utils.is_copy_tagging_directive(req),
encryption,
lock_settings
lock_settings,
azure_invalid_tag_header: req.headers['azure-metadata-handling'] || undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a situation where meta data is passed but this header isn't?

Copy link
Contributor Author

@Neon-White Neon-White Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this is the default situation where a user simply uploads objects with md - it's a header I made up that should be supplied by the user if they want to choose a specific behavior

src/util/http_utils.js Outdated Show resolved Hide resolved
Copy link
Contributor

@dannyzaken dannyzaken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

squash your commits

@Neon-White Neon-White force-pushed the handle-invalid-azure-metadata-keys branch from 27d389c to 31d3ca0 Compare April 17, 2023 15:28
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 17, 2023
Signed-off-by: Ben <belimele@redhat.com>
@Neon-White Neon-White force-pushed the handle-invalid-azure-metadata-keys branch from 31d3ca0 to 5a6f245 Compare April 17, 2023 15:29
@pull-request-size pull-request-size bot added size/M and removed size/L labels Apr 17, 2023
@dannyzaken dannyzaken merged commit f4c47f1 into noobaa:master Apr 17, 2023
5 checks passed
@dannyzaken dannyzaken mentioned this pull request Apr 17, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compatibility-Azure Blob Compatibility and Namespace over Azure size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants