Merge pull request #377 from cben/fix-non-suffix-plurals

[v4.1.z] Fix underscores for non-suffix plurals

CHANGELOG.md

@@ -4,7 +4,15 @@ Notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 Kubeclient release versioning follows [SemVer](https://semver.org/).
 
-## 4.1.0 — 2018-11-28
+## Unreleased
+
+### Fixed
+
+- Fixed method names for non-suffix plurals such as y -> ies  (#377).
+
+## 4.1.0 — 2018-11-28 — REGRESSION
+
+This version broke method names where plural is not just adding a suffix, notably y -> ies (bug #376).
 
 ### Fixed
 - Support custom resources with lowercase `kind` (#361).
@@ -33,7 +41,7 @@ Kubeclient release versioning follows [SemVer](https://semver.org/).
 
 ## 3.1.1 - 2018-06-01 — REGRESSION
 
-In this version `Kubeclient::Config.read` raises Psych::DisallowedClass on legal yaml configs containing a timestamp, for example gcp access-token expiry (#337).
+In this version `Kubeclient::Config.read` raises Psych::DisallowedClass on legal yaml configs containing a timestamp, for example gcp access-token expiry (bug #337).
 
 ### Security
 - Changed `Kubeclient::Config.read` to use `YAML.safe_load` (#334).

lib/kubeclient/common.rb

@@ -132,13 +132,13 @@ def discover
     end
 
     def self.parse_definition(kind, name)
-      # Kubernetes gives us have 3 inputs:
-      #   kind: "ComponentStatus"
-      #   name: "componentstatuses"
-      #   singularName: "componentstatus" (usually kind.downcase)
+      # Kubernetes gives us 3 inputs:
+      #   kind: "ComponentStatus", "NetworkPolicy", "Endpoints"
+      #   name: "componentstatuses", "networkpolicies", "endpoints"
+      #   singularName: "componentstatus" etc (usually omitted, defaults to kind.downcase)
       # and want to derive singular and plural method names, with underscores:
-      #   "component_status"
-      #   "component_statuses"
+      #   "network_policy"
+      #   "network_policies"
       # kind's CamelCase word boundaries determine our placement of underscores.
 
       if IRREGULAR_NAMES[kind]
@@ -150,13 +150,22 @@ def self.parse_definition(kind, name)
         # But how?  If it differs from kind.downcase, kind's word boundaries don't apply.
         singular_name = kind.downcase
 
-        if name.start_with?(kind.downcase)
-          plural_suffix = name[kind.downcase.length..-1]             # "es"
-          singular_underscores = ClientMixin.underscore_entity(kind) # "component_status"
-          method_names = [singular_underscores, singular_underscores + plural_suffix]
-        else
-          # Something weird, can't infer underscores for plural so just give them up
+        if !(/[A-Z]/ =~ kind)
+          # Some custom resources have a fully lowercase kind - can't infer underscores.
           method_names = [singular_name, name]
+        else
+          # Some plurals are not exact suffixes, e.g. NetworkPolicy -> networkpolicies.
+          # So don't expect full last word to match.
+          /^(?<prefix>(.*[A-Z]))(?<singular_suffix>[^A-Z]*)$/ =~ kind  # "NetworkP", "olicy"
+          if name.start_with?(prefix.downcase)
+            plural_suffix = name[prefix.length..-1]                    # "olicies"
+            prefix_underscores = ClientMixin.underscore_entity(prefix) # "network_p"
+            method_names = [prefix_underscores + singular_suffix,      # "network_policy"
+                            prefix_underscores + plural_suffix]        # "network_policies"
+          else
+            # Something weird, can't infer underscores for plural so just give them up
+            method_names = [singular_name, name]
+          end
         end
       end
 

test/json/extensions_v1beta1_api_resource_list.json

@@ -0,0 +1,217 @@
+{
+  "kind": "APIResourceList",
+  "groupVersion": "extensions/v1beta1",
+  "resources": [
+    {
+      "name": "daemonsets",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "DaemonSet",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "ds"
+      ]
+    },
+    {
+      "name": "daemonsets/status",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "DaemonSet",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "deployments",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "Deployment",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "deploy"
+      ]
+    },
+    {
+      "name": "deployments/rollback",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "DeploymentRollback",
+      "verbs": [
+        "create"
+      ]
+    },
+    {
+      "name": "deployments/scale",
+      "singularName": "",
+      "namespaced": true,
+      "group": "extensions",
+      "version": "v1beta1",
+      "kind": "Scale",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "deployments/status",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "Deployment",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "ingresses",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "Ingress",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "ing"
+      ]
+    },
+    {
+      "name": "ingresses/status",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "Ingress",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "networkpolicies",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "NetworkPolicy",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "netpol"
+      ]
+    },
+    {
+      "name": "podsecuritypolicies",
+      "singularName": "",
+      "namespaced": false,
+      "kind": "PodSecurityPolicy",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "psp"
+      ]
+    },
+    {
+      "name": "replicasets",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "ReplicaSet",
+      "verbs": [
+        "create",
+        "delete",
+        "deletecollection",
+        "get",
+        "list",
+        "patch",
+        "update",
+        "watch"
+      ],
+      "shortNames": [
+        "rs"
+      ]
+    },
+    {
+      "name": "replicasets/scale",
+      "singularName": "",
+      "namespaced": true,
+      "group": "extensions",
+      "version": "v1beta1",
+      "kind": "Scale",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "replicasets/status",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "ReplicaSet",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    },
+    {
+      "name": "replicationcontrollers",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "ReplicationControllerDummy",
+      "verbs": []
+    },
+    {
+      "name": "replicationcontrollers/scale",
+      "singularName": "",
+      "namespaced": true,
+      "kind": "Scale",
+      "verbs": [
+        "get",
+        "patch",
+        "update"
+      ]
+    }
+  ]
+}

test/test_common.rb

@@ -74,4 +74,17 @@ def test_format_datetime_with_time
     formatted = client.send(:format_datetime, value)
     assert_equal(formatted, '2018-04-30T19:20:33.000000000+00:00')
   end
+
+  def test_parse_definition_with_unconventional_names
+    %w[
+      PluralPolicy pluralpolicies plural_policy plural_policies
+      LatinDatum latindata latin_datum latin_data
+      Noseparator noseparators noseparator noseparators
+      lowercase lowercases lowercase lowercases
+    ].each_slice(4) do |kind, plural, expected_single, expected_plural|
+      method_names = Kubeclient::ClientMixin.parse_definition(kind, plural).method_names
+      assert_equal(method_names[0], expected_single)
+      assert_equal(method_names[1], expected_plural)
+    end
+  end
 end

test/test_missing_methods.rb

@@ -41,6 +41,18 @@ def test_missing
     end
   end
 
+  def test_nonsuffix_plurals
+    stub_request(:get, %r{/apis/extensions/v1beta1$}).to_return(
+      body: open_test_file('extensions_v1beta1_api_resource_list.json'),
+      status: 200
+    )
+    client = Kubeclient::Client.new('http://localhost:8080/apis/extensions', 'v1beta1')
+    assert_equal(true, client.respond_to?(:get_network_policy))
+    assert_equal(true, client.respond_to?(:get_network_policies))
+    assert_equal(true, client.respond_to?(:get_pod_security_policy))
+    assert_equal(true, client.respond_to?(:get_pod_security_policies))
+  end
+
   def test_irregular_names
     stub_core_api_list
     client = Kubeclient::Client.new('http://localhost:8080/api/', 'v1')