Skip to content

Conversation

@aryangupta1998
Copy link
Contributor

@aryangupta1998 aryangupta1998 commented Jun 27, 2023

What changes were proposed in this pull request?

Changing checkKeyAcls() to checkKeyAclsInOpenKeyTable() in S3MultipartUploadCommitPartRequest as for Acl type "WRITE", the key can only be found in OpenKeyTable.

What is the link to the Apache JIRA

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

How was this patch tested?

Tested Manually

when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
auditLogger = Mockito.mock(AuditLogger.class);
OmMetadataReader omMetadataReader = Mockito.mock(OmMetadataReader.class);
when(ozoneManager.getOmMetadataReader()).thenReturn(omMetadataReader);
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is duplicated with line 85. Need to verify if the mock is necessary.
when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);

Copy link
Contributor

Choose a reason for hiding this comment

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

They seem to be different: getMetadataManager vs. getOmMetadataReader.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, my mistake.

@adoroszlai adoroszlai requested review from jojochuang and smengcl June 29, 2023 19:48
Copy link
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

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

Thanks @aryangupta1998 for the patch, change looks good to me. The exact issue described in the jira seems not to be an issue.

@sadanand48 sadanand48 merged commit 4f19af2 into apache:master Jul 10, 2023
adoroszlai added a commit that referenced this pull request Jul 10, 2023
This reverts commit 4f19af2.

Reason for revert: compilation failure
@adoroszlai
Copy link
Contributor

@aryangupta1998 @sadanand48 I have reverted this from master because compilation is failing:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:testCompile (default-testCompile) on project ozone-manager: Compilation failure
[ERROR] /home/runner/work/ozone/ozone/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/TestS3MultipartRequest.java:[89,45] no suitable method found for thenReturn(org.apache.hadoop.ozone.om.OmMetadataReader)
[ERROR]     method org.mockito.stubbing.OngoingStubbing.thenReturn(org.apache.hadoop.ozone.om.snapshot.ReferenceCounted<org.apache.hadoop.ozone.om.IOmMetadataReader,org.apache.hadoop.ozone.om.snapshot.SnapshotCache>) is not applicable
[ERROR]       (argument mismatch; org.apache.hadoop.ozone.om.OmMetadataReader cannot be converted to org.apache.hadoop.ozone.om.snapshot.ReferenceCounted<org.apache.hadoop.ozone.om.IOmMetadataReader,org.apache.hadoop.ozone.om.snapshot.SnapshotCache>)
[ERROR]     method org.mockito.stubbing.OngoingStubbing.thenReturn(org.apache.hadoop.ozone.om.snapshot.ReferenceCounted<org.apache.hadoop.ozone.om.IOmMetadataReader,org.apache.hadoop.ozone.om.snapshot.SnapshotCache>,org.apache.hadoop.ozone.om.snapshot.ReferenceCounted<org.apache.hadoop.ozone.om.IOmMetadataReader,org.apache.hadoop.ozone.om.snapshot.SnapshotCache>...) is not applicable
[ERROR]       (argument mismatch; org.apache.hadoop.ozone.om.OmMetadataReader cannot be converted to org.apache.hadoop.ozone.om.snapshot.ReferenceCounted<org.apache.hadoop.ozone.om.IOmMetadataReader,org.apache.hadoop.ozone.om.snapshot.SnapshotCache>)

#4567 changed OzoneManager:

-  public OmMetadataReader getOmMetadataReader() {
-    return omMetadataReader;
+  public ReferenceCounted<
+      IOmMetadataReader, SnapshotCache> getOmMetadataReader() {
+    return rcOmMetadataReader;

errose28 added a commit to errose28/ozone that referenced this pull request Jul 10, 2023
* master: (36 commits)
  HDDS-8990. Intermittent timeout waiting on datanode4 9856 to become available (apache#5039)
  Revert "HDDS-7750. Incorrect WRITE ACL check. (apache#4992)"
  HDDS-7750. Incorrect WRITE ACL check. (apache#4992)
  HDDS-8985. Intermittent timeout exiting safe mode in HA secure tests (apache#5033)
  HDDS-8593. Add RootCARotationPoller to CertClient (apache#5030)
  HDDS-7645. Kubernetes check should fail fast if cluster cannot start (apache#5028)
  HDDS-8981. TestRootedOzoneFileSystem runs out of disk space (apache#5029)
  HDDS-8592. Fetch and save all root certificates during service's certificate rotation. (apache#5025)
  HDDS-8981. Disable TestRootedOzoneFileSystem#testSafeMode
  HDDS-8591. Create scheduler to check for new root ca certificates (apache#4961)
  HDDS-8979. error validating kustomization.yaml (apache#5024)
  HDDS-8973. Ozone SCM HA should not allocates duplicate IDs when transferring leadership (apache#5018)
  HDDS-8970. Snapshot Diff should return path relative to bucket root (apache#5015)
  HDDS-8975. Clarify SCM HA auto-bootstrap doc (apache#5021)
  HDDS-8689. Rotate Root CA and Sub CA in SCM. (apache#4943)
  HDDS-8436. Support setSafeMode(), isFileClosed() FileSystem API (apache#4825)
  HDDS-8880. Intermittent fork timeout in TestOMRatisSnapshots (apache#5022)
  HDDS-8962. Ensure docker env is stopped (apache#5011)
  HDDS-7794. [snapshot] SnapshotDiff should throw better error messages for exception handling (apache#5007)
  HDDS-7922. [FSO] S3G folder support fso layout filestatus s3A compatibility (apache#4448)
  ...
@aryangupta1998 aryangupta1998 deleted the HDDS-7750 branch July 11, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants