Merge pull request #2402 from benjaminapetersen/issue/16862/origin/ro…

…le-whitelist

Automatic merge from submit-queue.

Update membership filter to use MEMBERSHIP_WHITELIST in Constants.js

Moving back to using a simple whitelist via [origin issue 16862](openshift/origin#16862)

fixes [origin issue 16862](openshift/origin#16862)

History:
- issue #14411 
- PR #14510, 
- PR #15241
- [PR 11328](openshift/origin#11328) (original)

At this point ignoring the annotation `systemOnly` entirely.

@jwforres @spadgett @enj

app/scripts/constants.js

@@ -150,6 +150,16 @@ angular.extend(window.OPENSHIFT_CONSTANTS, {
     {resource: 'services', group: ''},
     {resource: 'statefulsets', group: 'apps'}
   ],
+  MEMBERSHIP_WHITELIST: [
+    "admin",
+    "basic-user",
+    "edit",
+    "system:deployer",
+    "system:image-builder",
+    "system:image-puller",
+    "system:image-pusher",
+    "view",
+  ],
   // TODO:
   // This map can drive both the drawer & toast messages by
   // updating it to the following format:

app/scripts/services/membership/membership.js

@@ -2,7 +2,7 @@
 
 angular
   .module('openshiftConsole')
-  .factory('MembershipService', function($filter) {
+  .factory('MembershipService', function($filter, Constants) {
 
     var annotation = $filter('annotation');
 
@@ -120,8 +120,7 @@ angular
 
     var filterRoles = function(roles) {
       return _.filter(roles, function(item) {
-        // system-only must be explicitly <string> 'true' to hide the annotation
-        return annotation(item, 'systemOnly') !== 'true';
+        return _.includes(Constants.MEMBERSHIP_WHITELIST, item.metadata.name);
       });
     };
 

dist/scripts/scripts.js

@@ -712,6 +712,7 @@ group: ""
 resource: "statefulsets",
 group: "apps"
 } ],
+MEMBERSHIP_WHITELIST: [ "admin", "basic-user", "edit", "system:deployer", "system:image-builder", "system:image-puller", "system:image-pusher", "view" ],
 EVENTS_TO_SHOW: {
 FailedCreate: !0,
 FailedDelete: !0,
@@ -2343,8 +2344,9 @@ t[e.tag] = t[e.tag] || {}, t[e.tag].name = e.tag, t[e.tag].status = angular.copy
 }), t;
 }
 };
-}), angular.module("openshiftConsole").factory("MembershipService", [ "$filter", function(e) {
-var t = e("annotation"), n = function() {
+}), angular.module("openshiftConsole").factory("MembershipService", [ "$filter", "Constants", function(e, t) {
+e("annotation");
+var n = function() {
 return _.reduce(_.slice(arguments), function(e, t, n) {
 return t ? _.isEqual(n, 0) ? t : e + "-" + t : e;
 }, "");
@@ -2398,7 +2400,7 @@ return _.sortBy(e, "metadata.name");
 },
 filterRoles: function(e) {
 return _.filter(e, function(e) {
-return "true" !== t(e, "systemOnly");
+return _.includes(t.MEMBERSHIP_WHITELIST, e.metadata.name);
 });
 },
 mapRolesForUI: function(e, t) {

test/spec/services/membership/membershipSpec.js

@@ -84,28 +84,21 @@ describe('MembershipService', function() {
   });
 
   describe('#filterRoles', function() {
-    it('should filter out system-only roles', function() {
+    // constants.js window.OPENSHIFT_CONSTANTS.MEMBERSHIP_WHITELIST
+    it('should filter out roles that do not exist in MEMBERSHIP_WHITELIST', function() {
       var fakeList = [
-        // the string 'true' is the only acceptable value for 'authorization.openshift.io/system-only'
-        {metadata: {name: 'system-only-role', annotations: {'authorization.openshift.io/system-only': 'true'}}},
-        // the rest of these will not be filtered
-        {metadata: {name: 'system-only-role2', annotations: {'authorization.openshift.io/system-only': 'false'}}},
-        {metadata: {name: 'system-only-role3', annotations: {'authorization.openshift.io/system-only': 'show'}}},
-        {metadata: {name: 'system-only-role4', annotations: {'authorization.openshift.io/system-only': ''}}},
-        {metadata: {name: 'system-only-role5', annotations: {'authorization.openshift.io/system-only': undefined}}},
-        {metadata: {name: 'system-only-role6', annotations: {'authorization.openshift.io/system-only': null}}},
-        {metadata : {name : 'not-system-only'}},
-        {metadata : {name : 'the-other-not-system-only' }}
+        {metadata: {name: 'admin'}},
+        {metadata: {name: 'basic-user'}},
+        {metadata: {name: 'edit'}},
+        {metadata: {name: 'not-an-admin'}},
+        {metadata: {name: 'not-a-basic-user'}},
+        {metadata: {name: 'system-only-thing-that-does-secret-stuff'}}
       ];
 
       expect(MembershipService.filterRoles(fakeList)).toEqual([
-        {metadata: {name: 'system-only-role2', annotations: {'authorization.openshift.io/system-only': 'false'}}},
-        {metadata: {name: 'system-only-role3', annotations: {'authorization.openshift.io/system-only': 'show'}}},
-        {metadata: {name: 'system-only-role4', annotations: {'authorization.openshift.io/system-only': ''}}},
-        {metadata: {name: 'system-only-role5', annotations: {'authorization.openshift.io/system-only': undefined}}},
-        {metadata: {name: 'system-only-role6', annotations: {'authorization.openshift.io/system-only': null}}},
-        {metadata : {name : 'not-system-only'}},
-        {metadata : {name : 'the-other-not-system-only' }}
+          {metadata: {name: 'admin'}},
+          {metadata: {name: 'basic-user'}},
+          {metadata: {name: 'edit'}}
       ]);
     });
   });