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
spinnaker · Oct 24, 2019 · 6460ffd · 6460ffd
1 parent 05f7ce3
commit 6460ffd
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 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
@@ -104,18 +104,17 @@ public static class Builder extends LinkedHashMap<Authorization, List<String>> {
     private static Permissions fromMap(Map<Authorization, List<String>> authConfig) {
       final Map<Authorization, List<String>> 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
@@ -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/.../test/groovy/com/netflix/spinnaker/fiat/permissions/RedisPermissionsRepositorySpec.groovy b/.../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