prashantpogde commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746009751
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -102,16 +92,42 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
// client does not need any proto changes.
OMRequest.Builder omRequest = OMRequest.newBuilder()
.setUserInfo(getUserInfo())
- .setUpdateGetS3SecretRequest(updateGetS3SecretRequest)
.setCmdType(getOmRequest().getCmdType())
.setClientId(getOmRequest().getClientId());
+ // createIfNotExist defaults to true if not specified.
+ boolean createIfNotExist = !s3GetSecretRequest.hasCreateIfNotExist()
+ || s3GetSecretRequest.getCreateIfNotExist();
+
+ // Recompose GetS3SecretRequest just in case createIfNotExist is missing
+ final GetS3SecretRequest newGetS3SecretRequest =
+ GetS3SecretRequest.newBuilder()
+ .setKerberosID(accessId)
+ .setCreateIfNotExist(createIfNotExist)
+ .build();
+ omRequest.setGetS3SecretRequest(newGetS3SecretRequest);
+
+ // When createIfNotExist is true, pass UpdateGetS3SecretRequest;
+ // otherwise, just use GetS3SecretRequest.
+ if (createIfNotExist) {
+ // Generate secret. Used only when doesn't the accessId entry doesn't
+ // exist in DB, discarded otherwise.
Review comment:
nitpick: fix comment
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -124,32 +140,46 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
boolean acquiredLock = false;
IOException exception = null;
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
- UpdateGetS3SecretRequest updateGetS3SecretRequest =
- getOmRequest().getUpdateGetS3SecretRequest();
- String kerberosID = updateGetS3SecretRequest.getKerberosID();
+
+ final GetS3SecretRequest getS3SecretRequest =
+ getOmRequest().getGetS3SecretRequest();
+ assert(getS3SecretRequest.hasCreateIfNotExist());
+ final boolean createIfNotExist = getS3SecretRequest.getCreateIfNotExist();
+ final String accessId = getS3SecretRequest.getKerberosID();
+ String awsSecret = null;
+ if (createIfNotExist) {
+ final UpdateGetS3SecretRequest updateGetS3SecretRequest =
+ getOmRequest().getUpdateGetS3SecretRequest();
+ awsSecret = updateGetS3SecretRequest.getAwsSecret();
+ assert(accessId.equals(updateGetS3SecretRequest.getKerberosID()));
+ }
+
try {
- String awsSecret = updateGetS3SecretRequest.getAwsSecret();
// Note: We use the same S3_SECRET_LOCK for TenantAccessIdTable.
acquiredLock = omMetadataManager.getLock()
- .acquireWriteLock(S3_SECRET_LOCK, kerberosID);
+ .acquireWriteLock(S3_SECRET_LOCK, accessId);
Review comment:
how does it work if someone is trying to get secret for plain kerberos
ID and not access ID ? wouldn't it be safer to always take lock based on
kerberos ID ? {you can get kerberos ID for the given accessID first}
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -102,16 +92,42 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
// client does not need any proto changes.
OMRequest.Builder omRequest = OMRequest.newBuilder()
.setUserInfo(getUserInfo())
- .setUpdateGetS3SecretRequest(updateGetS3SecretRequest)
.setCmdType(getOmRequest().getCmdType())
.setClientId(getOmRequest().getClientId());
+ // createIfNotExist defaults to true if not specified.
+ boolean createIfNotExist = !s3GetSecretRequest.hasCreateIfNotExist()
+ || s3GetSecretRequest.getCreateIfNotExist();
+
+ // Recompose GetS3SecretRequest just in case createIfNotExist is missing
+ final GetS3SecretRequest newGetS3SecretRequest =
+ GetS3SecretRequest.newBuilder()
+ .setKerberosID(accessId)
Review comment:
this is confusing since it says setKerberosID() but passing it accessID.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
