HADOOP-19197. S3A: KMS Encryption Context support: follow-up #7830
+207
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
|
🎊 +1 overall
This message was automatically generated. |
|
taking attention away from this as I've just reverted the patch from 3.4.x This still needs to go in though |
Followup the main HADOOP-19197 patch to address serialization and compilation issues * Recreate serialization ID * Restore two arg constructor * Define DEFAULT_S3_ENCRYPTION_CONTEXT to specify what the default value is (just "", but being explicit) * Tests
steveloughran
force-pushed
the
s3/HADOOP-19197-encryption-secrets
branch
from
7b1b8a6 to
5d66063
Compare
|
🎊 +1 overall
This message was automatically generated. |
raphaelazzolini
approved these changes
Jul 28, 2025
| * Encryption context: base64-encoded UTF-8 string. | ||
| */ | ||
| private String encryptionContext = ""; | ||
| private String encryptionContext = DEFAULT_S3_ENCRYPTION_CONTEXT; |
There was a problem hiding this comment.
encryptionAlgorithm and encryptionKey in the lines above are assigned to "" instead of a constant. I think we can create a default constant for them too in a future commit.
| /** | ||
| * Change this after any change to the payload: {@value}. | ||
| */ | ||
| private static final long serialVersionUID = 8834417969966697162L; |
There was a problem hiding this comment.
How did you generate this id? Is there a way to introduce a unit test to validate if new fields were added and the id should be updated?
There was a problem hiding this comment.
IntelliJ has a helper
- java serialization: you MUST update the value if the payload the set of serialized fields (everything non file not tagged as
transient) changes. I think consensus is you can generate any sufficiently random number and all is good. Key is: change it. - hadoop writable (which is how this stuff is actually marshalled in delegation tokens): you implement the read/write. This means we can be adaptive here in reading old versions too. which is what I'll do.
I don't worry about the java serialization so much as it'll only surface if people are trying to save delegation tokens in odd ways
raphaelazzolini
approved these changes
Jul 29, 2025
…rets. This allows for YARN services to load DTs supplied by older releases. If they marshall the secrets again the fact they were the older version is lost, they get upgraded. This may complicate any worker node launch where the DT list is modified before passing to the launched process
|
Latest commit will read old versions. What it doesn't do is track which version it received, so if it ever has to save that DT again it'll return a new one. I don't know if it that is an issue in the use case of hadoop 3.4.x app launched into cluster with 3.5.x servers; the DT list will now be safely parsed by the yarn RM, but if the list is saved again (and we do that for passing RM to container Credentials, don't we?) then the new version is saved. So if a container is now launched with a 3.4.x hadoop-aws module, it wouldn't be able to unmarshall the data. fix'd be to remember and use when saving, -but I need to be sure it is worth the effort first |
|
🎊 +1 overall
This message was automatically generated. |
|
@raphaelazzolini I'm really neglecting this. Do you want to take this to completion. Add your suggested changes plus tests that all is good and we can target 3.4.3 |
@steveloughran yes, I can take it to completion, I just want to confirm what we are missing here. At the current state, old versions are safely read but the new version, but we can't parse new version in the old version of the class. Is the new version -> old version scenario that you are asking me to complete? |
|
yes. if the old version gets a new record, there's nothing that can be done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followup the main HADOOP-19197 patch to address serialization and compilation issues
How was this patch tested?
Unit tests. ITests failures are meaningless until #7814 is in.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?