You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/11/04 19:27:12 UTC
[GitHub] [ozone] smengcl opened a new pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
smengcl opened a new pull request #2804:
URL: https://github.com/apache/ozone/pull/2804
## What changes were proposed in this pull request?
Currently `ozone s3 getsecret` automatically generates secret for an accessId if an accessId does not exist in OM DB.
For `ozone tenant user getsecret` we do not want such behavior.
The approach is to extend the existing ObjectStore#getS3Secret call to support the createIfNotExist argument.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5939
## How was this patch tested?
- Added new checks in `TestOzoneTenantShell`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #2804: HDDS-5939. [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2804:
URL: https://github.com/apache/ozone/pull/2804
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#issuecomment-964603196
@prashantpogde Thanks for reviewing this. I have updated the code comments.
The code logic itself isn't changed in any way with the latest commit. The only changes are in the comments.
Pls take another look. Thanks!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746083525
##########
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:
It turns out renaming the proto field from `kerberosID` to `accessId` is failing the proto compatibility check locally.
```
[INFO] --- proto-backwards-compatibility:1.0.5:backwards-compatibility-check (default) @ ozone-interface-client ---
[ERROR] CONFLICT: "GetS3SecretRequest" field: "accessId" ID: 1 has an updated name, previously "kerberosID" [OmClientProtocol.proto]
[ERROR] CONFLICT: "GetS3SecretRequest" field: "kerberosID" has been removed, but is not reserved [OmClientProtocol.proto]
```
I will add a comment in the code without renaming the proto field for the moment.
If we want to do the rename, we should probably do this later in a refactoring patch in the master branch. Because there are a lot of other references of `.setKerberosID`, `.getKerberosID` and in other proto messages as well (not just for `GetS3SecretRequest`).
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746081313
##########
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:
Hmm this comment is accurate, actually. It might have been a bit misleading. I am pushing an update.
Note this `S3GetSecretRequest` is not a new class. This is shared between the existing `ozone s3 getsecret`, and the new `ozone tenant s3 getsecret`. That's why we still have the generate secret logic here.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] prashantpogde commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746092996
##########
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:
This change is simply because of the varible renaming, from:
```
String kerberosID = updateGetS3SecretRequest.getKerberosID();
```
to
```
final String accessId = getS3SecretRequest.getKerberosID();
```
This change by itself wouldn't cause any problems. Since it is just a variable renaming.
We are placing the lock on `accessId` because it is the key of `S3SecretTable` and `TenantAccessIdTable`. It wouldn't make sense to lock the tables by **actual** Kerberos ID here. In my understanding, the goal of the lock is to prevent users from updating the **same** line simultaneously. Not to prevent the same user from updating the table from multiple clients at the same time.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746077015
##########
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:
With the introduction of multi-tenancy the meaning of the proto field has changed.
Before multi-tenancy, the accessId field is the same as Kerberos ID (and that's probably why the field was named `kerberosID`).
Now we are allowing arbitrary accessIds in multi-tenancy so `accessId == kerberosID` doesn't apply anymore. And the proto message field should have been renamed to `accessId` in previous MT patches.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746081313
##########
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:
Hmm this comment is accurate, actually. It might have been a bit misleading. I am pushing an update.
Note this `S3GetSecretRequest` is not a new class. This is shared between the existing `ozone s3 getsecret`, and the new `ozone tenant s3 getsecret` to reduce code duplication. That's why we still have the generate secret logic here -- it's for the old `s3 getsecret`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746083525
##########
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:
It turns out renaming the proto field from `kerberosID` to `` is failing the proto compatibility check locally.
```
[INFO] --- proto-backwards-compatibility:1.0.5:backwards-compatibility-check (default) @ ozone-interface-client ---
[ERROR] CONFLICT: "GetS3SecretRequest" field: "accessId" ID: 1 has an updated name, previously "kerberosID" [OmClientProtocol.proto]
[ERROR] CONFLICT: "GetS3SecretRequest" field: "kerberosID" has been removed, but is not reserved [OmClientProtocol.proto]
```
I will add a comment in the code without renaming the proto field for the moment.
If we want to do the rename, we should probably do this later in a refactoring patch in the master branch. Because there are a lot of other references of `.setKerberosID`, `.getKerberosID` and in other proto messages as well (not just for `GetS3SecretRequest`).
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746081313
##########
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:
Hmm this comment is accurate, actually. It might have been a bit misleading. I am pushing an update.
Note this `S3GetSecretRequest` is not a new class. This is shared between the existing `ozone s3 getsecret`, and the new `ozone tenant s3 getsecret`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2804: HDDS-5939. [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#issuecomment-965950341
Thanks @prashantpogde for reviewing this. Since the second last run passed, and the last commit doesn't have any code changes. It's just a flaky acceptance (HA) run. Will merge this soon.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2804: [Multi-Tenant] Implement `ozone tenant user getsecret` that does not generate secret when accessId does not exist
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746081313
##########
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:
Hmm this comment is accurate, actually. It might have been a bit misleading. I am pushing an update.
Note this `S3GetSecretRequest` is not a new class. This is shared between the existing `ozone s3 getsecret`, and the new `ozone tenant s3 getsecret` to reduce code duplication. That's why we still have the generate secret logic here.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org