fix(ownership): Corrects validation of ownership type and makes it consistent across graphQL calls (#9044)

Co-authored-by: Ellie O'Neil <oneile729@gmail.com>
This commit is contained in:
Pedro Silva 2023-10-19 01:43:05 +01:00 committed by GitHub
parent 409f981fd3
commit 269c4eac7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 183 additions and 71 deletions

View File

@ -2,14 +2,11 @@ package com.linkedin.datahub.graphql.resolvers.mutate;
import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.CorpuserUrn;
import com.linkedin.common.urn.Urn;
import com.linkedin.datahub.graphql.QueryContext;
import com.linkedin.datahub.graphql.exception.AuthorizationException;
import com.linkedin.datahub.graphql.generated.AddOwnerInput;
import com.linkedin.datahub.graphql.generated.OwnerEntityType;
import com.linkedin.datahub.graphql.generated.OwnerInput;
import com.linkedin.datahub.graphql.generated.OwnershipType;
import com.linkedin.datahub.graphql.generated.ResourceRefInput;
import com.linkedin.datahub.graphql.resolvers.mutate.util.OwnerUtils;
import com.linkedin.metadata.entity.EntityService;
@ -20,7 +17,6 @@ import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.*;
import static com.linkedin.datahub.graphql.resolvers.mutate.util.OwnerUtils.*;
@Slf4j
@ -32,30 +28,33 @@ public class AddOwnerResolver implements DataFetcher<CompletableFuture<Boolean>>
@Override
public CompletableFuture<Boolean> get(DataFetchingEnvironment environment) throws Exception {
final AddOwnerInput input = bindArgument(environment.getArgument("input"), AddOwnerInput.class);
Urn ownerUrn = Urn.createFromString(input.getOwnerUrn());
OwnerEntityType ownerEntityType = input.getOwnerEntityType();
OwnershipType type = input.getType() == null ? OwnershipType.NONE : input.getType();
String ownershipUrn = input.getOwnershipTypeUrn() == null ? mapOwnershipTypeToEntity(type.name()) : input.getOwnershipTypeUrn();
Urn targetUrn = Urn.createFromString(input.getResourceUrn());
OwnerInput.Builder ownerInputBuilder = OwnerInput.builder();
ownerInputBuilder.setOwnerUrn(input.getOwnerUrn());
ownerInputBuilder.setOwnerEntityType(input.getOwnerEntityType());
if (input.getType() != null) {
ownerInputBuilder.setType(input.getType());
}
if (input.getOwnershipTypeUrn() != null) {
ownerInputBuilder.setOwnershipTypeUrn(input.getOwnershipTypeUrn());
}
OwnerInput ownerInput = ownerInputBuilder.build();
if (!OwnerUtils.isAuthorizedToUpdateOwners(environment.getContext(), targetUrn)) {
throw new AuthorizationException("Unauthorized to perform this action. Please contact your DataHub administrator.");
}
return CompletableFuture.supplyAsync(() -> {
OwnerUtils.validateAddInput(
ownerUrn, input.getOwnershipTypeUrn(), ownerEntityType,
targetUrn,
_entityService
);
OwnerUtils.validateAddOwnerInput(ownerInput, ownerUrn, _entityService);
try {
log.debug("Adding Owner. input: {}", input);
Urn actor = CorpuserUrn.createFromString(((QueryContext) environment.getContext()).getActorUrn());
OwnerUtils.addOwnersToResources(
ImmutableList.of(new OwnerInput(input.getOwnerUrn(), ownerEntityType, type, ownershipUrn)),
ImmutableList.of(ownerInput),
ImmutableList.of(new ResourceRefInput(input.getResourceUrn(), null, null)),
actor,
_entityService

View File

@ -39,7 +39,7 @@ public class AddOwnersResolver implements DataFetcher<CompletableFuture<Boolean>
throw new AuthorizationException("Unauthorized to perform this action. Please contact your DataHub administrator.");
}
OwnerUtils.validateAddInput(
OwnerUtils.validateAddOwnerInput(
owners,
targetUrn,
_entityService

View File

@ -53,8 +53,7 @@ public class BatchAddOwnersResolver implements DataFetcher<CompletableFuture<Boo
private void validateOwners(List<OwnerInput> owners) {
for (OwnerInput ownerInput : owners) {
OwnerUtils.validateOwner(UrnUtils.getUrn(ownerInput.getOwnerUrn()), ownerInput.getOwnerEntityType(),
UrnUtils.getUrn(ownerInput.getOwnershipTypeUrn()), _entityService);
OwnerUtils.validateOwner(ownerInput, _entityService);
}
}

View File

@ -50,7 +50,7 @@ public class OwnerUtils {
) {
final List<MetadataChangeProposal> changes = new ArrayList<>();
for (ResourceRefInput resource : resources) {
changes.add(buildAddOwnersProposal(owners, UrnUtils.getUrn(resource.getResourceUrn()), actor, entityService));
changes.add(buildAddOwnersProposal(owners, UrnUtils.getUrn(resource.getResourceUrn()), entityService));
}
EntityUtils.ingestChangeProposals(changes, entityService, actor, false);
}
@ -69,7 +69,7 @@ public class OwnerUtils {
}
private static MetadataChangeProposal buildAddOwnersProposal(List<OwnerInput> owners, Urn resourceUrn, Urn actor, EntityService entityService) {
static MetadataChangeProposal buildAddOwnersProposal(List<OwnerInput> owners, Urn resourceUrn, EntityService entityService) {
Ownership ownershipAspect = (Ownership) EntityUtils.getAspectFromEntity(
resourceUrn.toString(),
Constants.OWNERSHIP_ASPECT_NAME, entityService,
@ -181,18 +181,13 @@ public class OwnerUtils {
orPrivilegeGroups);
}
public static Boolean validateAddInput(
public static Boolean validateAddOwnerInput(
List<OwnerInput> owners,
Urn resourceUrn,
EntityService entityService
) {
for (OwnerInput owner : owners) {
boolean result = validateAddInput(
UrnUtils.getUrn(owner.getOwnerUrn()),
owner.getOwnershipTypeUrn(),
owner.getOwnerEntityType(),
resourceUrn,
entityService);
boolean result = validateAddOwnerInput(owner, resourceUrn, entityService);
if (!result) {
return false;
}
@ -200,44 +195,29 @@ public class OwnerUtils {
return true;
}
public static Boolean validateAddInput(
Urn ownerUrn,
String ownershipEntityUrn,
OwnerEntityType ownerEntityType,
public static Boolean validateAddOwnerInput(
OwnerInput owner,
Urn resourceUrn,
EntityService entityService
) {
if (OwnerEntityType.CORP_GROUP.equals(ownerEntityType) && !Constants.CORP_GROUP_ENTITY_NAME.equals(ownerUrn.getEntityType())) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource %s. Expected a corp group urn.", resourceUrn));
}
if (OwnerEntityType.CORP_USER.equals(ownerEntityType) && !Constants.CORP_USER_ENTITY_NAME.equals(ownerUrn.getEntityType())) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource %s. Expected a corp user urn.", resourceUrn));
}
if (!entityService.exists(resourceUrn)) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource %s. Resource does not exist.", resourceUrn));
}
if (!entityService.exists(ownerUrn)) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource %s. Owner %s does not exist.", resourceUrn, ownerUrn));
}
if (ownershipEntityUrn != null && !entityService.exists(UrnUtils.getUrn(ownershipEntityUrn))) {
throw new IllegalArgumentException(String.format("Failed to change ownership type for resource %s. Ownership Type "
+ "%s does not exist.", resourceUrn, ownershipEntityUrn));
}
validateOwner(owner, entityService);
return true;
}
public static void validateOwner(
Urn ownerUrn,
OwnerEntityType ownerEntityType,
Urn ownershipEntityUrn,
OwnerInput owner,
EntityService entityService
) {
OwnerEntityType ownerEntityType = owner.getOwnerEntityType();
Urn ownerUrn = UrnUtils.getUrn(owner.getOwnerUrn());
if (OwnerEntityType.CORP_GROUP.equals(ownerEntityType) && !Constants.CORP_GROUP_ENTITY_NAME.equals(ownerUrn.getEntityType())) {
throw new IllegalArgumentException(
String.format("Failed to change ownership for resource(s). Expected a corp group urn, found %s", ownerUrn));
@ -252,9 +232,14 @@ public class OwnerUtils {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource(s). Owner with urn %s does not exist.", ownerUrn));
}
if (!entityService.exists(ownershipEntityUrn)) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource(s). Ownership type with "
+ "urn %s does not exist.", ownershipEntityUrn));
if (owner.getOwnershipTypeUrn() != null && !entityService.exists(UrnUtils.getUrn(owner.getOwnershipTypeUrn()))) {
throw new IllegalArgumentException(String.format("Failed to change ownership for resource(s). Custom Ownership type with "
+ "urn %s does not exist.", owner.getOwnershipTypeUrn()));
}
if (owner.getType() == null && owner.getOwnershipTypeUrn() == null) {
throw new IllegalArgumentException("Failed to change ownership for resource(s). Expected either "
+ "type or ownershipTypeUrn to be specified.");
}
}
@ -269,11 +254,11 @@ public class OwnerUtils {
}
public static void addCreatorAsOwner(
QueryContext context,
String urn,
OwnerEntityType ownerEntityType,
OwnershipType ownershipType,
EntityService entityService) {
QueryContext context,
String urn,
OwnerEntityType ownerEntityType,
OwnershipType ownershipType,
EntityService entityService) {
try {
Urn actorUrn = CorpuserUrn.createFromString(context.getActorUrn());
String ownershipTypeUrn = mapOwnershipTypeToEntity(ownershipType.name());

View File

@ -2,6 +2,11 @@ package com.linkedin.datahub.graphql.resolvers.owner;
import com.google.common.collect.ImmutableList;
import com.linkedin.common.AuditStamp;
import com.linkedin.common.Owner;
import com.linkedin.common.OwnerArray;
import com.linkedin.common.Ownership;
import com.linkedin.common.OwnershipSource;
import com.linkedin.common.OwnershipSourceType;
import com.linkedin.common.urn.Urn;
import com.linkedin.common.urn.UrnUtils;
import com.linkedin.datahub.graphql.QueryContext;
@ -28,6 +33,7 @@ public class AddOwnersResolverTest {
private static final String TEST_ENTITY_URN = "urn:li:dataset:(urn:li:dataPlatform:mysql,my-test,PROD)";
private static final String TEST_OWNER_1_URN = "urn:li:corpuser:test-id-1";
private static final String TEST_OWNER_2_URN = "urn:li:corpuser:test-id-2";
private static final String TEST_OWNER_3_URN = "urn:li:corpGroup:test-id-3";
@Test
public void testGetSuccessNoExistingOwners() throws Exception {
@ -75,33 +81,41 @@ public class AddOwnersResolverTest {
}
@Test
public void testGetSuccessExistingOwners() throws Exception {
public void testGetSuccessExistingOwnerNewType() throws Exception {
EntityService mockService = getMockEntityService();
com.linkedin.common.Ownership oldOwnership = new Ownership().setOwners(new OwnerArray(
ImmutableList.of(new Owner()
.setOwner(UrnUtils.getUrn(TEST_OWNER_1_URN))
.setType(com.linkedin.common.OwnershipType.NONE)
.setSource(new OwnershipSource().setType(OwnershipSourceType.MANUAL))
)));
Mockito.when(mockService.getAspect(
Mockito.eq(UrnUtils.getUrn(TEST_ENTITY_URN)),
Mockito.eq(Constants.OWNERSHIP_ASPECT_NAME),
Mockito.eq(0L)))
.thenReturn(null);
Mockito.eq(UrnUtils.getUrn(TEST_ENTITY_URN)),
Mockito.eq(Constants.OWNERSHIP_ASPECT_NAME),
Mockito.eq(0L)))
.thenReturn(oldOwnership);
Mockito.when(mockService.exists(Urn.createFromString(TEST_ENTITY_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_1_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_2_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(
OwnerUtils.mapOwnershipTypeToEntity(com.linkedin.datahub.graphql.generated.OwnershipType.TECHNICAL_OWNER.name()))))
.thenReturn(true);
OwnerUtils.mapOwnershipTypeToEntity(com.linkedin.datahub.graphql.generated.OwnershipType.TECHNICAL_OWNER.name()))))
.thenReturn(true);
AddOwnersResolver resolver = new AddOwnersResolver(mockService);
// Execute resolver
QueryContext mockContext = getMockAllowContext();
DataFetchingEnvironment mockEnv = Mockito.mock(DataFetchingEnvironment.class);
AddOwnersInput input = new AddOwnersInput(ImmutableList.of(
new OwnerInput(TEST_OWNER_1_URN, OwnerEntityType.CORP_USER, OwnershipType.TECHNICAL_OWNER,
OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name())),
new OwnerInput(TEST_OWNER_2_URN, OwnerEntityType.CORP_USER, OwnershipType.TECHNICAL_OWNER,
OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name()))
OwnerInput.builder()
.setOwnerUrn(TEST_OWNER_1_URN)
.setOwnershipTypeUrn(OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name()))
.setOwnerEntityType(OwnerEntityType.CORP_USER)
.build()
), TEST_ENTITY_URN);
Mockito.when(mockEnv.getArgument(Mockito.eq("input"))).thenReturn(input);
Mockito.when(mockEnv.getContext()).thenReturn(mockContext);
@ -111,11 +125,126 @@ public class AddOwnersResolverTest {
verifyIngestProposal(mockService, 1);
Mockito.verify(mockService, Mockito.times(1)).exists(
Mockito.eq(Urn.createFromString(TEST_OWNER_1_URN))
Mockito.eq(Urn.createFromString(TEST_OWNER_1_URN))
);
}
@Test
public void testGetSuccessDeprecatedTypeToOwnershipType() throws Exception {
EntityService mockService = getMockEntityService();
com.linkedin.common.Ownership oldOwnership = new Ownership().setOwners(new OwnerArray(
ImmutableList.of(new Owner()
.setOwner(UrnUtils.getUrn(TEST_OWNER_1_URN))
.setType(com.linkedin.common.OwnershipType.TECHNICAL_OWNER)
.setSource(new OwnershipSource().setType(OwnershipSourceType.MANUAL))
)));
Mockito.when(mockService.getAspect(
Mockito.eq(UrnUtils.getUrn(TEST_ENTITY_URN)),
Mockito.eq(Constants.OWNERSHIP_ASPECT_NAME),
Mockito.eq(0L)))
.thenReturn(oldOwnership);
Mockito.when(mockService.exists(Urn.createFromString(TEST_ENTITY_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_1_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(
OwnerUtils.mapOwnershipTypeToEntity(com.linkedin.datahub.graphql.generated.OwnershipType.TECHNICAL_OWNER.name()))))
.thenReturn(true);
AddOwnersResolver resolver = new AddOwnersResolver(mockService);
// Execute resolver
QueryContext mockContext = getMockAllowContext();
DataFetchingEnvironment mockEnv = Mockito.mock(DataFetchingEnvironment.class);
AddOwnersInput input = new AddOwnersInput(ImmutableList.of(OwnerInput.builder()
.setOwnerUrn(TEST_OWNER_1_URN)
.setOwnershipTypeUrn(OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name()))
.setOwnerEntityType(OwnerEntityType.CORP_USER)
.build()
), TEST_ENTITY_URN);
Mockito.when(mockEnv.getArgument(Mockito.eq("input"))).thenReturn(input);
Mockito.when(mockEnv.getContext()).thenReturn(mockContext);
assertTrue(resolver.get(mockEnv).get());
// Unable to easily validate exact payload due to the injected timestamp
verifyIngestProposal(mockService, 1);
Mockito.verify(mockService, Mockito.times(1)).exists(
Mockito.eq(Urn.createFromString(TEST_OWNER_1_URN))
);
}
@Test
public void testGetSuccessMultipleOwnerTypes() throws Exception {
EntityService mockService = getMockEntityService();
com.linkedin.common.Ownership oldOwnership = new Ownership().setOwners(new OwnerArray(
ImmutableList.of(new Owner()
.setOwner(UrnUtils.getUrn(TEST_OWNER_1_URN))
.setType(com.linkedin.common.OwnershipType.NONE)
.setSource(new OwnershipSource().setType(OwnershipSourceType.MANUAL))
)));
Mockito.when(mockService.getAspect(
Mockito.eq(UrnUtils.getUrn(TEST_ENTITY_URN)),
Mockito.eq(Constants.OWNERSHIP_ASPECT_NAME),
Mockito.eq(0L)))
.thenReturn(oldOwnership);
Mockito.when(mockService.exists(Urn.createFromString(TEST_ENTITY_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_1_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_2_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(TEST_OWNER_3_URN))).thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(
OwnerUtils.mapOwnershipTypeToEntity(com.linkedin.datahub.graphql.generated.OwnershipType.TECHNICAL_OWNER.name()))))
.thenReturn(true);
Mockito.when(mockService.exists(Urn.createFromString(
OwnerUtils.mapOwnershipTypeToEntity(com.linkedin.datahub.graphql.generated.OwnershipType.BUSINESS_OWNER.name()))))
.thenReturn(true);
AddOwnersResolver resolver = new AddOwnersResolver(mockService);
// Execute resolver
QueryContext mockContext = getMockAllowContext();
DataFetchingEnvironment mockEnv = Mockito.mock(DataFetchingEnvironment.class);
AddOwnersInput input = new AddOwnersInput(ImmutableList.of(OwnerInput.builder()
.setOwnerUrn(TEST_OWNER_1_URN)
.setOwnershipTypeUrn(OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name()))
.setOwnerEntityType(OwnerEntityType.CORP_USER)
.build(),
OwnerInput.builder()
.setOwnerUrn(TEST_OWNER_2_URN)
.setOwnershipTypeUrn(OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.BUSINESS_OWNER.name()))
.setOwnerEntityType(OwnerEntityType.CORP_USER)
.build(),
OwnerInput.builder()
.setOwnerUrn(TEST_OWNER_3_URN)
.setOwnershipTypeUrn(OwnerUtils.mapOwnershipTypeToEntity(OwnershipType.TECHNICAL_OWNER.name()))
.setOwnerEntityType(OwnerEntityType.CORP_GROUP)
.build()
), TEST_ENTITY_URN);
Mockito.when(mockEnv.getArgument(Mockito.eq("input"))).thenReturn(input);
Mockito.when(mockEnv.getContext()).thenReturn(mockContext);
assertTrue(resolver.get(mockEnv).get());
// Unable to easily validate exact payload due to the injected timestamp
verifyIngestProposal(mockService, 1);
Mockito.verify(mockService, Mockito.times(1)).exists(
Mockito.eq(Urn.createFromString(TEST_OWNER_1_URN))
);
Mockito.verify(mockService, Mockito.times(1)).exists(
Mockito.eq(Urn.createFromString(TEST_OWNER_2_URN))
Mockito.eq(Urn.createFromString(TEST_OWNER_2_URN))
);
Mockito.verify(mockService, Mockito.times(1)).exists(
Mockito.eq(Urn.createFromString(TEST_OWNER_3_URN))
);
}