From b27bdcd68081ad8a9d72cd80d5770199c49f5ac2 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 2 Nov 2023 21:02:35 +0530 Subject: [PATCH 1/6] PLT-302 Optimize Persona ES alias --- .../store/aliasstore/ESAliasStore.java | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 2b01f29399..67dde2e170 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -38,6 +38,7 @@ import javax.inject.Inject; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -165,6 +166,7 @@ private Map getFilterForPurpose(AtlasEntity purpose) throws Atla private void personaPolicyToESDslClauses(List policies, List> allowClauseList) throws AtlasBaseException { + Set terms = new HashSet<>(); for (AtlasEntity policy: policies) { if (policy.getStatus() == null || AtlasEntity.Status.ACTIVE.equals(policy.getStatus())) { @@ -178,19 +180,23 @@ private void personaPolicyToESDslClauses(List policies, } for (String asset : assets) { - addPersonaMetadataFilterClauses(asset, allowClauseList); + terms.add(asset); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); } - addPersonaMetadataFilterConnectionClause(connectionQName, allowClauseList); + terms.add(connectionQName); } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { for (String glossaryQName : assets) { - addPersonaGlossaryFilterClauses(glossaryQName, allowClauseList); + terms.add(glossaryQName); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); } } } } } + + allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } private Map esClausesToFilter(List> allowClauseList) { @@ -208,22 +214,6 @@ private String getAliasName(AtlasEntity entity) { return getESAliasName(entity); } - private void addPersonaMetadataFilterClauses(String asset, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, asset))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset))); - } - - private void addPersonaGlossaryFilterClauses(String asset, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, asset))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + asset))); - } - - private void addPersonaMetadataFilterConnectionClause(String connection, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, connection))); - } - private void addPurposeMetadataFilterClauses(List tags, List> clauseList) { clauseList.add(mapOf("terms", mapOf(TRAIT_NAMES_PROPERTY_KEY, tags))); clauseList.add(mapOf("terms", mapOf(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, tags))); From c07cb54332098be15114be899c5aa72578673984 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Fri, 3 Nov 2023 12:36:59 +0530 Subject: [PATCH 2/6] Remove redundant logging --- .../java/org/apache/atlas/plugin/util/KeycloakUserStore.java | 2 -- .../atlas/policytransformer/CachePolicyTransformerImpl.java | 4 +--- .../apache/atlas/repository/audit/ESBasedAuditRepository.java | 1 - .../main/java/org/apache/atlas/tasks/TaskQueueWatcher.java | 3 +-- webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java | 1 - 5 files changed, 2 insertions(+), 9 deletions(-) diff --git a/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java b/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java index 06fd614673..178b4f88d3 100644 --- a/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java +++ b/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java @@ -151,7 +151,6 @@ public RangerRoles loadRolesIfUpdated(long lastUpdatedTime) throws AtlasBaseExce boolean isKeycloakUpdated = isKeycloakSubjectsStoreUpdated(lastUpdatedTime); if (!isKeycloakUpdated) { - LOG.info("loadRolesIfUpdated: Skipping as no update found"); return null; } @@ -268,7 +267,6 @@ public RangerUserStore loadUserStoreIfUpdated(long lastUpdatedTime) throws Atlas boolean isKeycloakUpdated = isKeycloakSubjectsStoreUpdated(lastUpdatedTime); if (!isKeycloakUpdated) { - LOG.info("loadUserStoreIfUpdated: Skipping as no update found"); return null; } diff --git a/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java b/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java index 502412f804..4a81129a3c 100644 --- a/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java +++ b/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java @@ -188,9 +188,7 @@ public ServicePolicies getPolicies(String serviceName, String pluginId, Long las RequestContext.get().endMetricRecord(recorderFilterPolicies); - if (LOG.isDebugEnabled()) { - LOG.debug("Found {} policies", servicePolicies.getPolicies().size()); - } + LOG.info("Found {} policies", servicePolicies.getPolicies().size()); } } catch (Exception e) { diff --git a/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java b/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java index 3de1728a7a..cbab135606 100644 --- a/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java +++ b/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java @@ -206,7 +206,6 @@ public List listEventsV2(String entityId, EntityAuditEventV2 @Override public EntityAuditSearchResult searchEvents(String queryString) throws AtlasBaseException { - LOG.info("Hitting ES query to fetch audits: {}", queryString); try { String response = performSearchOnIndex(queryString); return getResultFromResponse(response); diff --git a/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java b/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java index 1d8762d328..25478e9a5a 100644 --- a/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java +++ b/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java @@ -98,9 +98,9 @@ public void run() { if (CollectionUtils.isNotEmpty(tasks)) { final CountDownLatch latch = new CountDownLatch(tasks.size()); submitAll(tasks, latch); + LOG.info("Submitted {} tasks to the queue", tasks.size()); waitForTasksToComplete(latch); } else { - LOG.info("No tasks to queue, sleeping for {} ms", pollInterval); redisService.releaseDistributedLock(ATLAS_TASK_LOCK); } Thread.sleep(pollInterval); @@ -153,7 +153,6 @@ public void run() { if (LOG.isDebugEnabled()) { LOG.debug("TasksFetcher: Fetching tasks for queuing"); } - LOG.info("TasksFetcher: Fetching tasks for queuing"); this.tasks = registry.getTasksForReQueue(); } diff --git a/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java b/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java index ac23824c3c..c5868cbfa3 100644 --- a/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java +++ b/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java @@ -209,7 +209,6 @@ private boolean isPolicyUpdated(String serviceName, long lastUpdatedTime) { EntityAuditSearchResult result = auditRepository.searchEvents(parameters.getQueryString()); if (result == null || CollectionUtils.isEmpty(result.getEntityAudits())) { - LOG.info("getPoliciesIfUpdated: Skipping as no update found"); return false; } } catch (AtlasBaseException e) { From 0f1e3aca74f7a61cda3818522671afccd40b9df9 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Wed, 8 Nov 2023 15:45:33 +0530 Subject: [PATCH 3/6] PLT-302 Limit Persona policies assets --- .../main/java/org/apache/atlas/AtlasConfiguration.java | 4 +++- .../src/main/java/org/apache/atlas/AtlasErrorCode.java | 4 +++- .../repository/store/aliasstore/ESAliasStore.java | 10 +++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java index 5cd42da2e3..7a0b2c835f 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java +++ b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java @@ -104,7 +104,9 @@ public enum AtlasConfiguration { INDEX_CLIENT_CONNECTION_TIMEOUT("atlas.index.client.connection.timeout.ms", 900000), INDEX_CLIENT_SOCKET_TIMEOUT("atlas.index.client.socket.timeout.ms", 900000), ENABLE_SEARCH_LOGGER("atlas.enable.search.logger", true), - SEARCH_LOGGER_MAX_THREADS("atlas.enable.search.logger.max.threads", 20); + SEARCH_LOGGER_MAX_THREADS("atlas.enable.search.logger.max.threads", 20), + + PERSONA_POLICY_ASSET_MAX_LIMIT("atlas.persona.policy.asset.maxlimit", 1000); private static final Configuration APPLICATION_PROPERTIES; diff --git a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java index 11120c466b..04d72bdfb2 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java +++ b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java @@ -282,7 +282,9 @@ public enum AtlasErrorCode { JSON_ERROR(400, "ATLAS-400-00-109", "Error occurred putting object into JSONObject: {0}"), DISABLED_OPERATION(400, "ATLAS-400-00-110", "This operation is temporarily disabled as it is under maintenance."), TASK_INVALID_PARAMETERS(400, "ATLAS-400-00-111", "Invalid parameters for task {0}"), - TASK_TYPE_NOT_SUPPORTED(400, "ATLAS-400-00-112", "Task type {0} is not supported"); + TASK_TYPE_NOT_SUPPORTED(400, "ATLAS-400-00-112", "Task type {0} is not supported"), + + PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED(400, "ATLAS-400-00-113", "Exceeded limit of maximum allowed assets across policies for a Persona: Limit: {0}, assets: {1}"); private String errorCode; diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 67dde2e170..cd595998ee 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -17,6 +17,8 @@ */ package org.apache.atlas.repository.store.aliasstore; +import org.apache.atlas.AtlasConfiguration; +import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.ESAliasRequestBuilder; import org.apache.atlas.ESAliasRequestBuilder.AliasAction; import org.apache.atlas.exception.AtlasBaseException; @@ -69,6 +71,8 @@ public class ESAliasStore implements IndexAliasStore { private final AtlasGraph graph; private final EntityGraphRetriever entityRetriever; + private final int assetsMaxLimit = AtlasConfiguration.PERSONA_POLICY_ASSET_MAX_LIMIT.getInt(); + @Inject public ESAliasStore(AtlasGraph graph, EntityGraphRetriever entityRetriever) { @@ -166,7 +170,7 @@ private Map getFilterForPurpose(AtlasEntity purpose) throws Atla private void personaPolicyToESDslClauses(List policies, List> allowClauseList) throws AtlasBaseException { - Set terms = new HashSet<>(); + List terms = new ArrayList<>(); for (AtlasEntity policy: policies) { if (policy.getStatus() == null || AtlasEntity.Status.ACTIVE.equals(policy.getStatus())) { @@ -196,6 +200,10 @@ private void personaPolicyToESDslClauses(List policies, } } + if (terms.size() > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } From 8da1cd4e8726a88d839ff5d129db92151bb59cf4 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:28:55 +0530 Subject: [PATCH 4/6] PLT-302 Address review comment --- .../repository/store/aliasstore/ESAliasStore.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index cd595998ee..87e59174ec 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -178,6 +178,11 @@ private void personaPolicyToESDslClauses(List policies, if (getIsAllowPolicy(policy)) { if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { + + if (terms.size() + assets.size() + 1 > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + String connectionQName = getPolicyConnectionQN(policy); if (StringUtils.isEmpty(connectionQName)) { connectionQName = getConnectionQualifiedNameFromPolicyAssets(entityRetriever, assets); @@ -191,6 +196,10 @@ private void personaPolicyToESDslClauses(List policies, terms.add(connectionQName); } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { + if (terms.size() + assets.size() > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + for (String glossaryQName : assets) { terms.add(glossaryQName); allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); @@ -200,10 +209,6 @@ private void personaPolicyToESDslClauses(List policies, } } - if (terms.size() > assetsMaxLimit) { - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); - } - allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } From 08be3166d542ed660e8d7d0c2f6a2b809532b3c4 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:52:23 +0530 Subject: [PATCH 5/6] PLT-302 Address review comment --- .../apache/atlas/repository/store/aliasstore/ESAliasStore.java | 1 + 1 file changed, 1 insertion(+) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 87e59174ec..955f801b75 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -180,6 +180,7 @@ private void personaPolicyToESDslClauses(List policies, if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { if (terms.size() + assets.size() + 1 > assetsMaxLimit) { + // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence omparing with "assets.size() + 1" throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); } From 4d31bf51220f17217f9dbdc63c9bd93cfbeec86f Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:55:16 +0530 Subject: [PATCH 6/6] PLT-302 Address review comment --- .../apache/atlas/repository/store/aliasstore/ESAliasStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 955f801b75..e6ff8f69d3 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -180,7 +180,7 @@ private void personaPolicyToESDslClauses(List policies, if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { if (terms.size() + assets.size() + 1 > assetsMaxLimit) { - // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence omparing with "assets.size() + 1" + // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); }