poojanilangekar commented on code in PR #1840: URL: https://github.com/apache/polaris/pull/1840#discussion_r2141228520
########## polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReferenceUrnHelper.java: ########## @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.secrets; + +import com.google.common.base.Preconditions; +import jakarta.annotation.Nonnull; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Utility class for UserSecretReference URNs. Provides the ability to validate, parse, and build + * URNs. + */ +public final class UserSecretReferenceUrnHelper { + + private UserSecretReferenceUrnHelper() {} + + private static final String URN_SCHEME = "urn"; + private static final String URN_NAMESPACE = "polaris-secret"; + private static final String SECRET_MANAGER_TYPE_REGEX = "([a-zA-Z0-9_-]+)"; + private static final String TYPE_SPECIFIC_IDENTIFIER_REGEX = + "([a-zA-Z0-9_-]+(?::[a-zA-Z0-9_-]+)*)"; + + /** + * Precompiled regex pattern for validating the secret manager type and type-specific identifier. + */ + private static final Pattern SECRET_MANAGER_TYPE_PATTERN = + Pattern.compile("^" + SECRET_MANAGER_TYPE_REGEX + "$"); + + private static final Pattern TYPE_SPECIFIC_IDENTIFIER_PATTERN = + Pattern.compile("^" + TYPE_SPECIFIC_IDENTIFIER_REGEX + "$"); + + /** + * Precompiled regex pattern for validating and parsing UserSecretReference URNs. Expected format: + * urn:polaris-secret:<secret-manager-type>:<identifier1>(:<identifier2>:...). + * + * <p>Groups: + * + * <p>Group 1: secret-manager-type (alphanumeric, hyphens, underscores). + * + * <p>Group 2: type-specific-identifier (one or more colon-separated alphanumeric components). + */ + private static final Pattern URN_PATTERN = + Pattern.compile( + "^" + + URN_SCHEME + + ":" + + URN_NAMESPACE + + ":" + + SECRET_MANAGER_TYPE_REGEX + + ":" + + TYPE_SPECIFIC_IDENTIFIER_REGEX + + "$"); + + /** + * Validates whether the given URN string matches the expected format for UserSecretReference + * URNs. + * + * @param urn The URN string to validate. + * @return true if the URN is valid, false otherwise. + */ + public static boolean isValid(@Nonnull String urn) { + return urn.trim().isEmpty() ? false : URN_PATTERN.matcher(urn).matches(); + } + + public static String getUrnPattern() { + return URN_PATTERN.toString(); + } + + /** + * Extracts the secret manager type from a valid URN. + * + * @param urn The URN string to parse. + * @return The secret manager type. + */ + @Nonnull + public static String getSecretManagerType(@Nonnull String urn) { + Matcher matcher = URN_PATTERN.matcher(urn); + Preconditions.checkState(matcher.matches(), "Invalid secret URN: " + urn); + return matcher.group(1); + } + + /** + * Extracts the type-specific identifier from a valid URN. + * + * @param urn The URN string to parse. + * @return The type-specific identifier. + */ + @Nonnull + public static String getTypeSpecificIdentifier(@Nonnull String urn) { Review Comment: Currently, the function is never called (except in tests). I added it here to support any future use cases that may arise. From a validation perspective, I think it is the responsibility of the caller because the type-specific format is unknown to this function. Unless you're suggesting that we accept a regex of the expected format. ########## polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java: ########## @@ -73,9 +73,9 @@ public UserSecretReference writeSecret( String secretUrn; for (int secretOrdinal = 0; ; ++secretOrdinal) { + String typeSpecificIdentifier = forEntity.getId() + ":" + secretOrdinal; secretUrn = - String.format( - "urn:polaris-secret:unsafe-in-memory:%d:%d", forEntity.getId(), secretOrdinal); + UserSecretReferenceUrnHelper.buildUrn(SECRET_MANAGER_TYPE, typeSpecificIdentifier); Review Comment: Done. Although I might have missed something here, please let me know if you think the buildUrn function meets your intended goal. ########## polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java: ########## @@ -45,6 +43,8 @@ public class UnsafeInMemorySecretsManager implements UserSecretsManager { private static final String CIPHERTEXT_HASH = "ciphertext-hash"; private static final String ENCRYPTION_KEY = "encryption-key"; + private static final String SECRET_MANAGER_TYPE = "unsafe-in-memory"; Review Comment: Done, I've made it public. ########## polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReference.java: ########## @@ -88,8 +88,7 @@ public UserSecretReference( */ @JsonIgnore public String getUserSecretManagerTypeFromUrn() { - // TODO: Add better/standardized parsing and validation of URN syntax - return urn.split(":")[2]; + return UserSecretReferenceUrnHelper.getSecretManagerType(urn); Review Comment: At the moment, we don't keep a registry of all the UserSecretReferences created by a given Polaris instance. Or are you suggesting something different? -- 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-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
