This is an automated email from the ASF dual-hosted git repository.
dhavalshah9131 pushed a commit to branch ds_RANGER-5861_mas_svctyp
in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 92d02a773f22dbce20b8b4ccec8e866b7f561277
Author: Dhaval Shah <dhavalshah9131@@gmail.com>
AuthorDate: Thu Oct 29 23:01:00 2026 +0530
RANGER-5861 : Update on service type should not be allowed
---
.../ranger/plugin/errors/ValidationErrorCode.java | 1 +
.../model/validation/RangerServiceValidator.java | 14 ++++++++
.../validation/TestRangerServiceValidator.java | 35 ++++++++++++++++++++
.../java/org/apache/ranger/biz/ServiceDBStore.java | 7 ++++
.../java/org/apache/ranger/rest/PublicAPIsv2.java | 2 +-
.../java/org/apache/ranger/rest/ServiceREST.java | 38 +++++++++++++++++-----
.../org/apache/ranger/rest/TestPublicAPIsv2.java | 4 +--
.../org/apache/ranger/rest/TestServiceREST.java | 35 ++++++++++++++++----
8 files changed, 118 insertions(+), 18 deletions(-)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
index 976dc7944..5b15bda11 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
@@ -40,6 +40,7 @@ public enum ValidationErrorCode {
SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF(1009, "Missing service def"),
SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF(1010, "Service def not found:
service-def-name=[{0}]"),
SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING(1011, "Missing required
configuration parameter(s): missing parameters={0}"),
+ SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE(1012, "Service type
cannot be changed on update: existing type=[{0}], requested type=[{1}]"),
// SERVICE-DEF VALIDATION
SERVICE_DEF_VALIDATION_ERR_UNSUPPORTED_ACTION(2001, "Internal error:
unsupported action[{0}]; isValid(Long) is only supported for DELETE"),
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
index 5debe7ed8..4035b4a6e 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
@@ -288,6 +288,20 @@ boolean isValid(RangerService service, Action action,
List<ValidationFailureDeta
valid = false;
}
+
+ if (serviceDef != null && action == Action.UPDATE && valid) {
+ RangerService existingForType = getService(id);
+ if (existingForType != null &&
!StringUtils.equals(existingForType.getType(), type)) {
+ ValidationErrorCode error =
ValidationErrorCode.SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE;
+ failures.add(new ValidationFailureDetailsBuilder()
+ .field("type")
+ .isSemanticallyIncorrect()
+ .errorCode(error.getErrorCode())
+
.becauseOf(error.getMessage(existingForType.getType(), type))
+ .build());
+ valid = false;
+ }
+ }
}
if (nameSpecified && serviceDef != null) {
diff --git
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
index bf9a90723..a77a1f1c8 100644
---
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
+++
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
@@ -516,6 +516,7 @@ public void test_isValid_happyPath() throws Exception {
when(service.getId()).thenReturn(7L);
RangerService existingService = mock(RangerService.class);
when(existingService.getId()).thenReturn(7L);
+ when(existingService.getType()).thenReturn("aType");
when(store.getService(7L)).thenReturn(existingService);
when(store.getServiceByName("aName")).thenReturn(existingService);
assertTrue(validator.isValid(service, Action.UPDATE, failures));
@@ -524,6 +525,40 @@ public void test_isValid_happyPath() throws Exception {
assertTrue(validator.isValid(service, Action.UPDATE, failures));
}
+ @Test
+ public void test_isValid_update_serviceTypeNotUpdatable() throws Exception
{
+ initialize();
+ Object[][] serviceDefInput = new Object[][] {
+ {"param1", true},
+ {"param2", true},
+ {"param3", false},
+ {"param4", false},
+ {"param5", true},
+ };
+ List<RangerServiceConfigDef> configDefs =
utils.createServiceConditionDefs(serviceDefInput);
+ RangerServiceDef serviceDef = mock(RangerServiceDef.class);
+ when(serviceDef.getConfigs()).thenReturn(configDefs);
+
+ RangerService service = mock(RangerService.class);
+ when(service.getId()).thenReturn(7L);
+ when(service.getName()).thenReturn("aName");
+ when(service.getDisplayName()).thenReturn("aDisplayName");
+ when(service.getType()).thenReturn("aType");
+ String[] configs = new String[] {"param1", "param2",
"param3", "param5", "param6"};
+ Map<String, String> configMap = utils.createMap(configs);
+ when(service.getConfigs()).thenReturn(configMap);
+
+ when(store.getServiceDefByName("aType")).thenReturn(serviceDef);
+ when(store.getServiceByName("aName")).thenReturn(null);
+
+ RangerService existingService = mock(RangerService.class);
+ when(existingService.getType()).thenReturn("storedType");
+ when(store.getService(7L)).thenReturn(existingService);
+
+ checkFailure_isValid(validator, service, Action.UPDATE, failures,
"semantic", "type");
+
assertEquals(ValidationErrorCode.SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE.getErrorCode(),
failures.get(0).errorCode);
+ }
+
@Test
public void test_isValid_withId_errorConditions() throws Exception {
initialize();
diff --git
a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
index 83e32d3ff..4fba9bc4e 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
@@ -1074,6 +1074,13 @@ public RangerService updateService(RangerService
service, Map<String, Object> op
RangerService existing = svcService.getPopulatedViewObject(xExisting);
+ if (StringUtils.isNotBlank(service.getType()) &&
!StringUtils.equals(existing.getType(), service.getType())) {
+ throw restErrorUtil.createRESTException(
+ "Service type cannot be changed on update. Existing
type=[" + existing.getType() + "], requested type=[" + service.getType() + "]",
+ MessageEnums.DATA_NOT_UPDATABLE);
+ }
+ service.setType(existing.getType());
+
String existingName = existing.getName();
boolean renamed = !StringUtils.equalsIgnoreCase(service.getName(),
existingName);
diff --git
a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
index eb1323aba..09326c651 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
@@ -464,7 +464,7 @@ public RangerService updateService(RangerService service,
@PathParam("id") Long
throw
restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "service
id mismatch", true);
}
- return serviceREST.updateService(service, request);
+ return serviceREST.updateService(service, id, request);
}
@PUT
diff --git
a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
index 665cd44f1..06dd68173 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
@@ -728,12 +728,16 @@ public RangerService createService(RangerService service)
{
return ret;
}
+ public RangerService updateService(RangerService service,
HttpServletRequest request) {
+ return updateService(service, null, request);
+ }
+
@PUT
@Path("/services/{id}")
@Consumes("application/json")
@Produces("application/json")
@PreAuthorize("@rangerPreAuthSecurityHandler.isAPIAccessible(\"" +
RangerAPIList.UPDATE_SERVICE + "\")")
- public RangerService updateService(RangerService service, @Context
HttpServletRequest request) {
+ public RangerService updateService(RangerService service, @PathParam("id")
Long reqServiceId, @Context HttpServletRequest request) {
LOG.debug("==> ServiceREST.updateService(): {}", service);
// if service.id and param 'id' are specified, service.id should be
same as the param 'id'
// if service.id is null, then set param 'id' into service Object
@@ -764,6 +768,14 @@ public RangerService updateService(RangerService service,
@Context HttpServletRe
perf = RangerPerfTracer.getPerfTracer(PERF_LOG,
"ServiceREST.updateService(serviceName=" + service.getName() + ")");
}
+ if (reqServiceId != null) {
+ if (service.getId() == null) {
+ service.setId(reqServiceId);
+ } else if (!service.getId().equals(reqServiceId)) {
+ throw
restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "service
id mismatch", true);
+ }
+ }
+
/**
* If display name is blank (EMPTY String or NULL), use previous
display name.
*/
@@ -778,10 +790,6 @@ public RangerService updateService(RangerService service,
@Context HttpServletRe
}
}
- RangerServiceValidator validator =
validatorFactory.getServiceValidator(svcStore);
-
- validator.validate(service, Action.UPDATE);
-
if (!StringUtils.isEmpty(service.getName().trim())) {
service.setName(service.getName().trim());
}
@@ -792,14 +800,28 @@ public RangerService updateService(RangerService service,
@Context HttpServletRe
bizUtil.hasAdminPermissions("Services");
- // TODO: As of now we are allowing SYS_ADMIN to create all the
- // services including KMS
+ RangerService existingService =
svcStore.getService(service.getId());
+ String storedType = existingService.getType();
- XXServiceDef xxServiceDef =
daoManager.getXXServiceDef().findByName(service.getType());
+ if (StringUtils.isNotBlank(service.getType()) &&
!StringUtils.equals(storedType, service.getType())) {
+ throw restErrorUtil.createRESTException("Service type cannot
be changed on update", MessageEnums.DATA_NOT_UPDATABLE);
+ }
+
+ service.setType(storedType);
+
+ XXServiceDef xxServiceDef =
daoManager.getXXServiceDef().findByName(storedType);
+
+ if (xxServiceDef == null) {
+ throw restErrorUtil.createRESTException("Service def not
found: service-def-name=" + storedType, MessageEnums.INVALID_INPUT_DATA);
+ }
bizUtil.hasKMSPermissions("Service",
xxServiceDef.getImplclassname());
bizUtil.blockAuditorRoleUser();
+ RangerServiceValidator validator =
validatorFactory.getServiceValidator(svcStore);
+
+ validator.validate(service, Action.UPDATE);
+
Map<String, Object> options = getOptions(request);
ret = svcStore.updateService(service, options);
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
index 1d7895b9f..d08adbc9c 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
@@ -273,13 +273,13 @@ public void test12createService() throws Exception {
public void test13updateService() throws Exception {
RangerService rangerService = rangerService();
HttpServletRequest request = null;
- Mockito.when(serviceREST.updateService(rangerService,
request)).thenReturn(rangerService);
+ Mockito.when(serviceREST.updateService(rangerService, Id,
request)).thenReturn(rangerService);
RangerService dbRangerService =
publicAPIsv2.updateService(rangerService, Id, request);
Assertions.assertNotNull(dbRangerService);
Assertions.assertEquals(dbRangerService, rangerService);
Assertions.assertEquals(dbRangerService.getId(),
rangerService.getId());
Assertions.assertEquals(dbRangerService.getName(),
rangerService.getName());
- Mockito.verify(serviceREST).updateService(rangerService, request);
+ Mockito.verify(serviceREST).updateService(rangerService, Id, request);
}
@Test
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
index bc208f314..7d81250f5 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
@@ -335,6 +335,14 @@ public XXServiceDef serviceDef() {
return xServiceDef;
}
+ private void stubStoredServiceForUpdate(RangerService existingService,
XXServiceDef xServiceDef) throws Exception {
+ XXServiceDefDao xServiceDefDao = Mockito.mock(XXServiceDefDao.class);
+
+ Mockito.when(daoManager.getXXServiceDef()).thenReturn(xServiceDefDao);
+
Mockito.when(svcStore.getService(existingService.getId())).thenReturn(existingService);
+
Mockito.when(xServiceDefDao.findByName(existingService.getType())).thenReturn(xServiceDef);
+ }
+
public XXService xService() {
XXService xService = new XXService();
xService.setAddedByUserId(Id);
@@ -545,11 +553,9 @@ public void test8updateServiceDef() throws Exception {
HttpServletRequest request = null;
Map<String, Object> options = null;
- XXServiceDefDao xServiceDefDao = Mockito.mock(XXServiceDefDao.class);
Mockito.when(validatorFactory.getServiceValidator(svcStore)).thenReturn(serviceValidator);
- Mockito.when(daoManager.getXXServiceDef()).thenReturn(xServiceDefDao);
-
Mockito.when(xServiceDefDao.findByName(rangerService.getType())).thenReturn(xServiceDef);
+ stubStoredServiceForUpdate(rangerService, xServiceDef);
Mockito.when(svcStore.updateService(Mockito.any(),
Mockito.any())).thenReturn(rangerService);
@@ -3553,8 +3559,8 @@ public void test129UpdateServiceWithValidationFailure()
throws Exception {
RangerService service = rangerService();
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
XXServiceDef xServiceDef = serviceDef();
- XXServiceDefDao xServiceDefDao = Mockito.mock(XXServiceDefDao.class);
+ stubStoredServiceForUpdate(service, xServiceDef);
Mockito.when(validatorFactory.getServiceValidator(svcStore)).thenThrow(new
RuntimeException("Validation failed"));
Mockito.when(restErrorUtil.createRESTException(Mockito.anyString())).thenReturn(new
WebApplicationException());
@@ -3758,11 +3764,9 @@ public void test141UpdateServiceWithNameTrimming()
throws Exception {
service.setDisplayName(" display-name-with-spaces ");
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
XXServiceDef xServiceDef = serviceDef();
- XXServiceDefDao xServiceDefDao = Mockito.mock(XXServiceDefDao.class);
Mockito.when(validatorFactory.getServiceValidator(svcStore)).thenReturn(serviceValidator);
- Mockito.when(daoManager.getXXServiceDef()).thenReturn(xServiceDefDao);
-
Mockito.when(xServiceDefDao.findByName(service.getType())).thenReturn(xServiceDef);
+ stubStoredServiceForUpdate(service, xServiceDef);
Mockito.when(svcStore.updateService(Mockito.any(),
Mockito.any())).thenReturn(service);
RangerService result = serviceREST.updateService(service, request);
@@ -3773,6 +3777,23 @@ public void test141UpdateServiceWithNameTrimming()
throws Exception {
Mockito.verify(svcStore).updateService(Mockito.any(RangerService.class),
Mockito.any());
}
+ @Test
+ public void test157UpdateServiceRejectsTypeChange() throws Exception {
+ RangerService service = rangerService();
+ service.setType("kms");
+ HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ RangerService storedService = rangerService();
+
+
Mockito.when(svcStore.getService(storedService.getId())).thenReturn(storedService);
+ Mockito.when(restErrorUtil.createRESTException("Service type cannot be
changed on update", MessageEnums.DATA_NOT_UPDATABLE))
+ .thenThrow(new WebApplicationException());
+
+ Assertions.assertThrows(WebApplicationException.class, () ->
serviceREST.updateService(service, request));
+
+ Mockito.verify(restErrorUtil).createRESTException("Service type cannot
be changed on update", MessageEnums.DATA_NOT_UPDATABLE);
+ Mockito.verify(svcStore, Mockito.never()).updateService(Mockito.any(),
Mockito.any());
+ }
+
@Test
public void test142GetPoliciesForResourceWithInvalidServiceDef() throws
Exception {
String serviceDefName = "invalid";
