Add logic to dedupe kinds by ignoring "openshift" namespace in APIService.calculateAvailableKinds()

Author: benjaminapetersen

dist/origin-web-common-services.js

@@ -471,8 +471,10 @@ angular.module('openshiftCommonServices')
     var kinds = [];
     var rejectedKinds = Constants.AVAILABLE_KINDS_BLACKLIST;
 
-    // Legacy openshift and k8s kinds
-    _.each(API_CFG, function(api) {
+    // ignore the legacy openshift kinds, these have been migrated to api groups
+    _.each(_.pick(API_CFG, function(value, key) {
+      return key !== 'openshift';
+    }), function(api) {
       _.each(api.resources.v1, function(resource) {
         if (resource.namespaced || includeClusterScoped) {
           // Exclude subresources and any rejected kinds

dist/origin-web-common.js

@@ -1764,8 +1764,10 @@ angular.module('openshiftCommonServices')
     var kinds = [];
     var rejectedKinds = Constants.AVAILABLE_KINDS_BLACKLIST;
 
-    // Legacy openshift and k8s kinds
-    _.each(API_CFG, function(api) {
+    // ignore the legacy openshift kinds, these have been migrated to api groups
+    _.each(_.pick(API_CFG, function(value, key) {
+      return key !== 'openshift';
+    }), function(api) {
       _.each(api.resources.v1, function(resource) {
         if (resource.namespaced || includeClusterScoped) {
           // Exclude subresources and any rejected kinds

dist/origin-web-common.min.js

@@ -725,7 +725,9 @@ var kind = "<none>", version = "<none>";
 return apiObject && apiObject.kind && (kind = apiObject.kind), apiObject && apiObject.apiVersion && (version = apiObject.apiVersion), "The API version " + version + " for kind " + kind + " is not supported by this server";
 }, calculateAvailableKinds = function(includeClusterScoped) {
 var kinds = [], rejectedKinds = Constants.AVAILABLE_KINDS_BLACKLIST;
-return _.each(API_CFG, function(api) {
+return _.each(_.pick(API_CFG, function(value, key) {
+return "openshift" !== key;
+}), function(api) {
 _.each(api.resources.v1, function(resource) {
 if (resource.namespaced || includeClusterScoped) {
 if (resource.name.indexOf("/") >= 0 || _.contains(rejectedKinds, resource.kind)) return;

src/services/apiService.js

@@ -268,8 +268,10 @@ angular.module('openshiftCommonServices')
     var kinds = [];
     var rejectedKinds = Constants.AVAILABLE_KINDS_BLACKLIST;
 
-    // Legacy openshift and k8s kinds
-    _.each(API_CFG, function(api) {
+    // ignore the legacy openshift kinds, these have been migrated to api groups
+    _.each(_.pick(API_CFG, function(value, key) {
+      return key !== 'openshift';
+    }), function(api) {
       _.each(api.resources.v1, function(resource) {
         if (resource.namespaced || includeClusterScoped) {
           // Exclude subresources and any rejected kinds

test/.jshintrc

@@ -0,0 +1,54 @@
+{
+  "node": true,
+  "browser": true,
+  "esversion": 6,
+  "bitwise": true,
+  "camelcase": false,
+  "curly": true,
+  "eqeqeq": true,
+  "immed": true,
+  "indent": 2,
+  "latedef": true,
+  "newcap": false,
+  "noarg": true,
+  "quotmark": false,
+  "regexp": true,
+  "smarttabs": true,
+  "strict": true,
+  "sub" : true,
+  "undef": true,
+  "unused": true,
+  "expr" : true,
+  "globals": {
+    "ace": false,
+    "after": false,
+    "afterEach": false,
+    "afterAll": false,
+    "angular": false,
+    "by": false,
+    "before": false,
+    "beforeAll": false,
+    "beforeEach": false,
+    "browser": false,
+    "describe": false,
+    "element" : false,
+    "expect": false,
+    "inject": false,
+    "it": false,
+    "xit": false,
+    "jasmine": false,
+    "spyOn": false,
+    "hawtioPluginLoader": false,
+    "HawtioCore": false,
+    "Logger" : false,
+    "LabelSelector": false,
+    "moment": false,
+    "Messenger" : false,
+    "protractor" : false,
+    "URI": false,
+    "ZeroClipboard": false,
+    "$": false,
+    "_" : false
+
+  }
+}

test/spec/fixtures/api-discovery.js

@@ -1,13 +1,13 @@
-describe("APIService", function(){
+describe("APIService", function() {
   var APIService;
 
   beforeEach(function(){
-    inject(function(_APIService_){
+    inject(function(_APIService_) {
       APIService = _APIService_;
     });
   });
 
-  describe("#toResourceGroupVersion", function(){
+  describe("#toResourceGroupVersion", function() {
 
     var tc = [
       // string args
@@ -62,7 +62,8 @@ describe("APIService", function(){
     }));
 
   });
-/*
+
+
   describe("#parseGroupVersion", function(){
     var tc = [
       // invalid cases
@@ -234,6 +235,47 @@ describe("APIService", function(){
       });
     }));
   });
-  */
+
+
+  describe('#availableKinds', function() {
+    var bothSample = ['Binding','ConfigMap','DeploymentConfig','Event','LimitRange','Pod','ReplicaSet','Role','Service', 'Template'];
+    var onlyClusterSample = ['ClusterResourceQuota','Namespace','OAuthAccessToken','PersistentVolume','ProjectRequest','User'];
+
+    // at minimum, prove with tests that it is deduplicating by ignoring openshift!
+    it('should return list of kinds that are scoped to a namespace by default', function() {
+      var namespacedKinds = _.map(APIService.availableKinds(), 'kind');
+      // from the fixtures included, we should have 51 namespaced kinds
+      expect(namespacedKinds.length).toEqual(51);
+      expect( _.difference(bothSample, namespacedKinds).length ).toEqual(0);
+    });
+
+    it('should return list of all kinds, including those that are cluster scoped, when passed a truthy argument', function() {
+      var allKinds = _.map(APIService.availableKinds(true), 'kind');
+      // from the fixtures included, we should have 84 total kinds
+      expect(allKinds.length).toEqual(84);
+      expect( _.difference(bothSample, allKinds).length ).toEqual(0);
+      expect( _.difference(onlyClusterSample, allKinds).length ).toEqual(0);
+    });
+
+    it('should not duplicate kinds that exist in both an api group & the openshift namespace', function() {
+      var allKinds =  _.map(APIService.availableKinds(true), 'kind');
+      var counts = _.reduce(allKinds, function(result, kind) {
+        if(!result[kind]) {
+          result[kind] = 0;
+        }
+        result[kind]++;
+        return result;
+      }, {});
+      // TODO: need a review, not sure why these are still duplicated
+      var whitelist = ['LocalSubjectAccessReview', 'SubjectAccessReview'];
+
+      _.each(counts, function(kindCount, key) {
+        if(!_.includes(whitelist, key)) {
+          expect(kindCount).toEqual(1);
+        }
+      });
+    });
+
+  });
 
 });