From e439ee42a93b466953ee25a51a9182db08a38958 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sat, 15 Dec 2018 23:23:11 +0200 Subject: [PATCH 1/8] Add tests for non-suffix y->ies plurals #376 --- .../extensions_v1beta1_api_resource_list.json | 217 ++++++++++++++++++ test/test_missing_methods.rb | 12 + 2 files changed, 229 insertions(+) create mode 100644 test/json/extensions_v1beta1_api_resource_list.json diff --git a/test/json/extensions_v1beta1_api_resource_list.json b/test/json/extensions_v1beta1_api_resource_list.json new file mode 100644 index 00000000..16fc80cf --- /dev/null +++ b/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" + ] + } + ] +} \ No newline at end of file diff --git a/test/test_missing_methods.rb b/test/test_missing_methods.rb index a52181a4..67614c95 100644 --- a/test/test_missing_methods.rb +++ b/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') From 2e904534b10ba886a76bf37f9a89007619af00e0 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sat, 15 Dec 2018 23:25:12 +0200 Subject: [PATCH 2/8] Restore non-exact matching of last word to support y->ies plurals. Fixes #376. Partially undoes 783c58ba4124064d13abdbc5ea5dc272bf0dbe77. --- lib/kubeclient/common.rb | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/kubeclient/common.rb b/lib/kubeclient/common.rb index 98272f43..0addad37 100644 --- a/lib/kubeclient/common.rb +++ b/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. + /^(?(.*[A-Z]))(?[^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 From bfe132bf3a090e4edee1cd99f0f7ccfed29bbe1e Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 16 Dec 2018 17:17:53 +0200 Subject: [PATCH 3/8] CHANGELOG for #377. --- CHANGELOG.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 619d974d..37eb7ef0 100644 --- a/CHANGELOG.md +++ b/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). From cd345909b9a080986d24dc2de96bdc95c75ebd53 Mon Sep 17 00:00:00 2001 From: Moti Asayag Date: Sun, 16 Dec 2018 18:14:07 +0200 Subject: [PATCH 4/8] Add more plural tests, including LatinDatum / latindata --- test/test_common.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_common.rb b/test/test_common.rb index c37ae336..a7906bf2 100644 --- a/test/test_common.rb +++ b/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 From 6fa703408a6281bdc27d8f02770301ef088c0580 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Mon, 17 Dec 2018 19:16:24 +0200 Subject: [PATCH 5/8] Test lack of methods for `processedtemplates` pseudo-entity Openshift discovery lists both `templates` and `processedtemplates` for same kind "Template": https://github.com/openshift/origin/issues/21668 `templates` is a regular stored api object. `processedtemplates` endpoint isn't a stored object, its POST is a special input (template + params) -> output function. That functionality is already covered in kubeclient by `process_template` method. It doesn't need regular create_, get_, watch_ etc methods. This is already so because in all openshift versions, `templates` comes after `processedtemplates` and replaces it in `@entities["template"]`. --- test/json/template.json | 27 +++++++ ...mplate.openshift.io_api_resource_list.json | 75 +++++++++++++++++++ test/json/template_list.json | 35 +++++++++ test/test_process_template.rb | 36 +++++++++ 4 files changed, 173 insertions(+) create mode 100644 test/json/template.json create mode 100644 test/json/template.openshift.io_api_resource_list.json create mode 100644 test/json/template_list.json diff --git a/test/json/template.json b/test/json/template.json new file mode 100644 index 00000000..85d8bad7 --- /dev/null +++ b/test/json/template.json @@ -0,0 +1,27 @@ +{ + "apiVersion": "template.openshift.io/v1", + "kind": "Template", + "metadata": { + "creationTimestamp": "2018-12-17T16:11:36Z", + "name": "my-template", + "namespace": "default", + "resourceVersion": "21954", + "selfLink": "/apis/template.openshift.io/v1/namespaces/default/templates/my-template", + "uid": "6e03e3e6-0216-11e9-b1e0-68f728fac3ab" + }, + "objects": [ + { + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "name": "${NAME_PREFIX}my-service" + } + } + ], + "parameters": [ + { + "description": "Prefix for names", + "name": "NAME_PREFIX" + } + ] +} diff --git a/test/json/template.openshift.io_api_resource_list.json b/test/json/template.openshift.io_api_resource_list.json new file mode 100644 index 00000000..1ba147f7 --- /dev/null +++ b/test/json/template.openshift.io_api_resource_list.json @@ -0,0 +1,75 @@ +{ + "kind": "APIResourceList", + "apiVersion": "v1", + "groupVersion": "template.openshift.io/v1", + "resources": [ + { + "name": "brokertemplateinstances", + "singularName": "", + "namespaced": false, + "kind": "BrokerTemplateInstance", + "verbs": [ + "create", + "delete", + "deletecollection", + "get", + "list", + "patch", + "update", + "watch" + ] + }, + { + "name": "processedtemplates", + "singularName": "", + "namespaced": true, + "kind": "Template", + "verbs": [ + "create" + ] + }, + { + "name": "templateinstances", + "singularName": "", + "namespaced": true, + "kind": "TemplateInstance", + "verbs": [ + "create", + "delete", + "deletecollection", + "get", + "list", + "patch", + "update", + "watch" + ] + }, + { + "name": "templateinstances/status", + "singularName": "", + "namespaced": true, + "kind": "TemplateInstance", + "verbs": [ + "get", + "patch", + "update" + ] + }, + { + "name": "templates", + "singularName": "", + "namespaced": true, + "kind": "Template", + "verbs": [ + "create", + "delete", + "deletecollection", + "get", + "list", + "patch", + "update", + "watch" + ] + } + ] +} \ No newline at end of file diff --git a/test/json/template_list.json b/test/json/template_list.json new file mode 100644 index 00000000..a0f84ad3 --- /dev/null +++ b/test/json/template_list.json @@ -0,0 +1,35 @@ +{ + "kind": "TemplateList", + "apiVersion": "template.openshift.io/v1", + "metadata": { + "selfLink": "/apis/template.openshift.io/v1/namespaces/default/templates", + "resourceVersion": "22758" + }, + "items": [ + { + "metadata": { + "name": "my-template", + "namespace": "default", + "selfLink": "/apis/template.openshift.io/v1/namespaces/default/templates/my-template", + "uid": "6e03e3e6-0216-11e9-b1e0-68f728fac3ab", + "resourceVersion": "21954", + "creationTimestamp": "2018-12-17T16:11:36Z" + }, + "objects": [ + { + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "name": "${NAME_PREFIX}my-service" + } + } + ], + "parameters": [ + { + "name": "NAME_PREFIX", + "description": "Prefix for names" + } + ] + } + ] +} diff --git a/test/test_process_template.rb b/test/test_process_template.rb index a22bf975..e3b4670f 100644 --- a/test/test_process_template.rb +++ b/test/test_process_template.rb @@ -41,4 +41,40 @@ def test_process_template data['metadata']['namespace'] == 'default' end end + + # Ensure _template and _templates methods hit `/templates` rather than + # `/processedtemplates` URL. + def test_templates_methods + stub_request(:get, %r{/apis/template\.openshift\.io/v1$}).to_return( + body: open_test_file('template.openshift.io_api_resource_list.json'), + status: 200 + ) + client = Kubeclient::Client.new('http://localhost:8080/apis/template.openshift.io', 'v1') + + expected_url = 'http://localhost:8080/apis/template.openshift.io/v1/namespaces/default/templates' + stub_request(:get, expected_url) + .to_return(body: open_test_file('template_list.json'), status: 200) + client.get_templates(namespace: 'default') + assert_requested(:get, expected_url, times: 1) + + expected_url = 'http://localhost:8080/apis/template.openshift.io/v1/namespaces/default/templates/my-template' + stub_request(:get, expected_url) + .to_return(body: open_test_file('template.json'), status: 200) + client.get_template('my-template', 'default') + assert_requested(:get, expected_url, times: 1) + end + + def test_no_processedtemplates_methods + stub_request(:get, %r{/apis/template\.openshift\.io/v1$}).to_return( + body: open_test_file('template.openshift.io_api_resource_list.json'), + status: 200 + ) + client = Kubeclient::Client.new('http://localhost:8080/apis/template.openshift.io', 'v1') + client.discover + + refute_respond_to(client, :get_processedtemplates) + refute_respond_to(client, :get_processedtemplate) + refute_respond_to(client, :get_processed_templates) + refute_respond_to(client, :get_processed_template) + end end From 7610ecfe5f35470bddb7a95a1937f729fc0e2c8a Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Mon, 17 Dec 2018 19:37:03 +0200 Subject: [PATCH 6/8] Don't try to create methods for `processedtemplates` pseudo-entity --- lib/kubeclient/common.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/kubeclient/common.rb b/lib/kubeclient/common.rb index 0addad37..6f2c847d 100644 --- a/lib/kubeclient/common.rb +++ b/lib/kubeclient/common.rb @@ -516,6 +516,9 @@ def load_entities @entities = {} fetch_entities['resources'].each do |resource| next if resource['name'].include?('/') + # Not a regular entity, special functionality covered by `process_template`. + # https://github.com/openshift/origin/issues/21668 + next if resource['kind'] == 'Template' && resource['name'] == 'processedtemplates' resource['kind'] ||= Kubeclient::Common::MissingKindCompatibility.resource_kind(resource['name']) entity = ClientMixin.parse_definition(resource['kind'], resource['name']) From d0cf2fabae0c057b9b0efe3f6554d3b1a85fc2a8 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Mon, 17 Dec 2018 23:38:26 +0200 Subject: [PATCH 7/8] Bump kubeclient to 4.1.1 --- lib/kubeclient/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kubeclient/version.rb b/lib/kubeclient/version.rb index dae1669e..1142792f 100644 --- a/lib/kubeclient/version.rb +++ b/lib/kubeclient/version.rb @@ -1,4 +1,4 @@ # Kubernetes REST-API Client module Kubeclient - VERSION = '4.1.0'.freeze + VERSION = '4.1.1'.freeze end From 2361746fa55b6faaf6c1f3ed194cfeb46ec24fac Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Mon, 17 Dec 2018 23:38:19 +0200 Subject: [PATCH 8/8] Changelog for 4.1.1 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37eb7ef0..7273194f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ 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/). -## Unreleased +## 4.1.1 — 2018-12-17 ### Fixed