-
Notifications
You must be signed in to change notification settings - Fork 5
feat: kubernetes tokens #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
472c512
8db919a
8a4753b
fb180b5
fa92156
3711118
d2d711c
681a91b
a516ba9
1704a49
4c77c4a
c4e58a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,66 @@ | ||||||||||||||||
| package com.netcracker.cloud.dbaas.config.security; | ||||||||||||||||
|
|
||||||||||||||||
| import io.quarkus.security.identity.IdentityProviderManager; | ||||||||||||||||
| import io.quarkus.security.identity.SecurityIdentity; | ||||||||||||||||
| import io.quarkus.security.identity.request.AuthenticationRequest; | ||||||||||||||||
| import io.quarkus.smallrye.jwt.runtime.auth.JWTAuthMechanism; | ||||||||||||||||
| import io.quarkus.vertx.http.runtime.security.BasicAuthenticationMechanism; | ||||||||||||||||
| import io.quarkus.vertx.http.runtime.security.ChallengeData; | ||||||||||||||||
| import io.quarkus.vertx.http.runtime.security.HttpAuthenticationMechanism; | ||||||||||||||||
| import io.quarkus.vertx.http.runtime.security.HttpCredentialTransport; | ||||||||||||||||
| import io.smallrye.mutiny.Uni; | ||||||||||||||||
| import io.vertx.ext.web.RoutingContext; | ||||||||||||||||
| import jakarta.annotation.Priority; | ||||||||||||||||
| import jakarta.enterprise.context.ApplicationScoped; | ||||||||||||||||
| import jakarta.inject.Inject; | ||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||
|
|
||||||||||||||||
| import java.util.HashSet; | ||||||||||||||||
| import java.util.Set; | ||||||||||||||||
|
|
||||||||||||||||
| @Priority(1) | ||||||||||||||||
| @ApplicationScoped | ||||||||||||||||
| @Slf4j | ||||||||||||||||
| public class BasicAndKubernetesAuthMechanism implements HttpAuthenticationMechanism { | ||||||||||||||||
| @Inject | ||||||||||||||||
| BasicAuthenticationMechanism basicAuth; | ||||||||||||||||
|
|
||||||||||||||||
| @Inject | ||||||||||||||||
| JWTAuthMechanism jwtAuth; | ||||||||||||||||
|
|
||||||||||||||||
| @Override | ||||||||||||||||
| public Uni<SecurityIdentity> authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) { | ||||||||||||||||
| return selectMechanism(context).authenticate(context, identityProviderManager); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| @Override | ||||||||||||||||
| public Uni<ChallengeData> getChallenge(RoutingContext context) { | ||||||||||||||||
| return selectMechanism(context).getChallenge(context); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| @Override | ||||||||||||||||
| public Set<Class<? extends AuthenticationRequest>> getCredentialTypes() { | ||||||||||||||||
| Set<Class<? extends AuthenticationRequest>> types = new HashSet<>(); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Set.of() for immutable sets if the set doesn't need modification, otherwise initialize with expected capacity.
Suggested change
#deepseek-review:inline |
||||||||||||||||
| types.addAll(basicAuth.getCredentialTypes()); | ||||||||||||||||
| types.addAll(jwtAuth.getCredentialTypes()); | ||||||||||||||||
| return types; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| @Override | ||||||||||||||||
| public Uni<HttpCredentialTransport> getCredentialTransport(RoutingContext context) { | ||||||||||||||||
| return selectMechanism(context).getCredentialTransport(context); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| private HttpAuthenticationMechanism selectMechanism(RoutingContext context) { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mechanism selection logic should handle cases where both authentication methods are present or neither is present. #deepseek-review:inline |
||||||||||||||||
| if (isBearerTokenPresent(context)) { | ||||||||||||||||
| return jwtAuth; | ||||||||||||||||
| } else { | ||||||||||||||||
| return basicAuth; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| private boolean isBearerTokenPresent(RoutingContext context) { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract 'Bearer ' string to a constant to avoid magic strings and improve maintainability.
Suggested change
#deepseek-review:inline |
||||||||||||||||
| String authHeader = context.request().getHeader("Authorization"); | ||||||||||||||||
| return authHeader != null && authHeader.startsWith("Bearer "); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package com.netcracker.cloud.dbaas.config.security; | ||
|
|
||
| import com.netcracker.cloud.security.core.utils.k8s.KubernetesTokenVerificationException; | ||
| import com.netcracker.cloud.security.core.utils.k8s.KubernetesTokenVerifier; | ||
| import io.quarkus.runtime.StartupEvent; | ||
| import io.smallrye.jwt.auth.principal.*; | ||
| import jakarta.annotation.Priority; | ||
| import jakarta.enterprise.context.ApplicationScoped; | ||
| import jakarta.enterprise.event.Observes; | ||
| import jakarta.enterprise.inject.Alternative; | ||
| import jakarta.inject.Inject; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.eclipse.microprofile.config.inject.ConfigProperty; | ||
|
|
||
| @ApplicationScoped | ||
| @Alternative | ||
| @Priority(1) | ||
| @Slf4j | ||
| public class KubernetesJWTCallerPrincipalFactory extends JWTCallerPrincipalFactory { | ||
|
|
||
| private final KubernetesTokenVerifier verifier; | ||
|
|
||
| @Inject | ||
| public KubernetesJWTCallerPrincipalFactory( | ||
| @ConfigProperty(name = "dbaas.security.k8s.jwt.enabled") boolean isJwtEnabled, | ||
|
||
| @ConfigProperty(name = "dbaas.security.k8s.jwt.audience") String jwtAudience | ||
| ) { | ||
| if (!isJwtEnabled) { | ||
| log.info("JWT not enabled, skipping verifier initialization"); | ||
| this.verifier = null; | ||
| return; | ||
| } | ||
|
|
||
| log.info("Initializing KubernetesTokenVerifier"); | ||
| try { | ||
| this.verifier = new KubernetesTokenVerifier(jwtAudience); | ||
| log.info("KubernetesTokenVerifier initialized successfully"); | ||
| } catch (RuntimeException e) { | ||
| log.error("Failed to initialize KubernetesTokenVerifier", e); | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| KubernetesJWTCallerPrincipalFactory(KubernetesTokenVerifier verifier) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Package-private constructor for testing bypasses configuration checks, potentially creating an inconsistent state. #deepseek-review:inline |
||
| this.verifier = verifier; | ||
| } | ||
|
|
||
| @Override | ||
| public JWTCallerPrincipal parse(String token, JWTAuthContextInfo authContextInfo) throws ParseException { | ||
| try { | ||
| var prin = new DefaultJWTCallerPrincipal(verifier.verify(token)); | ||
|
||
| return prin; | ||
| } catch (KubernetesTokenVerificationException e) { | ||
| throw new ParseException("failed to parse kubernetes projected volume token", e); | ||
| } | ||
| } | ||
|
|
||
| // observe StartupEvent so that bean is created at startup | ||
| void onStartUp(@Observes StartupEvent event) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty startup observer method adds no value and should be removed. #deepseek-review:inline |
||
| } | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,22 +1,21 @@ | ||||||||||||
| package com.netcracker.cloud.dbaas.controller.error; | ||||||||||||
|
|
||||||||||||
| import com.netcracker.cloud.dbaas.exceptions.ForbiddenDeleteOperationException; | ||||||||||||
| import jakarta.ws.rs.core.Context; | ||||||||||||
| import jakarta.ws.rs.core.Response; | ||||||||||||
| import jakarta.ws.rs.core.UriInfo; | ||||||||||||
| import jakarta.ws.rs.ext.ExceptionMapper; | ||||||||||||
| import jakarta.ws.rs.ext.Provider; | ||||||||||||
| import com.netcracker.cloud.dbaas.exceptions.ForbiddenException; | ||||||||||||
|
|
||||||||||||
| import static com.netcracker.cloud.dbaas.controller.error.Utils.buildDefaultResponse; | ||||||||||||
|
|
||||||||||||
| @Provider | ||||||||||||
| public class ForbiddenDeleteOperationExceptionMapper implements ExceptionMapper<ForbiddenDeleteOperationException> { | ||||||||||||
|
|
||||||||||||
| public class ForbiddenExceptionMapper implements ExceptionMapper<ForbiddenException> { | ||||||||||||
| @Context | ||||||||||||
| UriInfo uriInfo; | ||||||||||||
|
|
||||||||||||
| @Override | ||||||||||||
| public Response toResponse(ForbiddenDeleteOperationException e) { | ||||||||||||
| public Response toResponse(ForbiddenException e) { | ||||||||||||
| return buildDefaultResponse(uriInfo, e, Response.Status.FORBIDDEN); | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding null check for uriInfo to prevent potential NullPointerException.
Suggested change
#deepseek-review:inline |
||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||
| package com.netcracker.cloud.dbaas.controller.v3; | ||||||||
|
|
||||||||
| import com.netcracker.cloud.context.propagation.core.ContextManager; | ||||||||
| import com.netcracker.cloud.dbaas.controller.abstact.AbstractDatabaseAdministrationController; | ||||||||
| import com.netcracker.cloud.dbaas.dto.ClassifierWithRolesRequest; | ||||||||
| import com.netcracker.cloud.dbaas.dto.Source; | ||||||||
|
|
@@ -12,13 +13,19 @@ | |||||||
| import com.netcracker.cloud.dbaas.exceptions.NotFoundException; | ||||||||
| import com.netcracker.cloud.dbaas.exceptions.*; | ||||||||
| import com.netcracker.cloud.dbaas.monitoring.model.DatabasesInfo; | ||||||||
| import com.netcracker.cloud.dbaas.security.validators.NamespaceValidator; | ||||||||
| import com.netcracker.cloud.dbaas.service.*; | ||||||||
| import com.netcracker.cloud.dbaas.utils.JwtUtils; | ||||||||
| import com.netcracker.cloud.framework.contexts.tenant.BaseTenantProvider; | ||||||||
| import com.netcracker.cloud.framework.contexts.tenant.TenantContextObject; | ||||||||
| import io.smallrye.jwt.auth.principal.DefaultJWTCallerPrincipal; | ||||||||
| import jakarta.annotation.security.RolesAllowed; | ||||||||
| import jakarta.inject.Inject; | ||||||||
| import jakarta.transaction.Transactional; | ||||||||
| import jakarta.ws.rs.*; | ||||||||
| import jakarta.ws.rs.core.MediaType; | ||||||||
| import jakarta.ws.rs.core.Response; | ||||||||
| import jakarta.ws.rs.core.SecurityContext; | ||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||
| import org.eclipse.microprofile.openapi.annotations.Operation; | ||||||||
| import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; | ||||||||
|
|
@@ -63,6 +70,10 @@ public class AggregatedDatabaseAdministrationControllerV3 extends AbstractDataba | |||||||
| PasswordEncryption encryption; | ||||||||
| @Inject | ||||||||
| BlueGreenService blueGreenService; | ||||||||
| @Inject | ||||||||
| NamespaceValidator namespaceValidator; | ||||||||
| @Inject | ||||||||
| SecurityContext securityContext; | ||||||||
|
|
||||||||
| @Operation(summary = "V3. Creates new database V3", | ||||||||
| description = "Creates new database and returns it with connection information, " + | ||||||||
|
|
@@ -84,11 +95,11 @@ public Response createDatabase(@Parameter(description = "The model for creating | |||||||
| @PathParam(NAMESPACE_PARAMETER) String namespace, | ||||||||
| @Parameter(description = "Determines if database should be created asynchronously") | ||||||||
| @QueryParam(ASYNC_PARAMETER) Boolean async) { | ||||||||
| if (!AggregatedDatabaseAdministrationService.AggregatedDatabaseAdministrationUtils.isClassifierCorrect(createRequest.getClassifier())) { | ||||||||
| throw new InvalidClassifierException("Classifier doesn't contain all mandatory fields. " + | ||||||||
| "Check that classifier has `microserviceName`, `scope`. If `scope` = `tenant`, classifier must contain `tenantId` property", | ||||||||
| createRequest.getClassifier(), Source.builder().pointer("/classifier").build()); | ||||||||
| if (!AggregatedDatabaseAdministrationService.AggregatedDatabaseAdministrationUtils.isClassifierCorrect(createRequest.getClassifier()) || | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's split these two checks and throw different exception messages for them (in all similar cases).
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combine multiple validation conditions into separate if statements for better readability. #deepseek-review:inline |
||||||||
| !namespaceValidator.checkNamespaceFromClassifier(securityContext, createRequest.getClassifier())) { | ||||||||
| throw InvalidClassifierException.withDefaultMsg(createRequest.getClassifier()); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use specific exception messages instead of generic ones to help with debugging. #deepseek-review:inline |
||||||||
| } | ||||||||
| checkTenantId(createRequest.getClassifier()); | ||||||||
| checkOriginService(createRequest); | ||||||||
|
|
||||||||
| namespace = (String) createRequest.getClassifier().get(NAMESPACE); | ||||||||
|
|
@@ -196,9 +207,10 @@ public Response getDatabaseByClassifier(@Parameter(description = "Classifier on | |||||||
| @Parameter(description = "The type of base in which the database was created. For example PostgreSQL or MongoDB", required = true) | ||||||||
| @PathParam("type") String type) { | ||||||||
| checkOriginService(classifierRequest); | ||||||||
| if (!dBaaService.isValidClassifierV3(classifierRequest.getClassifier())) { | ||||||||
| if (!dBaaService.isValidClassifierV3(classifierRequest.getClassifier()) || !namespaceValidator.checkNamespaceFromClassifier(securityContext, classifierRequest.getClassifier())) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split complex conditional into separate if statements for better maintainability. #deepseek-review:inline |
||||||||
| throw new InvalidClassifierException("Invalid V3 classifier", classifierRequest.getClassifier(), Source.builder().pointer("").build()); | ||||||||
| } | ||||||||
| checkTenantId(classifierRequest.getClassifier()); | ||||||||
| checkOriginService(classifierRequest); | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the duplicated check.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||||||||
| namespace = (String) classifierRequest.getClassifier().get(NAMESPACE); | ||||||||
|
|
||||||||
|
|
@@ -280,11 +292,10 @@ public Response addExternalDatabase(@Parameter(description = "Request with conne | |||||||
| @Parameter(description = "Namespace with which new database will be connected", required = true) | ||||||||
| @PathParam(NAMESPACE_PARAMETER) String namespace) { | ||||||||
| log.info("Get request on adding external database with classifier {} and type {} in namespace {}", externalDatabaseRequest.getClassifier(), externalDatabaseRequest.getType(), namespace); | ||||||||
| if (!AggregatedDatabaseAdministrationService.AggregatedDatabaseAdministrationUtils.isClassifierCorrect(externalDatabaseRequest.getClassifier())) { | ||||||||
| throw new InvalidClassifierException("Classifier doesn't contain all mandatory fields. " + | ||||||||
| "Check that classifier has `microserviceName`, `scope`. If `scope` = `tenant`, classifier must contain `tenantId` property", | ||||||||
| externalDatabaseRequest.getClassifier(), Source.builder().pointer("/classifier").build()); | ||||||||
| if (!AggregatedDatabaseAdministrationService.AggregatedDatabaseAdministrationUtils.isClassifierCorrect(externalDatabaseRequest.getClassifier()) || !namespaceValidator.checkNamespaceFromClassifier(securityContext, externalDatabaseRequest.getClassifier())) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split long conditional expression into separate statements for clarity. #deepseek-review:inline |
||||||||
| throw InvalidClassifierException.withDefaultMsg(externalDatabaseRequest.getClassifier()); | ||||||||
| } | ||||||||
| checkTenantId(externalDatabaseRequest.getClassifier()); | ||||||||
| DatabaseRegistry databaseRegistry = externalDatabaseRequest.toDatabaseRegistry(); | ||||||||
| Optional<BgDomain> bgDomainOpt = aggregatedDatabaseAdministrationService.getBgDomain(namespace); | ||||||||
| if (bgDomainOpt.isPresent()) { | ||||||||
|
|
@@ -396,9 +407,23 @@ public Response deleteDatabaseByClassifier(@Parameter(description = "A unique id | |||||||
| } | ||||||||
|
|
||||||||
| private void checkOriginService(UserRolesServices rolesServices) { | ||||||||
| if (securityContext.getUserPrincipal() instanceof DefaultJWTCallerPrincipal principal) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null check for securityContext.getUserPrincipal() to prevent potential NPE.
Suggested change
#deepseek-review:inline |
||||||||
| rolesServices.setOriginService(JwtUtils.getServiceAccountName(principal)); | ||||||||
| } | ||||||||
| if (rolesServices.getOriginService() == null || rolesServices.getOriginService().isEmpty()) { | ||||||||
| log.error("Request body={} must contain originService", rolesServices); | ||||||||
| throw new InvalidOriginServiceException(); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private void checkTenantId(Map<String, Object> classifier) { | ||||||||
| if (!Objects.equals(classifier.get(SCOPE), SCOPE_VALUE_TENANT)) { | ||||||||
| return; | ||||||||
| } | ||||||||
| String tenantId = ((TenantContextObject) ContextManager.get(BaseTenantProvider.TENANT_CONTEXT_NAME)).getTenant(); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null check for ContextManager.get result to prevent ClassCastException.
Suggested change
#deepseek-review:inline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract tenant ID retrieval into a separate method for reusability. #deepseek-review:inline |
||||||||
| if (tenantId.equals(classifier.get(TENANT_ID))) { | ||||||||
| return; | ||||||||
| } | ||||||||
| throw new ForbiddenTenantIdException(); | ||||||||
| } | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,7 @@ public class ClassifierWithRolesRequest implements UserRolesServices { | |||||
| @Schema(description = "Database composite identify key. See details in https://perch.qubership.org/display/CLOUDCORE/DbaaS+Database+Classifier", required = true) | ||||||
| private Map<String, Object> classifier; | ||||||
|
|
||||||
| @Schema(description = "Origin service which send request", required = true) | ||||||
| @Schema(description = "Origin service which send request") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Grammar error in description - should be 'sends request' instead of 'send request'.
Suggested change
#deepseek-review:inline |
||||||
| private String originService; | ||||||
|
|
||||||
| @Schema(description = "Indicates connection properties with which user role should be returned to a client") | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||
| package com.netcracker.cloud.dbaas.dto.role; | ||||||
|
|
||||||
| import lombok.Getter; | ||||||
| import lombok.Setter; | ||||||
|
|
||||||
| import java.util.Set; | ||||||
|
|
||||||
| public class ServiceAccountWithRoles { | ||||||
|
||||||
| @Setter | ||||||
| @Getter | ||||||
| private String name; | ||||||
|
||||||
| private String name; | |
| private Set<String> roles = new HashSet<>(); |
#deepseek-review:inline
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ public DatabaseCreateRequestV3(@NonNull Map<String, Object> classifier, @NonNull | |
| super(classifier, type); | ||
| } | ||
|
|
||
| @Schema(description = "Origin service which send request", required = true) | ||
| @Schema(description = "Origin service which send request") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| private String originService; | ||
|
|
||
| @Schema(description = "Indicates connection properties with which user role should be returned to a client") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
| public interface UserRolesServices { | ||
| String getOriginService(); | ||
|
|
||
| void setOriginService(String originService); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding @param javadoc to document the originService parameter. #deepseek-review:inline |
||
|
|
||
| String getUserRole(); | ||
|
|
||
| Map<String, Object> getClassifier(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -208,6 +208,10 @@ public enum ErrorCodes implements ErrorCode { | |||||
| "Adapter address name has wrong format", | ||||||
| "register request contains adapter address field, but it has wrong format: %s. " + | ||||||
| "Must be in format: <schema>://<service-name>.<namespace>:<port>, e.g.: http://dbaas-postgres-adapter.postgresql:8080"), | ||||||
| CORE_DBAAS_4046( | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error code CORE_DBAAS_4046 should follow the established naming pattern with consistent spacing.
Suggested change
#deepseek-review:inline |
||||||
| "CORE-DBAAS-4046", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error code string should be consistent with the pattern used in other error codes.
Suggested change
#deepseek-review:inline |
||||||
| "Invalid tenantId in classifier", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error message should be more descriptive about what constitutes an invalid tenantId.
Suggested change
#deepseek-review:inline |
||||||
| "tenantId from classifier and tenantId from request don't match"), | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error description should be more specific about the nature of the mismatch.
Suggested change
#deepseek-review:inline |
||||||
|
|
||||||
| CORE_DBAAS_7002( | ||||||
| "CORE-DBAAS-7002", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class ForbiddenDeleteBackupOperationException extends ErrorCodeException { | ||
| public class ForbiddenDeleteBackupOperationException extends ForbiddenException { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good change - extending ForbiddenException is more semantically appropriate than ErrorCodeException for forbidden operations. #deepseek-review:inline |
||
|
|
||
| public ForbiddenDeleteBackupOperationException(String detail) { | ||
| super(ErrorCodes.CORE_DBAAS_4013, ErrorCodes.CORE_DBAAS_4013.getDetail(detail)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class ForbiddenDeleteOperationException extends ErrorCodeException { | ||
| public class ForbiddenDeleteOperationException extends ForbiddenException { | ||
|
|
||
| public ForbiddenDeleteOperationException() { | ||
| super(ErrorCodes.CORE_DBAAS_4003, ErrorCodes.CORE_DBAAS_4003.getDetail()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error code CORE_DBAAS_4003 appears to be for validation errors, not forbidden operations - consider using a more appropriate error code. #deepseek-review:inline |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using SNAPSHOT version may cause build instability; consider using a stable release version.
#deepseek-review:inline