From 6460ffda0362868bf71593dda1569bc17e1aed6d Mon Sep 17 00:00:00 2001 From: Cameron Fieber Date: Thu, 24 Oct 2019 15:12:22 -0700 Subject: [PATCH] fix(api): fix serialization issues with new Authorization enum values. (#491) This reverts a well intended change where the resulting Permissions object always had an entry for each Authorization type. This change makes rolling out new values in this enum painful to coordinate across services. This reverts the behaviour to now only store an explicit entry in the Map if there are specifically roles defined for that Authorization type --- .../fiat/model/resources/Permissions.java | 23 +++++++++---------- .../model/resources/PermissionsSpec.groovy | 4 +--- .../RedisPermissionsRepositorySpec.groovy | 2 +- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/resources/Permissions.java b/fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/resources/Permissions.java index 1a4ea54f3..2be43a164 100644 --- a/fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/resources/Permissions.java +++ b/fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/resources/Permissions.java @@ -104,18 +104,17 @@ public static class Builder extends LinkedHashMap> { private static Permissions fromMap(Map> authConfig) { final Map> perms = new EnumMap<>(Authorization.class); for (Authorization auth : Authorization.values()) { - perms.put( - auth, - Optional.ofNullable(authConfig.get(auth)) - .map( - groups -> - groups.stream() - .map(String::trim) - .filter(s -> !s.isEmpty()) - .map(String::toLowerCase) - .collect(Collectors.toList())) - .map(Collections::unmodifiableList) - .orElse(Collections.emptyList())); + Optional.ofNullable(authConfig.get(auth)) + .map( + groups -> + groups.stream() + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(String::toLowerCase) + .collect(Collectors.toList())) + .filter(g -> !g.isEmpty()) + .map(Collections::unmodifiableList) + .ifPresent(roles -> perms.put(auth, roles)); } return new Permissions(perms); } diff --git a/fiat-core/src/test/groovy/com/netflix/spinnaker/fiat/model/resources/PermissionsSpec.groovy b/fiat-core/src/test/groovy/com/netflix/spinnaker/fiat/model/resources/PermissionsSpec.groovy index 522431049..2590d98e3 100644 --- a/fiat-core/src/test/groovy/com/netflix/spinnaker/fiat/model/resources/PermissionsSpec.groovy +++ b/fiat-core/src/test/groovy/com/netflix/spinnaker/fiat/model/resources/PermissionsSpec.groovy @@ -52,9 +52,7 @@ class PermissionsSpec extends Specification { String permissionSerialized = '''\ { "READ" : [ "foo" ], - "WRITE" : [ "bar" ], - "EXECUTE" : [ ], - "CREATE" : [ ] + "WRITE" : [ "bar" ] }'''.stripIndent() def "should deserialize"() { diff --git a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy index bed71424a..79818b694 100644 --- a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy +++ b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy @@ -37,7 +37,7 @@ import spock.lang.Subject class RedisPermissionsRepositorySpec extends Specification { - private static final String EMPTY_PERM_JSON = "{${Authorization.values().collect {/"$it":[]/}.join(',')}}" + private static final String EMPTY_PERM_JSON = "{}" private static final String UNRESTRICTED = UnrestrictedResourceConfig.UNRESTRICTED_USERNAME