From 4add679528d316e9fe936e7a358690ed7a68c204 Mon Sep 17 00:00:00 2001
From: Nikhita Raghunath <nikitaraghunath@gmail.com>
Date: Wed, 1 Nov 2017 19:04:36 +0530
Subject: [PATCH] kubectl explain: ignore trailing period

kubectl explain ingress.spec.rules.http.paths. is valid
and defaults to kubectl explain ingress.spec.rules.http.paths

Rational: We use kubectl explain by adding fields
(e.g. service, then service.spec, then service.spec.ports ...)
so it's very easy to forget a trailing . at the end. We should
ignore the trailing period and display the result without it.
---
 pkg/kubectl/explain/BUILD                |  3 +
 pkg/kubectl/explain/explain.go           |  4 ++
 pkg/kubectl/explain/explain_test.go      | 81 ++++++++++++++++++++++++
 pkg/kubectl/explain/field_lookup_test.go |  4 ++
 4 files changed, 92 insertions(+)
 create mode 100644 pkg/kubectl/explain/explain_test.go

diff --git a/pkg/kubectl/explain/BUILD b/pkg/kubectl/explain/BUILD
index 6792dbf6bf1..1346ff0c900 100644
--- a/pkg/kubectl/explain/BUILD
+++ b/pkg/kubectl/explain/BUILD
@@ -37,6 +37,7 @@ filegroup(
 go_test(
     name = "go_default_test",
     srcs = [
+        "explain_test.go",
         "field_lookup_test.go",
         "fields_printer_test.go",
         "formatter_test.go",
@@ -48,7 +49,9 @@ go_test(
     importpath = "k8s.io/kubernetes/pkg/kubectl/explain",
     library = ":go_default_library",
     deps = [
+        "//pkg/kubectl/cmd/testing:go_default_library",
         "//pkg/kubectl/cmd/util/openapi/testing:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
     ],
 )
diff --git a/pkg/kubectl/explain/explain.go b/pkg/kubectl/explain/explain.go
index a0dcc389a2d..b940eae61ab 100644
--- a/pkg/kubectl/explain/explain.go
+++ b/pkg/kubectl/explain/explain.go
@@ -30,6 +30,10 @@ type fieldsPrinter interface {
 
 func splitDotNotation(model string) (string, []string) {
 	var fieldsPath []string
+
+	// ignore trailing period
+	model = strings.TrimSuffix(model, ".")
+
 	dotModel := strings.Split(model, ".")
 	if len(dotModel) >= 1 {
 		fieldsPath = dotModel[1:]
diff --git a/pkg/kubectl/explain/explain_test.go b/pkg/kubectl/explain/explain_test.go
new file mode 100644
index 00000000000..29862fa6383
--- /dev/null
+++ b/pkg/kubectl/explain/explain_test.go
@@ -0,0 +1,81 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package explain
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
+)
+
+func TestSplitAndParseResourceRequest(t *testing.T) {
+	tests := []struct {
+		name       string
+		inresource string
+
+		expectedInResource string
+		expectedFieldsPath []string
+		expectedErr        bool
+	}{
+		{
+			name:       "no trailing period",
+			inresource: "field1.field2.field3",
+
+			expectedInResource: "field1",
+			expectedFieldsPath: []string{"field2", "field3"},
+		},
+		{
+			name:       "trailing period with correct fieldsPath",
+			inresource: "field1.field2.field3.",
+
+			expectedInResource: "field1",
+			expectedFieldsPath: []string{"field2", "field3"},
+		},
+		{
+			name:       "trailing period with incorrect fieldsPath",
+			inresource: "field1.field2.field3.",
+
+			expectedInResource: "field1",
+			expectedFieldsPath: []string{"field2", "field3", ""},
+			expectedErr:        true,
+		},
+	}
+
+	mapper := getMapper()
+	for _, test := range tests {
+		gotInResource, gotFieldsPath, err := SplitAndParseResourceRequest(test.inresource, mapper)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+
+		if !reflect.DeepEqual(test.expectedInResource, gotInResource) && !test.expectedErr {
+			t.Errorf("%s: expected inresource: %s, got: %s", test.name, test.expectedInResource, gotInResource)
+		}
+
+		if !reflect.DeepEqual(test.expectedFieldsPath, gotFieldsPath) && !test.expectedErr {
+			t.Errorf("%s: expected fieldsPath: %s, got: %s", test.name, test.expectedFieldsPath, gotFieldsPath)
+		}
+	}
+}
+
+func getMapper() meta.RESTMapper {
+	f, _, _, _ := cmdtesting.NewTestFactory()
+	mapper, _ := f.Object()
+	return mapper
+}
diff --git a/pkg/kubectl/explain/field_lookup_test.go b/pkg/kubectl/explain/field_lookup_test.go
index 239ad704005..a2b99212b44 100644
--- a/pkg/kubectl/explain/field_lookup_test.go
+++ b/pkg/kubectl/explain/field_lookup_test.go
@@ -54,6 +54,10 @@ func TestFindField(t *testing.T) {
 			path: []string{"field1", "what?"},
 			err:  `field "what?" does not exist`,
 		},
+		{
+			path: []string{"field1", ""},
+			err:  `field "" does not exist`,
+		},
 	}
 
 	for _, test := range tests {