Skip to content

Commit 025cb11

Browse files
keiskukeisku
authored
Merge pull request #39 from keisku/fix-bug
Fix resource name detection logic from user input
2 parents 9234322 + 1251317 commit 025cb11

File tree

3 files changed

+150
-64
lines changed

3 files changed

+150
-64
lines changed

‎.github/workflows/test.yaml

+5-1
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,12 @@ jobs:
3434
diff <(sudo microk8s kubectl explore --disable-print-path no.*pro | tr -d '[:space:]') <(sudo microk8s kubectl explain node.spec.providerID | tr -d [':space:'])
3535
diff <(sudo microk8s kubectl explore --disable-print-path node.*pro | tr -d '[:space:]') <(sudo microk8s kubectl explain node.spec.providerID | tr -d [':space:'])
3636
diff <(sudo microk8s kubectl explore --disable-print-path nodes.*pro | tr -d '[:space:]') <(sudo microk8s kubectl explain node.spec.providerID | tr -d [':space:'])
37+
diff <(sudo microk8s kubectl explore --disable-print-path Node.*pro | tr -d '[:space:]') <(sudo microk8s kubectl explain node.spec.providerID | tr -d [':space:'])
3738
diff <(sudo microk8s kubectl explore --disable-print-path provider | tr -d '[:space:]') <(sudo microk8s kubectl explain node.spec.providerID | tr -d [':space:'])
38-
- name: Since 1.27+, kubectl-explain has --disable-print-path been upgraded to v2 which enables OpenAPI v3 by default
39+
- name: For 1.27+
3940
run: |
4041
diff <(sudo microk8s kubectl explore --disable-print-path hpa.*own.*id | tr -d '[:space:]') <(sudo microk8s kubectl explain horizontalpodautoscaler.metadata.ownerReferences.uid | tr -d [':space:'])
42+
diff <(sudo microk8s kubectl explore --disable-print-path csistoragecapacity.maximumVolumeSize | tr -d '[:space:]') <(sudo microk8s kubectl explain csistoragecapacity.maximumVolumeSize | tr -d [':space:'])
43+
diff <(sudo microk8s kubectl explore --disable-print-path csistoragecapacities.maximumVolumeSize | tr -d '[:space:]') <(sudo microk8s kubectl explain csistoragecapacity.maximumVolumeSize | tr -d [':space:'])
44+
diff <(sudo microk8s kubectl explore --disable-print-path CSIStorageCapacity.*VolumeSize | tr -d '[:space:]') <(sudo microk8s kubectl explain csistoragecapacity.maximumVolumeSize | tr -d [':space:'])
4145
if: ${{ 26 < matrix.k8s-minor }}

‎explore/options.go

+71-46
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ import (
1111
"github.com/ktr0731/go-fuzzyfinder"
1212
"github.com/spf13/cobra"
1313
"k8s.io/apimachinery/pkg/api/meta"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415
"k8s.io/apimachinery/pkg/runtime/schema"
1516
"k8s.io/cli-runtime/pkg/genericclioptions"
1617
"k8s.io/client-go/discovery"
1718
openapiclient "k8s.io/client-go/openapi"
1819
_ "k8s.io/client-go/plugin/pkg/client/auth"
1920
"k8s.io/kube-openapi/pkg/util/proto"
2021
cmdutil "k8s.io/kubectl/pkg/cmd/util"
21-
"k8s.io/kubectl/pkg/explain"
2222
"k8s.io/kubectl/pkg/util/openapi"
2323
)
2424

@@ -135,44 +135,54 @@ func (o *Options) Complete(f cmdutil.Factory, args []string) error {
135135
return nil
136136
}
137137

138-
var gotGVR schema.GroupVersionResource
139-
var idx int
140-
// Find the first valid resource name in the inputFieldPath.
141-
for i := 1; i <= len(o.inputFieldPath); i++ {
142-
gotGVR, err = GetGVR(o, o.inputFieldPath[:i])
143-
if err != nil {
144-
continue
138+
gvarMap, gvrs, err := o.discover()
139+
if err != nil {
140+
return err
141+
}
142+
143+
var gvar *groupVersionAPIResource
144+
var resourceIdx int
145+
for i := len(o.inputFieldPath); i > 0; i-- {
146+
var ok bool
147+
gvar, ok = gvarMap[o.inputFieldPath[:i]]
148+
if ok {
149+
resourceIdx = i
150+
break
145151
}
146-
idx = i
147-
break
148152
}
149153
// If the inputFieldPath does not contain a valid resource name,
150-
// inputFiledPath is treated as a regex directly.
151-
if gotGVR.Empty() {
152-
o.gvrs, err = o.listGVRs()
153-
if err != nil {
154-
return err
155-
}
154+
// inputFiledPath is treated as a regex.
155+
if gvar == nil {
156+
o.gvrs = gvrs
156157
return nil
157158
}
158159
// Overwrite the regex if the inputFieldPath contains a valid resource name.
160+
_, ok := gvarMap[o.inputFieldPath[:resourceIdx]]
161+
if !ok {
162+
return fmt.Errorf("no resource found for %s", o.inputFieldPath)
163+
}
159164
var re string
160-
if strings.HasPrefix(o.inputFieldPath, gotGVR.Resource) {
161-
// E.g., "nodes.*spec" -> ".*spec"
162-
re = strings.TrimPrefix(o.inputFieldPath, gotGVR.Resource)
163-
} else if strings.HasPrefix(o.inputFieldPath, singularResource(gotGVR.Resource)) {
164-
// E.g., "node.*spec" -> ".*spec"
165-
re = strings.TrimPrefix(o.inputFieldPath, singularResource(gotGVR.Resource))
165+
if strings.HasPrefix(o.inputFieldPath, gvar.Resource) {
166+
re = strings.TrimPrefix(o.inputFieldPath, gvar.Resource)
167+
} else if strings.HasPrefix(o.inputFieldPath, gvar.Kind) {
168+
re = strings.TrimPrefix(o.inputFieldPath, gvar.Kind)
169+
} else if strings.HasPrefix(o.inputFieldPath, gvar.SingularName) {
170+
re = strings.TrimPrefix(o.inputFieldPath, gvar.SingularName)
166171
} else {
167-
// E.g., "no.*spec" -> ".*spec"
168-
prefix := o.inputFieldPath[:idx]
169-
re = strings.TrimPrefix(o.inputFieldPath, prefix)
172+
for _, shortName := range gvar.ShortNames {
173+
if strings.HasPrefix(o.inputFieldPath, shortName) {
174+
re = strings.TrimPrefix(o.inputFieldPath, shortName)
175+
}
176+
}
177+
}
178+
if re == "" {
179+
return fmt.Errorf("cannot find resource name in %s", o.inputFieldPath)
170180
}
171181
o.inputFieldPathRegex, err = regexp.Compile(re)
172182
if err != nil {
173183
return err
174184
}
175-
o.gvrs = []schema.GroupVersionResource{gotGVR}
185+
o.gvrs = []schema.GroupVersionResource{gvar.GroupVersionResource}
176186

177187
return nil
178188
}
@@ -237,13 +247,6 @@ func (o *Options) Run() error {
237247
return pathExplainers[paths[idx]].explain(o.Out, paths[idx])
238248
}
239249

240-
func singularResource(resource string) string {
241-
if strings.HasSuffix(resource, "s") {
242-
return resource[:len(resource)-1]
243-
}
244-
return resource
245-
}
246-
247250
func (o *Options) listGVRs() ([]schema.GroupVersionResource, error) {
248251
lists, err := o.discovery.ServerPreferredResources()
249252
if err != nil {
@@ -287,21 +290,43 @@ func (o *Options) findGVR() (schema.GroupVersionResource, error) {
287290
return gvrs[idx], nil
288291
}
289292

290-
// TODO: Find a way to mock meta.RESTMapper to avoid defining it as a variable.
291-
var GetGVR = func(o *Options, name string) (schema.GroupVersionResource, error) {
292-
return o.getGVR(name)
293+
type groupVersionAPIResource struct {
294+
schema.GroupVersionResource
295+
metav1.APIResource
293296
}
294297

295-
func (o *Options) getGVR(name string) (schema.GroupVersionResource, error) {
296-
var ret schema.GroupVersionResource
297-
var err error
298-
if len(o.apiVersion) == 0 {
299-
ret, _, err = explain.SplitAndParseResourceRequestWithMatchingPrefix(name, o.mapper)
300-
} else {
301-
ret, _, err = explain.SplitAndParseResourceRequest(name, o.mapper)
302-
}
298+
func (o *Options) discover() (map[string]*groupVersionAPIResource, []schema.GroupVersionResource, error) {
299+
lists, err := o.discovery.ServerPreferredResources()
303300
if err != nil {
304-
return schema.GroupVersionResource{}, fmt.Errorf("get the group version resource by %s %s: %w", o.apiVersion, name, err)
301+
return nil, nil, err
302+
}
303+
var gvrs []schema.GroupVersionResource
304+
m := make(map[string]*groupVersionAPIResource)
305+
for _, list := range lists {
306+
if len(list.APIResources) == 0 {
307+
continue
308+
}
309+
gv, err := schema.ParseGroupVersion(list.GroupVersion)
310+
if err != nil {
311+
continue
312+
}
313+
for _, resource := range list.APIResources {
314+
gvr := gv.WithResource(resource.Name)
315+
gvrs = append(gvrs, gvr)
316+
r := groupVersionAPIResource{
317+
GroupVersionResource: gvr,
318+
APIResource: resource,
319+
}
320+
m[resource.Name] = &r
321+
m[resource.Kind] = &r
322+
m[resource.SingularName] = &r
323+
for _, shortName := range resource.ShortNames {
324+
m[shortName] = &r
325+
}
326+
}
305327
}
306-
return ret, nil
328+
sort.SliceStable(gvrs, func(i, j int) bool {
329+
return gvrs[i].String() < gvrs[j].String()
330+
})
331+
return m, gvrs, nil
307332
}

‎explore/options_test.go

+74-17
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/keisku/kubectl-explore/explore"
1818
"github.com/stretchr/testify/require"
1919
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20-
"k8s.io/apimachinery/pkg/runtime/schema"
2120
"k8s.io/cli-runtime/pkg/genericclioptions"
2221
"k8s.io/client-go/discovery"
2322
openapiclient "k8s.io/client-go/openapi"
@@ -188,22 +187,30 @@ func Test_Run(t *testing.T) {
188187
},
189188
},
190189
},
191-
}
192-
explore.GetGVR = func(_ *explore.Options, inputFieldPath string) (schema.GroupVersionResource, error) {
193-
node := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "nodes"}
194-
hpa := schema.GroupVersionResource{Group: "autoscaling", Version: "v2", Resource: "horizontalpodautoscalers"}
195-
gvr, ok := map[string]schema.GroupVersionResource{
196-
"no": node,
197-
"node": node,
198-
"nodes": node,
199-
"hpa": hpa,
200-
"horizontalpodautoscaler": hpa,
201-
"horizontalpodautoscalers": hpa,
202-
}[inputFieldPath]
203-
if !ok {
204-
return schema.GroupVersionResource{}, fmt.Errorf("no resource found for %s", inputFieldPath)
205-
}
206-
return gvr, nil
190+
{
191+
GroupVersion: "storage.k8s.io/v1",
192+
APIResources: []v1.APIResource{
193+
{
194+
Name: "csistoragecapacities",
195+
SingularName: "csistoragecapacity",
196+
Namespaced: true,
197+
Kind: "CSIStorageCapacity",
198+
ShortNames: []string{},
199+
},
200+
},
201+
},
202+
{
203+
GroupVersion: "v1",
204+
APIResources: []v1.APIResource{
205+
{
206+
Name: "componentstatuses",
207+
SingularName: "componentstatus",
208+
Namespaced: false,
209+
Kind: "ComponentStatus",
210+
ShortNames: []string{"cs"},
211+
},
212+
},
213+
},
207214
}
208215
tests := []struct {
209216
inputFieldPath string
@@ -256,6 +263,56 @@ func Test_Run(t *testing.T) {
256263
"PATH: horizontalpodautoscalers.metadata.ownerReferences.uid",
257264
},
258265
},
266+
{
267+
inputFieldPath: "horizontalpodautoscalers.*own.*id",
268+
expectRunError: false,
269+
expectKeywords: []string{
270+
"autoscaling",
271+
"HorizontalPodAutoscaler",
272+
"v2",
273+
"PATH: horizontalpodautoscalers.metadata.ownerReferences.uid",
274+
},
275+
},
276+
{
277+
inputFieldPath: "horizontalpodautoscaler.*own.*id",
278+
expectRunError: false,
279+
expectKeywords: []string{
280+
"autoscaling",
281+
"HorizontalPodAutoscaler",
282+
"v2",
283+
"PATH: horizontalpodautoscalers.metadata.ownerReferences.uid",
284+
},
285+
},
286+
{
287+
inputFieldPath: "csistoragecapacity.maximumVolumeSize",
288+
expectRunError: false,
289+
expectKeywords: []string{
290+
"CSIStorageCapacity",
291+
"storage.k8s.io",
292+
"v1",
293+
"PATH: csistoragecapacities.maximumVolumeSize",
294+
},
295+
},
296+
{
297+
inputFieldPath: "csistoragecapacities.maximumVolumeSize",
298+
expectRunError: false,
299+
expectKeywords: []string{
300+
"CSIStorageCapacity",
301+
"storage.k8s.io",
302+
"v1",
303+
"PATH: csistoragecapacities.maximumVolumeSize",
304+
},
305+
},
306+
{
307+
inputFieldPath: "CSIStorageCapacity.*VolumeSize",
308+
expectRunError: false,
309+
expectKeywords: []string{
310+
"CSIStorageCapacity",
311+
"storage.k8s.io",
312+
"v1",
313+
"PATH: csistoragecapacities.maximumVolumeSize",
314+
},
315+
},
259316
}
260317
for _, tt := range tests {
261318
for _, version := range k8sVersions {

0 commit comments

Comments
 (0)