You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "fmorg-git (via GitHub)" <gi...@apache.org> on 2026/01/22 06:53:52 UTC
[PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
fmorg-git opened a new pull request, #9658:
URL: https://github.com/apache/ozone/pull/9658
Please describe your PR in detail:
* Refactor constants and validation methods from endpoint and backend OzoneManager to a shared location to reduce code duplication. For example, validating roleArn, duration, etc.
* Further, certain validations were using regular expressions which perform poorly, so use more manual approaches instead.
* This PR depends on https://github.com/apache/ozone/pull/9654
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14472
## How was this patch tested?
unit tests and smoke tests
--
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
Re: [PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel merged PR #9658:
URL: https://github.com/apache/ozone/pull/9658
--
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
Re: [PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9658:
URL: https://github.com/apache/ozone/pull/9658#discussion_r2747239519
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java:
##########
@@ -41,4 +56,115 @@ public static void addAssumeRoleAuditParams(Map<String, String> auditParams, Str
auditParams.put("isPolicyIncluded", Strings.isNullOrEmpty(awsIamSessionPolicy) ? "N" : "Y");
auditParams.put("requestId", requestId);
}
+
+ /**
+ * Validates the duration in seconds.
+ * @param durationSeconds duration in seconds
+ * @return validated duration
+ * @throws OMException if duration is invalid
+ */
+ public static int validateDuration(Integer durationSeconds) throws OMException {
+ if (durationSeconds == null) {
+ return DEFAULT_DURATION_SECONDS;
+ }
+
+ if (durationSeconds < MIN_DURATION_SECONDS || durationSeconds > MAX_DURATION_SECONDS) {
+ throw new OMException(
+ "Invalid Value: DurationSeconds must be between " + MIN_DURATION_SECONDS + " and " + MAX_DURATION_SECONDS +
+ " seconds", INVALID_REQUEST);
+ }
+
+ return durationSeconds;
+ }
+
+ /**
+ * Validates the role session name.
+ * @param roleSessionName role session name
+ * @throws OMException if role session name is invalid
+ */
+ public static void validateRoleSessionName(String roleSessionName) throws OMException {
+ if (Strings.isNullOrEmpty(roleSessionName)) {
+ throw new OMException(
+ "Value null at 'roleSessionName' failed to satisfy constraint: Member must not be null", INVALID_REQUEST);
+ }
+
+ final int roleSessionNameLength = roleSessionName.length();
+ if (roleSessionNameLength < ASSUME_ROLE_SESSION_NAME_MIN_LENGTH ||
+ roleSessionNameLength > ASSUME_ROLE_SESSION_NAME_MAX_LENGTH) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
Review Comment:
Here new know the problem is that the length is outside bounds. In the loop below we know the length bound are fine (as it passed this check), buts its checking characters. Should be adjust the error messages in both cases to make it more clear that the problem is length in this case and bad characters in the other case?
--
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
Re: [PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #9658:
URL: https://github.com/apache/ozone/pull/9658#discussion_r2747472108
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java:
##########
@@ -41,4 +56,115 @@ public static void addAssumeRoleAuditParams(Map<String, String> auditParams, Str
auditParams.put("isPolicyIncluded", Strings.isNullOrEmpty(awsIamSessionPolicy) ? "N" : "Y");
auditParams.put("requestId", requestId);
}
+
+ /**
+ * Validates the duration in seconds.
+ * @param durationSeconds duration in seconds
+ * @return validated duration
+ * @throws OMException if duration is invalid
+ */
+ public static int validateDuration(Integer durationSeconds) throws OMException {
+ if (durationSeconds == null) {
+ return DEFAULT_DURATION_SECONDS;
+ }
+
+ if (durationSeconds < MIN_DURATION_SECONDS || durationSeconds > MAX_DURATION_SECONDS) {
+ throw new OMException(
+ "Invalid Value: DurationSeconds must be between " + MIN_DURATION_SECONDS + " and " + MAX_DURATION_SECONDS +
+ " seconds", INVALID_REQUEST);
+ }
+
+ return durationSeconds;
+ }
+
+ /**
+ * Validates the role session name.
+ * @param roleSessionName role session name
+ * @throws OMException if role session name is invalid
+ */
+ public static void validateRoleSessionName(String roleSessionName) throws OMException {
+ if (Strings.isNullOrEmpty(roleSessionName)) {
+ throw new OMException(
+ "Value null at 'roleSessionName' failed to satisfy constraint: Member must not be null", INVALID_REQUEST);
+ }
+
+ final int roleSessionNameLength = roleSessionName.length();
+ if (roleSessionNameLength < ASSUME_ROLE_SESSION_NAME_MIN_LENGTH ||
+ roleSessionNameLength > ASSUME_ROLE_SESSION_NAME_MAX_LENGTH) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
Review Comment:
updated
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java:
##########
@@ -41,4 +56,115 @@ public static void addAssumeRoleAuditParams(Map<String, String> auditParams, Str
auditParams.put("isPolicyIncluded", Strings.isNullOrEmpty(awsIamSessionPolicy) ? "N" : "Y");
auditParams.put("requestId", requestId);
}
+
+ /**
+ * Validates the duration in seconds.
+ * @param durationSeconds duration in seconds
+ * @return validated duration
+ * @throws OMException if duration is invalid
+ */
+ public static int validateDuration(Integer durationSeconds) throws OMException {
+ if (durationSeconds == null) {
+ return DEFAULT_DURATION_SECONDS;
+ }
+
+ if (durationSeconds < MIN_DURATION_SECONDS || durationSeconds > MAX_DURATION_SECONDS) {
+ throw new OMException(
+ "Invalid Value: DurationSeconds must be between " + MIN_DURATION_SECONDS + " and " + MAX_DURATION_SECONDS +
+ " seconds", INVALID_REQUEST);
+ }
+
+ return durationSeconds;
+ }
+
+ /**
+ * Validates the role session name.
+ * @param roleSessionName role session name
+ * @throws OMException if role session name is invalid
+ */
+ public static void validateRoleSessionName(String roleSessionName) throws OMException {
+ if (Strings.isNullOrEmpty(roleSessionName)) {
+ throw new OMException(
+ "Value null at 'roleSessionName' failed to satisfy constraint: Member must not be null", INVALID_REQUEST);
+ }
+
+ final int roleSessionNameLength = roleSessionName.length();
+ if (roleSessionNameLength < ASSUME_ROLE_SESSION_NAME_MIN_LENGTH ||
+ roleSessionNameLength > ASSUME_ROLE_SESSION_NAME_MAX_LENGTH) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
+ ASSUME_ROLE_SESSION_NAME_MAX_LENGTH + " characters long and " +
+ "contain only alphanumeric characters, +, =, ,, ., @, -", INVALID_REQUEST);
+ }
+
+ // AWS allows: alphanumeric, +, =, ,, ., @, -
+ // Pattern: [\w+=,.@-]*
+ // Don't use regex for performance reasons
+ for (int i = 0; i < roleSessionNameLength; i++) {
+ final char c = roleSessionName.charAt(i);
+ if (!isRoleSessionNameChar(c)) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
Review Comment:
updated
--
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
Re: [PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9658:
URL: https://github.com/apache/ozone/pull/9658#discussion_r2747244949
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java:
##########
@@ -41,4 +56,115 @@ public static void addAssumeRoleAuditParams(Map<String, String> auditParams, Str
auditParams.put("isPolicyIncluded", Strings.isNullOrEmpty(awsIamSessionPolicy) ? "N" : "Y");
auditParams.put("requestId", requestId);
}
+
+ /**
+ * Validates the duration in seconds.
+ * @param durationSeconds duration in seconds
+ * @return validated duration
+ * @throws OMException if duration is invalid
+ */
+ public static int validateDuration(Integer durationSeconds) throws OMException {
+ if (durationSeconds == null) {
+ return DEFAULT_DURATION_SECONDS;
+ }
+
+ if (durationSeconds < MIN_DURATION_SECONDS || durationSeconds > MAX_DURATION_SECONDS) {
+ throw new OMException(
+ "Invalid Value: DurationSeconds must be between " + MIN_DURATION_SECONDS + " and " + MAX_DURATION_SECONDS +
+ " seconds", INVALID_REQUEST);
+ }
+
+ return durationSeconds;
+ }
+
+ /**
+ * Validates the role session name.
+ * @param roleSessionName role session name
+ * @throws OMException if role session name is invalid
+ */
+ public static void validateRoleSessionName(String roleSessionName) throws OMException {
+ if (Strings.isNullOrEmpty(roleSessionName)) {
+ throw new OMException(
+ "Value null at 'roleSessionName' failed to satisfy constraint: Member must not be null", INVALID_REQUEST);
+ }
+
+ final int roleSessionNameLength = roleSessionName.length();
+ if (roleSessionNameLength < ASSUME_ROLE_SESSION_NAME_MIN_LENGTH ||
+ roleSessionNameLength > ASSUME_ROLE_SESSION_NAME_MAX_LENGTH) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
+ ASSUME_ROLE_SESSION_NAME_MAX_LENGTH + " characters long and " +
+ "contain only alphanumeric characters, +, =, ,, ., @, -", INVALID_REQUEST);
+ }
+
+ // AWS allows: alphanumeric, +, =, ,, ., @, -
+ // Pattern: [\w+=,.@-]*
+ // Don't use regex for performance reasons
+ for (int i = 0; i < roleSessionNameLength; i++) {
+ final char c = roleSessionName.charAt(i);
+ if (!isRoleSessionNameChar(c)) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
Review Comment:
In this error, we have the character which is at fault, so we could add it to the error message to make it a little bit more helpful `"Invalid character '" + c + "' in RoleSessionName ..... "`
--
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
Re: [PR] HDDS-14472. [STS] Refactor constants and validation methods to shared location [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9658:
URL: https://github.com/apache/ozone/pull/9658#discussion_r2747239519
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3STSUtils.java:
##########
@@ -41,4 +56,115 @@ public static void addAssumeRoleAuditParams(Map<String, String> auditParams, Str
auditParams.put("isPolicyIncluded", Strings.isNullOrEmpty(awsIamSessionPolicy) ? "N" : "Y");
auditParams.put("requestId", requestId);
}
+
+ /**
+ * Validates the duration in seconds.
+ * @param durationSeconds duration in seconds
+ * @return validated duration
+ * @throws OMException if duration is invalid
+ */
+ public static int validateDuration(Integer durationSeconds) throws OMException {
+ if (durationSeconds == null) {
+ return DEFAULT_DURATION_SECONDS;
+ }
+
+ if (durationSeconds < MIN_DURATION_SECONDS || durationSeconds > MAX_DURATION_SECONDS) {
+ throw new OMException(
+ "Invalid Value: DurationSeconds must be between " + MIN_DURATION_SECONDS + " and " + MAX_DURATION_SECONDS +
+ " seconds", INVALID_REQUEST);
+ }
+
+ return durationSeconds;
+ }
+
+ /**
+ * Validates the role session name.
+ * @param roleSessionName role session name
+ * @throws OMException if role session name is invalid
+ */
+ public static void validateRoleSessionName(String roleSessionName) throws OMException {
+ if (Strings.isNullOrEmpty(roleSessionName)) {
+ throw new OMException(
+ "Value null at 'roleSessionName' failed to satisfy constraint: Member must not be null", INVALID_REQUEST);
+ }
+
+ final int roleSessionNameLength = roleSessionName.length();
+ if (roleSessionNameLength < ASSUME_ROLE_SESSION_NAME_MIN_LENGTH ||
+ roleSessionNameLength > ASSUME_ROLE_SESSION_NAME_MAX_LENGTH) {
+ throw new OMException("Invalid RoleSessionName: must be " + ASSUME_ROLE_SESSION_NAME_MIN_LENGTH + "-" +
Review Comment:
Here we know the problem is that the length is outside bounds. In the loop below we know the length bounds are fine (as it passed this check), buts its checking characters. Should be adjust the error messages in both cases to make it more clear that the problem is length in this case and bad characters in the other case?
--
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