This is an automated email from the ASF dual-hosted git repository. billyliu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 4de8bfc92814410155fa42b9a030bb86986e9652 Author: Jiatao Tao <245915...@qq.com> AuthorDate: Thu Feb 1 11:14:03 2018 +0800 KYLIN-3220, add manager for project ACL. KYLIN-3220, add manager for project ACL. code review minor changes --- .../kylin/common/persistence/ResourceStore.java | 1 + .../apache/kylin/metadata/acl/TableACLManager.java | 2 +- .../kylin/metadata/cachesync/CachedCrudAssist.java | 6 +- .../kylin/rest/security/springacl/AclRecord.java | 5 + .../apache/kylin/rest/service/AccessService.java | 2 - .../org/apache/kylin/rest/service/AclService.java | 181 ++++++++++++++------- .../kylin/rest/service/KylinUserService.java | 19 +++ .../org/apache/kylin/rest/service/UserService.java | 2 + .../apache/kylin/rest/service/AclServiceTest.java | 2 +- .../apache/kylin/rest/service/UserServiceTest.java | 7 + 10 files changed, 161 insertions(+), 66 deletions(-) diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java index a6b3337..2bccd67 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java @@ -76,6 +76,7 @@ abstract public class ResourceStore { public static final String CUBE_STATISTICS_ROOT = "/cube_statistics"; public static final String BAD_QUERY_RESOURCE_ROOT = "/bad_query"; public static final String DRAFT_RESOURCE_ROOT = "/draft"; + public static final String USER_ROOT = "/user"; public static final String METASTORE_UUID_TAG = "/UUID"; diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java index 163d340..9f74dec 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java @@ -58,7 +58,7 @@ public class TableACLManager { logger.info("Initializing TableACLManager with config " + config); this.config = config; this.tableACLMap = new CaseInsensitiveStringCache<>(config, "table_acl"); - this.crud = new CachedCrudAssist<TableACL>(getStore(), "/table_acl", "", TableACL.class, tableACLMap) { + this.crud = new CachedCrudAssist<TableACL>(getStore(), "/table_acl", "", TableACL.class, tableACLMap, true) { @Override protected TableACL initEntityAfterReload(TableACL acl, String resourceName) { acl.init(resourceName); diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java b/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java index b3c200e..be3d8d4 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java @@ -51,16 +51,16 @@ abstract public class CachedCrudAssist<T extends RootPersistentEntity> { public CachedCrudAssist(ResourceStore store, String resourceRootPath, Class<T> entityType, SingleValueCache<String, T> cache) { - this(store, resourceRootPath, MetadataConstants.FILE_SURFIX, entityType, cache); + this(store, resourceRootPath, MetadataConstants.FILE_SURFIX, entityType, cache, false); } public CachedCrudAssist(ResourceStore store, String resourceRootPath, String resourcePathSuffix, - Class<T> entityType, SingleValueCache<String, T> cache) { + Class<T> entityType, SingleValueCache<String, T> cache, boolean compact) { this.store = store; this.entityType = entityType; this.resRootPath = resourceRootPath; this.resPathSuffix = resourcePathSuffix; - this.serializer = new JsonSerializer<T>(entityType); + this.serializer = new JsonSerializer<T>(entityType, compact); this.cache = cache; this.checkCopyOnWrite = store.getConfig().isCheckCopyOnWrite(); diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java index 3fff632..eb5b792 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java +++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java @@ -105,6 +105,11 @@ public class AclRecord extends RootPersistentEntity implements Acl, OwnershipAcl } } + @Override + public String resourceName() { + return String.valueOf(domainObjectInfo.getIdentifier()); + } + public SidInfo getOwnerInfo() { return ownerInfo; } diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java index 74a87c8..09a89c8 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java @@ -66,8 +66,6 @@ import org.springframework.transaction.annotation.Transactional; import com.google.common.base.Preconditions; -/** - */ @Component("accessService") public class AccessService { @SuppressWarnings("unused") diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java index 73a6fb2..adc7c30 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java @@ -21,14 +21,22 @@ package org.apache.kylin.rest.service; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.annotation.Nullable; + import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.persistence.JsonSerializer; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.Serializer; +import org.apache.kylin.common.util.AutoReadWriteLock; +import org.apache.kylin.common.util.AutoReadWriteLock.AutoLock; +import org.apache.kylin.metadata.cachesync.Broadcaster; +import org.apache.kylin.metadata.cachesync.CachedCrudAssist; +import org.apache.kylin.metadata.cachesync.CaseInsensitiveStringCache; import org.apache.kylin.rest.exception.BadRequestException; import org.apache.kylin.rest.exception.InternalErrorException; import org.apache.kylin.rest.msg.Message; @@ -38,6 +46,7 @@ import org.apache.kylin.rest.security.springacl.MutableAclRecord; import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.acls.domain.PermissionFactory; import org.springframework.security.acls.domain.PrincipalSid; @@ -51,12 +60,11 @@ import org.springframework.security.acls.model.ObjectIdentity; import org.springframework.security.acls.model.Permission; import org.springframework.security.acls.model.PermissionGrantingStrategy; import org.springframework.security.acls.model.Sid; -import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; @Component("aclService") -public class AclService implements MutableAclService { +public class AclService implements MutableAclService, InitializingBean { private static final Logger logger = LoggerFactory.getLogger(AclService.class); public static final String DIR_PREFIX = "/acl/"; @@ -69,35 +77,67 @@ public class AclService implements MutableAclService { @Autowired protected PermissionFactory aclPermissionFactory; + // cache + private CaseInsensitiveStringCache<AclRecord> aclMap; + private CachedCrudAssist<AclRecord> crud; + private AutoReadWriteLock lock = new AutoReadWriteLock(); - // @Autowired - // protected AclAuthorizationStrategy aclAuthorizationStrategy; + public AclService() throws IOException { + KylinConfig config = KylinConfig.getInstanceFromEnv(); + ResourceStore aclStore = ResourceStore.getStore(config); + this.aclMap = new CaseInsensitiveStringCache<>(config, "acl"); + this.crud = new CachedCrudAssist<AclRecord>(aclStore, "/acl", "", AclRecord.class, aclMap, true) { + @Override + protected AclRecord initEntityAfterReload(AclRecord acl, String resourceName) { + acl.init(null, aclPermissionFactory, permissionGrantingStrategy); + return acl; + } + }; + crud.reloadAll(); + } - // @Autowired - // protected AuditLogger auditLogger; + @Override + public void afterPropertiesSet() throws Exception { + Broadcaster.getInstance(KylinConfig.getInstanceFromEnv()).registerStaticListener(new AclRecordSyncListener(), "acl"); + } - protected ResourceStore aclStore; + private class AclRecordSyncListener extends Broadcaster.Listener { - public AclService() throws IOException { - aclStore = ResourceStore.getStore(KylinConfig.getInstanceFromEnv()); + @Override + public void onEntityChange(Broadcaster broadcaster, String entity, Broadcaster.Event event, String cacheKey) + throws IOException { + try (AutoLock l = lock.lockForWrite()) { + if (event == Broadcaster.Event.DROP) + aclMap.removeLocal(cacheKey); + else + crud.reloadQuietly(cacheKey); + } + broadcaster.notifyProjectACLUpdate(cacheKey); + } + + @Override + public void onClearAll(Broadcaster broadcaster) throws IOException { + try (AutoLock l = lock.lockForWrite()) { + aclMap.clear(); + } + } } @Override public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) { - List<ObjectIdentity> oids = new ArrayList<ObjectIdentity>(); - try { - List<AclRecord> allAclRecords = aclStore.getAllResources(DIR_PREFIX, AclRecord.class, SERIALIZER); - for (AclRecord record : allAclRecords) { - ObjectIdentityImpl parent = record.getParentDomainObjectInfo(); - if (parent != null && parent.equals(parentIdentity)) { - ObjectIdentityImpl child = record.getDomainObjectInfo(); - oids.add(child); - } + List<ObjectIdentity> oids = new ArrayList<>(); + Collection<AclRecord> allAclRecords; + try (AutoLock l = lock.lockForRead()) { + allAclRecords = new ArrayList<>(aclMap.values()); + } + for (AclRecord record : allAclRecords) { + ObjectIdentityImpl parent = record.getParentDomainObjectInfo(); + if (parent != null && parent.equals(parentIdentity)) { + ObjectIdentityImpl child = record.getDomainObjectInfo(); + oids.add(child); } - return oids; - } catch (IOException e) { - throw new InternalErrorException(e); } + return oids; } public MutableAclRecord readAcl(ObjectIdentity oid) throws NotFoundException { @@ -128,40 +168,33 @@ public class AclService implements MutableAclService { @Override public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> oids, List<Sid> sids) throws NotFoundException { Map<ObjectIdentity, Acl> aclMaps = new HashMap<>(); - try { - for (ObjectIdentity oid : oids) { - AclRecord record = aclStore.getResource(resourceKey(oid), AclRecord.class, SERIALIZER); - if (record == null) { - Message msg = MsgPicker.getMsg(); - throw new NotFoundException(String.format(msg.getACL_INFO_NOT_FOUND(), oid)); - } + for (ObjectIdentity oid : oids) { + AclRecord record = getAclRecordByCache(objID(oid)); + if (record == null) { + Message msg = MsgPicker.getMsg(); + throw new NotFoundException(String.format(msg.getACL_INFO_NOT_FOUND(), oid)); + } - Acl parentAcl = null; - if (record.isEntriesInheriting() && record.getParentDomainObjectInfo() != null) - parentAcl = readAclById(record.getParentDomainObjectInfo()); + Acl parentAcl = null; + if (record.isEntriesInheriting() && record.getParentDomainObjectInfo() != null) + parentAcl = readAclById(record.getParentDomainObjectInfo()); - record.init(parentAcl, aclPermissionFactory, permissionGrantingStrategy); + record.init(parentAcl, aclPermissionFactory, permissionGrantingStrategy); - aclMaps.put(oid, new MutableAclRecord(record)); - } - return aclMaps; - } catch (IOException e) { - throw new InternalErrorException(e); + aclMaps.put(oid, new MutableAclRecord(record)); } + return aclMaps; } @Override public MutableAcl createAcl(ObjectIdentity objectIdentity) throws AlreadyExistsException { - try { - if (aclStore.exists(resourceKey(objectIdentity))) { + try (AutoLock l = lock.lockForWrite()) { + AclRecord aclRecord = getAclRecordByCache(objID(objectIdentity)); + if (aclRecord != null) { throw new AlreadyExistsException("ACL of " + objectIdentity + " exists!"); } - - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - PrincipalSid owner = new PrincipalSid(auth); - AclRecord record = new AclRecord(objectIdentity, owner); - record.init(null, aclPermissionFactory, permissionGrantingStrategy); - aclStore.putResource(resourceKey(objectIdentity), record, 0, SERIALIZER); + AclRecord record = newPrjACL(objectIdentity); + crud.save(record); logger.debug("ACL of " + objectIdentity + " created successfully."); } catch (IOException e) { throw new InternalErrorException(e); @@ -171,7 +204,7 @@ public class AclService implements MutableAclService { @Override public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren) throws ChildrenExistException { - try { + try (AutoLock l = lock.lockForWrite()) { List<ObjectIdentity> children = findChildren(objectIdentity); if (!deleteChildren && children.size() > 0) { Message msg = MsgPicker.getMsg(); @@ -180,7 +213,7 @@ public class AclService implements MutableAclService { for (ObjectIdentity oid : children) { deleteAcl(oid, deleteChildren); } - aclStore.deleteResource(resourceKey(String.valueOf(objectIdentity.getIdentifier()))); + crud.delete(objID(objectIdentity)); logger.debug("ACL of " + objectIdentity + " deleted successfully."); } catch (IOException e) { throw new InternalErrorException(e); @@ -190,10 +223,9 @@ public class AclService implements MutableAclService { // Try use the updateAclWithRetry() method family whenever possible @Override public MutableAcl updateAcl(MutableAcl mutableAcl) throws NotFoundException { - try { + try (AutoLock l = lock.lockForWrite()) { AclRecord record = ((MutableAclRecord) mutableAcl).getAclRecord(); - String resPath = resourceKey(mutableAcl.getObjectIdentity()); - aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER); + crud.save(record); logger.debug("ACL of " + mutableAcl.getObjectIdentity() + " updated successfully."); } catch (IOException e) { throw new InternalErrorException(e); @@ -202,7 +234,7 @@ public class AclService implements MutableAclService { } // a NULL permission means to delete the ace - public MutableAclRecord upsertAce(MutableAclRecord acl, final Sid sid, final Permission perm) { + MutableAclRecord upsertAce(MutableAclRecord acl, final Sid sid, final Permission perm) { return updateAclWithRetry(acl, new AclRecordUpdater() { @Override public void update(AclRecord record) { @@ -210,8 +242,8 @@ public class AclService implements MutableAclService { } }); } - - public MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) { + + MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) { return updateAclWithRetry(acl, new AclRecordUpdater() { @Override public void update(AclRecord record) { @@ -221,6 +253,33 @@ public class AclService implements MutableAclService { }); } + @Nullable + private AclRecord getAclRecordByCache(String id) { + try (AutoLock l = lock.lockForRead()) { + if (aclMap.size() > 0) { + return aclMap.get(id); + } + } + + try (AutoLock l = lock.lockForWrite()) { + crud.reloadAll(); + return aclMap.get(id); + } catch (IOException e) { + throw new RuntimeException("Can not get ACL record from cache.", e); + } + } + + private AclRecord newPrjACL(ObjectIdentity objID) { + AclRecord acl = new AclRecord(objID, getCurrentSid()); + acl.init(null, this.aclPermissionFactory, this.permissionGrantingStrategy); + acl.updateRandomUuid(); + return acl; + } + + private Sid getCurrentSid() { + return new PrincipalSid(SecurityContextHolder.getContext().getAuthentication()); + } + public interface AclRecordUpdater { void update(AclRecord record); } @@ -231,9 +290,8 @@ public class AclService implements MutableAclService { AclRecord record = acl.getAclRecord(); updater.update(record); - String resPath = resourceKey(record.getObjectIdentity()); try { - aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER); + crud.save(record); return acl; // here we are done } catch (IllegalStateException ise) { @@ -242,7 +300,8 @@ public class AclService implements MutableAclService { throw ise; } - logger.warn("Write conflict to update ACL " + resPath + " retry remaining " + retry + ", will retry..."); + logger.warn("Write conflict to update ACL " + resourceKey(record.getObjectIdentity()) + + " retry remaining " + retry + ", will retry..."); acl = readAcl(acl.getObjectIdentity()); } catch (IOException e) { @@ -252,11 +311,15 @@ public class AclService implements MutableAclService { throw new RuntimeException("should not reach here"); } - public static String resourceKey(ObjectIdentity domainObjId) { - return resourceKey(String.valueOf(domainObjId.getIdentifier())); + private static String resourceKey(ObjectIdentity domainObjId) { + return resourceKey(objID(domainObjId)); + } + + private static String objID(ObjectIdentity domainObjId) { + return String.valueOf(domainObjId.getIdentifier()); } - public static String resourceKey(String domainObjId) { + static String resourceKey(String domainObjId) { return DIR_PREFIX + domainObjId; } } diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java index 7e8919c..c35d737 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; import javax.annotation.PostConstruct; import org.apache.kylin.common.KylinConfig; @@ -39,7 +40,9 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UsernameNotFoundException; +import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; public class KylinUserService implements UserService { @@ -146,6 +149,22 @@ public class KylinUserService implements UserService { } @Override + public List<String> listUsernames() throws IOException { + List<String> paths = new ArrayList<>(); + paths.addAll(aclStore.listResources(ResourceStore.USER_ROOT)); + List<String> users = Lists.transform(paths, new Function<String, String>() { + @Nullable + @Override + public String apply(@Nullable String input) { + String[] path = input.split("/"); + Preconditions.checkArgument(path.length == 3); + return path[2]; + } + }); + return users; + } + + @Override public List<String> listAdminUsers() throws IOException{ List<String> adminUsers = new ArrayList<>(); for (ManagedUser managedUser : listUsers()) { diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java index 21c4cf9..5b50456 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java @@ -32,6 +32,8 @@ public interface UserService extends UserDetailsManager { List<ManagedUser> listUsers() throws IOException; + List<String> listUsernames() throws IOException; + List<String> listAdminUsers() throws IOException; //For performance consideration, list all users may be incomplete(eg. not load user's authorities until authorities has benn used). diff --git a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java index ad0d524..875a2a1 100644 --- a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java +++ b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java @@ -109,7 +109,7 @@ public class AclServiceTest extends ServiceTestBase { // inherit parent childAcl = aclService.inherit(childAcl, parentAcl); Assert.assertEquals(parentOid, childAcl.getAclRecord().getParentDomainObjectInfo()); - Assert.assertEquals(null, childAclOutdated.getAclRecord().getParentDomainObjectInfo()); + Assert.assertEquals(parentOid, childAclOutdated.getAclRecord().getParentDomainObjectInfo()); // update permission on an outdated ACL, retry should keep things going PrincipalSid user1 = new PrincipalSid("user1"); diff --git a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java index 6304712..2b3e0f5 100644 --- a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java +++ b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import com.google.common.collect.Lists; import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.exception.InternalErrorException; import org.apache.kylin.rest.security.ManagedUser; @@ -63,6 +64,12 @@ public class UserServiceTest extends ServiceTestBase { } + @Test + public void testGetAllUserNames() throws IOException { + List<String> users = userService.listUsernames(); + List<String> expected = Lists.newArrayList("ADMIN", "ANALYST", "MODELER"); + Assert.assertEquals(expected, users); + } @Test public void testDeleteAdmin() throws IOException { -- To stop receiving notification emails like this one, please contact billy...@apache.org.