Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Fix exception:

ClassCastException: class org.apache.hadoop.crypto.CryptoOutputStream cannot be cast to class org.apache.hadoop.ozone.client.io.KeyMetadataAware ...
  at org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint.createMultipartKey(ObjectEndpoint.java:1003)
  at org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint.put(ObjectEndpoint.java:239)

Steps to reproduce:

  1. Start cluster with Ratis streaming disabled (default setting)
  2. Create encrypted bucket in S3 volume
  3. Upload key via multipart upload to encrypted bucket

https://issues.apache.org/jira/browse/HDDS-10784

How was this patch tested?

Added acceptance test coverage for S3 with Ratis streaming disabled in encrypted bucket in ozonesecure environment. ozonesecure-ha covers the same with Ratis streaming enabled.

CI:
https://github.com/adoroszlai/ozone/actions/runs/8920571448

@adoroszlai adoroszlai added the s3 S3 Gateway label May 2, 2024
@adoroszlai adoroszlai self-assigned this May 2, 2024
@adoroszlai adoroszlai requested review from hemantk-12 and ivandika3 May 2, 2024 09:28
Copy link
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix. LGTM +1.

I also saw some other explicit casting instances in ObjectEndpointStreaming:

  • ObjectEndpointStreaming#putKeyWithStream
  • ObjectEndpointStreaming#copyKeyWithStream
  • ObjectEndpointStreaming#createMultipartKey

The wrapping for createStreamKey and createMultipartStreamKey:

  • OzoneDataStreamOutput
    • KeyDataStreamOutput (non-secure case)
    • OzoneOutputStream (secure case)
      • CryptoOutputStream (FileEncryptionInfo specified)
        • KeyDataStreamOutput
      • OzoneOutputStream (FileEncryptionInfo not specified, but GDPR enabled)
        • CipherOutputStreamOzone
          • KeyDataStreamOutput

I think should be fine to leave it be since these cases did not cast CryptoOutputStream to KeyMetadataAware directly. However, I think we can remove the unnecessary KeyMetadataAware casting from these methods, but I'm also fine to leave it as it is.

Another possible solution might be to extend CryptoOutputStream to support the KeyMetadataAware and delegate it to the wrapped stream, but it might entails a lot of change.

@adoroszlai
Copy link
Contributor Author

Thanks @ivandika3 for the review.

I also saw some other explicit casting instances in ObjectEndpointStreaming:

Thanks for pointing those out. I'll remove them as part of HDDS-10791.

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai for the quick fix.

LGTM+1.

@adoroszlai adoroszlai merged commit a368769 into apache:master May 2, 2024
@adoroszlai adoroszlai deleted the HDDS-10784 branch May 2, 2024 18:19
@adoroszlai
Copy link
Contributor Author

Thanks @hemantk-12, @ivandika3 for the review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 18, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants