Skip to content

Commit

Permalink
Throw error on exceeding tags limit (#1389)
Browse files Browse the repository at this point in the history
* Throw error on exceeding tags limit

Signed-off-by: Dvir Guttman <[email protected]>

* Fix PR comments
Signed-off-by: Dvir Guttman <[email protected]>

Signed-off-by: Dvir Guttman <[email protected]>

Co-authored-by: Dvir Guttman <[email protected]>
  • Loading branch information
dvirguttman and Dvir Guttman authored Mar 24, 2021
1 parent 87015b7 commit 1921261
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1081,11 +1081,15 @@ public boolean insertDomainTags(String domainName, Map<String, StringList> tags)
throw notFoundError(caller, ZMSConsts.OBJECT_DOMAIN, domainName);
}
int curTagCount = getDomainTagsCount(domainId);
int remainingTagsToInsert = domainTagsLimit - curTagCount;
int newTagCount = calculateTagCount(tags);
if (curTagCount + newTagCount > domainTagsLimit) {
throw requestError(caller, "domain tag quota exceeded - limit: "
+ domainTagsLimit + ", current tags count: " + curTagCount + ", new tags count: " + newTagCount);
}

boolean res = true;
for (Map.Entry<String, StringList> e : tags.entrySet()) {
for (int i = 0; i < e.getValue().getList().size() && remainingTagsToInsert-- > 0; i++) {
String tagValue = e.getValue().getList().get(i);
for (String tagValue : e.getValue().getList()) {
try (PreparedStatement ps = con.prepareStatement(SQL_INSERT_DOMAIN_TAG)) {
ps.setInt(1, domainId);
ps.setString(2, processInsertValue(e.getKey()));
Expand All @@ -1096,12 +1100,18 @@ public boolean insertDomainTags(String domainName, Map<String, StringList> tags)
}
}
}
if (remainingTagsToInsert < 0) {
LOG.info("Domain tags limit for domain: [{}] has reached", domainName);
}

return res;
}

private int calculateTagCount(Map<String, StringList> tags) {
int count = 0;
for (Map.Entry<String, StringList> e : tags.entrySet()) {
count += e.getValue().getList().size();
}
return count;
}

private int getDomainTagsCount(int domainId) {
final String caller = "getDomainTagsCount";
int count = 0;
Expand Down Expand Up @@ -6015,12 +6025,15 @@ public boolean insertRoleTags(String roleName, String domainName, Map<String, St
throw notFoundError(caller, ZMSConsts.OBJECT_ROLE, ResourceUtils.roleResourceName(domainName, roleName));
}
int curTagCount = getRoleTagsCount(roleId);

int remainingTagsToInsert = roleTagsLimit - curTagCount;
int newTagCount = calculateTagCount(roleTags);
if (curTagCount + newTagCount > roleTagsLimit) {
throw requestError(caller, "role tag quota exceeded - limit: "
+ roleTagsLimit + ", current tags count: " + curTagCount + ", new tags count: " + newTagCount);
}

boolean res = true;
for (Map.Entry<String, StringList> e : roleTags.entrySet()) {
for (int i = 0; i < e.getValue().getList().size() && remainingTagsToInsert-- > 0; i++) {
String tagValue = e.getValue().getList().get(i);
for (String tagValue : e.getValue().getList()) {
try (PreparedStatement ps = con.prepareStatement(SQL_INSERT_ROLE_TAG)) {
ps.setInt(1, roleId);
ps.setString(2, processInsertValue(e.getKey()));
Expand All @@ -6031,9 +6044,6 @@ public boolean insertRoleTags(String roleName, String domainName, Map<String, St
}
}
}
if (remainingTagsToInsert < 0) {
LOG.info("Role tags limit for role: [{}], domain: [{}] has reached", roleName, domainName);
}
return res;
}

Expand Down
93 changes: 28 additions & 65 deletions servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25193,40 +25193,22 @@ public void testRoleTagsLimit() throws Exception {
List<String> tagValues = Arrays.asList("val1", "val2", "val3", "val4");
Role role = createRoleObject(domainName, roleName, null);
role.setTags(Collections.singletonMap(tagKey, new StringList().setList(tagValues)));
zmsTest.putRole(mockDomRsrcCtx, domainName, roleName, auditRef, role);
Role dbRole = zmsTest.getRole(mockDomRsrcCtx, domainName, roleName, false, false, false);

// expect for only 3 first tags to be presented
assertEquals(dbRole.getTags().get(tagKey).getList().size(), 3);
assertTrue(dbRole.getTags().get(tagKey).getList().containsAll(
Arrays.asList("val1", "val2", "val3")));

// new tags map - 2 key with 2 values
Map<String, StringList> tags = new HashMap<>();
tags.put("newKey", new StringList().setList(Arrays.asList("newVal1", "newVal2")));
tags.put("newKey2", new StringList().setList(Arrays.asList("newVal3", "newVal4")));
RoleMeta rm = new RoleMeta().setTags(tags);
try {
zmsTest.putRole(mockDomRsrcCtx, domainName, roleName, auditRef, role);
fail();
} catch(ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
assertTrue(ex.getMessage().contains("role tag quota exceeded - limit: 3, current tags count: 0, new tags count: 4"));
}

// update role tags using role meta
zmsTest.putRoleMeta(mockDomRsrcCtx, domainName, roleName, auditRef, rm);
dbRole = zmsTest.getRole(mockDomRsrcCtx, domainName, roleName, false, false, false);

// processing order of Map is not guarantee
// expect for 2 tags from one key, and only one from the other..
if (dbRole.getTags().get("newKey").getList().size() == 2) {
assertTrue(dbRole.getTags().get("newKey").getList().containsAll(
Arrays.asList("newVal1", "newVal2")));

assertEquals(dbRole.getTags().get("newKey2").getList().size(), 1);
assertTrue(dbRole.getTags().get("newKey2").getList().contains("newVal3"));
} else {
assertEquals(dbRole.getTags().get("newKey").getList().size(), 1);
assertTrue(dbRole.getTags().get("newKey").getList().contains("newVal1"));

assertEquals(dbRole.getTags().get("newKey2").getList().size(), 2);
assertTrue(dbRole.getTags().get("newKey2").getList().containsAll(
Arrays.asList("newVal3", "newVal4")));
try {
// role should not be created if fails to process tags..
zmsTest.getRole(mockDomRsrcCtx, domainName, roleName, false, false, false);
fail();
} catch(ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.NOT_FOUND);
}

System.clearProperty(ZMSConsts.ZMS_PROP_QUOTA_ROLE_TAG);
}

Expand Down Expand Up @@ -25406,40 +25388,21 @@ public void testDomainTagsLimit() throws Exception {

TopLevelDomain topLevelDomain = createTopLevelDomainObject(domainName, "Test Domain With Tag Limit", "testOrg", adminUser);
topLevelDomain.setTags(Collections.singletonMap("tag-key", new StringList().setList(Arrays.asList("val1", "val2", "val3", "val4"))));
zmsTest.postTopLevelDomain(mockDomRsrcCtx, auditRef, topLevelDomain);
Domain domain = zmsTest.getDomain(mockDomRsrcCtx, domainName);

// expect for only 3 first tags to be presented
assertEquals(domain.getTags().get("tag-key").getList().size(), 3);
assertTrue(domain.getTags().get("tag-key").getList().containsAll(
Arrays.asList("val1", "val2", "val3")));

// new tags map - 2 key with 2 values
Map<String, StringList> tags = new HashMap<>();
tags.put("newKey", new StringList().setList(Arrays.asList("newVal1", "newVal2")));
tags.put("newKey2", new StringList().setList(Arrays.asList("newVal3", "newVal4")));
DomainMeta dm = new DomainMeta().setTags(tags);

// update domain tags using domain meta
zmsTest.putDomainMeta(mockDomRsrcCtx, domainName, auditRef, dm);
domain = zmsTest.getDomain(mockDomRsrcCtx, domainName);

// processing order of Map is not guarantee
// expect for 2 tags from one key, and only one from the other..
if (domain.getTags().get("newKey").getList().size() == 2) {
assertTrue(domain.getTags().get("newKey").getList().containsAll(
Arrays.asList("newVal1", "newVal2")));

assertEquals(domain.getTags().get("newKey2").getList().size(), 1);
assertTrue(domain.getTags().get("newKey2").getList().contains("newVal3"));
} else {
assertEquals(domain.getTags().get("newKey").getList().size(), 1);
assertTrue(domain.getTags().get("newKey").getList().contains("newVal1"));

assertEquals(domain.getTags().get("newKey2").getList().size(), 2);
assertTrue(domain.getTags().get("newKey2").getList().containsAll(
Arrays.asList("newVal3", "newVal4")));
try {
zmsTest.postTopLevelDomain(mockDomRsrcCtx, auditRef, topLevelDomain);
fail();
} catch(ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
assertTrue(ex.getMessage().contains("domain tag quota exceeded - limit: 3, current tags count: 0, new tags count: 4"));
}
try {
// domain should not be created if fails to process tags..
zmsTest.getDomain(mockDomRsrcCtx, domainName);
fail();
} catch(ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.NOT_FOUND);
}

System.clearProperty(ZMSConsts.ZMS_PROP_QUOTA_DOMAIN_TAG);
}

Expand Down

0 comments on commit 1921261

Please sign in to comment.