fix: kubectl kustomize can have bogus output

This also fixes a long-standing issue in the test util `expectError`. It was doing incomplete validation of the error code.

Fixes #6114

Author: starpit

packages/test/src/api/repl-expect.ts

@@ -171,23 +171,30 @@ export const ok = async (res: AppAndCount) =>
     .then(elts => assert.strictEqual(elts.length, 0))
     .then(() => res)
 
-export const error = (statusCode: number | string, expect?: string) => async (res: AppAndCount) =>
-  expectOK(res, {
-    selfSelector: `.oops[data-status-code="${statusCode || 0}"]`,
-    expectError: true,
-    expect: expect
-  }).then(() => res.app)
-
-export const errorWithPassthrough = (statusCode: number | string, expect?: string) => async (
-  res: AppAndCount
-): Promise<number> =>
-  expectOK(res, {
-    selector: `.oops[data-status-code="${statusCode || 0}"]`,
-    expectError: true,
-    expect: expect,
-    passthrough: true
+export const error = (statusCode: number | string, expect?: string) => async (res: AppAndCount) => {
+  await expectOK(res, {
+    expectError: true
   })
 
+  if (expect) {
+    const selfSelector = `.oops[data-status-code="${statusCode || 0}"]`
+    const selector = `${Selectors.OUTPUT_N(res.count, res.splitIndex || 1)}${selfSelector || ''}`
+    let idx = 0
+    await res.app.client.waitUntil(
+      async () => {
+        const actualText = await res.app.client.$(selector).then(_ => _.getText())
+        if (++idx > 5) {
+          console.error(`still waiting for actualText=${actualText} expectedText=${expect}`)
+        }
+        return actualText.indexOf(expect) >= 0
+      },
+      { timeout: waitTimeout }
+    )
+  }
+
+  return res.app
+}
+
 export const blankWithOpts = (opts = {}) => async (res: AppAndCount) =>
   expectOK(res, Object.assign({ selector: '', expectError: true }, opts))
 

packages/test/src/api/util.ts

@@ -241,78 +241,6 @@ export const expectText = (app: Application, expectedText: string, exact = true)
   return app
 }
 
-/**
- *
- * - Type the command
- * - Expect a command not found
- * - Expect the given list of available related commands
- * - Optionally click on a given "click" index of the available list
- * - If so, then either: expect the subsequent command output to have the given terminal breadcrumb in its usage message
- *                   or: expect the sidecar icon to be "sidecar"
- *
- */
-export function expectSuggestionsFor(
-  this: Common.ISuite,
-  cmd: string,
-  expectedAvailable: string[],
-  {
-    click = undefined,
-    expectedBreadcrumb = undefined,
-    sidecar: expectedIcon = undefined,
-    expectedString = undefined
-  }: { click?: number; expectedBreadcrumb?: string; sidecar?: string; expectedString?: string } = {}
-) {
-  return CLI.command(cmd, this.app)
-    .then(ReplExpect.errorWithPassthrough(404))
-    .then(N => {
-      const base = `${Selectors.OUTPUT_N(N)} .user-error-available-commands .log-line`
-      const availableItems = `${base} .clickable`
-
-      return this.app.client
-        .$(availableItems)
-        .then(_ => _.getText())
-        .then(expectArray(expectedAvailable, false, true))
-        .then(async () => {
-          if (click !== undefined) {
-            // then click on the given index; note that nth-child is 1-indexed, hence the + 1 part
-            const clickOn = `${base}:nth-child(${click + 1}) .clickable`
-
-            await this.app.client.$(clickOn).then(_ => _.click())
-
-            if (expectedBreadcrumb) {
-              //
-              // then expect the next command to have the given terminal breadcrumb
-              //
-              const breadcrumb = `${Selectors.OUTPUT_N(N + 1)} .bx--breadcrumb-item:last-child .bx--no-link`
-              return this.app.client
-                .$(breadcrumb)
-                .then(_ => _.getText())
-                .then(actualBreadcrumb => assert.strictEqual(actualBreadcrumb, expectedBreadcrumb))
-            } else if (expectedIcon) {
-              //
-              // then wait for the sidecar to be open and showing the expected sidecar icon text
-              //
-              const icon = `${Selectors.SIDECAR(N)} .sidecar-header-icon-wrapper .sidecar-header-icon`
-              return SidecarExpect.open({ app: this.app, count: N })
-                .then(() => this.app.client.$(icon))
-                .then(_ => _.getText())
-                .then(actualIcon => actualIcon.toLowerCase())
-                .then(actualIcon => assert.strictEqual(actualIcon, expectedIcon))
-            } else if (expectedString) {
-              //
-              // then wait for the given command output
-              //
-              return this.app.client.waitUntil(async () => {
-                const text = await this.app.client.$(Selectors.OUTPUT_N(N + 1)).then(_ => _.getText())
-                return text === expectedString
-              })
-            }
-          }
-        })
-    })
-    .catch(Common.oops(this))
-}
-
 /** @return the current number of tabs */
 export async function tabCount(app: Application): Promise<number> {
   const topTabs = await app.client.$$(Selectors.TOP_TAB)

plugins/plugin-client-common/src/components/Content/Table/kui2carbon.ts

@@ -80,7 +80,7 @@ export function kuiRow2carbonRow(headers: DataTableHeader[], justUpdated = false
 
     if (!row.attributes) row.attributes = []
     row.attributes.forEach((attr, cidx) => {
-      const kkey = attr.key || headers[cidx + 1] ? headers[cidx + 1].key : undefined
+      const kkey = headers[cidx + 1] ? headers[cidx + 1].key : attr.key
       if (!attr.key && kkey) {
         attr.key = kkey
       }

plugins/plugin-core-support/src/test/core/command-not-found-suggestions.ts

@@ -20,7 +20,7 @@ import { Arguments, Table, isTable, is404or409 } from '@kui-shell/core'
 import { fetchFile } from '../../../lib/util/fetch-file'
 import { Explained, getKindAndVersion } from '../../kubectl/explain'
 
-import { KubeOptions, fileOf, getLabel, getNamespace } from '../../kubectl/options'
+import { KubeOptions, getFileFromArgv, getLabel, getNamespace } from '../../kubectl/options'
 
 import status from './status'
 import handleErrors from './errors'
@@ -48,7 +48,12 @@ export default async function createDirect(
   _kind?: Promise<Explained>
 ) {
   // For now, we only handle create-by-name
-  if (!fileOf(args) && !getLabel(args) && !args.parsedOptions['dry-run'] && !args.parsedOptions['field-selector']) {
+  if (
+    !getFileFromArgv(args) &&
+    !getLabel(args) &&
+    !args.parsedOptions['dry-run'] &&
+    !args.parsedOptions['field-selector']
+  ) {
     const explainedKind = await (_kind ||
       getKindAndVersion(getCommandFromArgs(args), args, args.argvNoOptions[args.argvNoOptions.indexOf(verb) + 1]))
     const { kind, version } = explainedKind

plugins/plugin-kubectl/src/controller/client/direct/create.ts

@@ -20,7 +20,7 @@ import { Arguments, is404 } from '@kui-shell/core'
 import { fetchFile } from '../../../lib/util/fetch-file'
 import { Explained, getKindAndVersion } from '../../kubectl/explain'
 
-import { KubeOptions, fileOf, getLabel, getNamespace } from '../../kubectl/options'
+import { KubeOptions, getFileFromArgv, getLabel, getNamespace } from '../../kubectl/options'
 
 import status from './status'
 import handleErrors from './errors'
@@ -33,7 +33,12 @@ const debug = Debug('plugin-kubectl/controller/client/direct/delete')
 
 export default async function deleteDirect(args: Arguments<KubeOptions>, _kind?: Promise<Explained>) {
   // For now, we only handle delete-by-name
-  if (!fileOf(args) && !getLabel(args) && !args.parsedOptions['dry-run'] && !args.parsedOptions['field-selector']) {
+  if (
+    !getFileFromArgv(args) &&
+    !getLabel(args) &&
+    !args.parsedOptions['dry-run'] &&
+    !args.parsedOptions['field-selector']
+  ) {
     const explainedKind = await (_kind ||
       getKindAndVersion(getCommandFromArgs(args), args, args.argvNoOptions[args.argvNoOptions.indexOf('delete') + 1]))
     const { kind } = explainedKind

plugins/plugin-kubectl/src/controller/client/direct/delete.ts

@@ -26,9 +26,8 @@ import { toKuiTable, withNotFound } from '../../../lib/view/formatTable'
 import { doStatus } from '../../kubectl/status'
 import {
   KubeOptions,
-  fileOf,
   formatOf,
-  kustomizeOf,
+  getFileFromArgv,
   getNamespace,
   isEntityFormat,
   isTableRequest,
@@ -130,7 +129,7 @@ export async function get(
   format: string,
   args: Arguments<KubeOptions>
 ) {
-  const isFileRequest = fileOf(args) || kustomizeOf(args)
+  const isFileRequest = getFileFromArgv(args)
   const isEntityAndNameFormat = isEntityFormat(format) || format === 'name'
 
   /** 1. file request with table output, e.g. `kubectl get -f` and `kubectl get -k` */

plugins/plugin-kubectl/src/controller/client/direct/get.ts

@@ -45,6 +45,11 @@ export function urlFormatterFor(
   // e.g. "apis/apps/v1" for deployments
   const apiOnPath = apiOnPathFor(version)
 
+  if (!namespace) {
+    console.error('Internal oddity with namespace, falling back on "default", and was given:', namespace)
+    namespace = 'default'
+  }
+
   // a bit complex: "kubectl get ns", versus "kubectl get ns foo"
   // the "which" is "foo" in the second case
   const namespaceOnPath = isForAllNamespaces(parsedOptions)

plugins/plugin-kubectl/src/controller/client/direct/url.ts

@@ -66,7 +66,7 @@ export async function getAllContexts({ REPL }: { REPL: REPLType }): Promise<Kube
   return (await REPL.rexec<KubeContext[]>('contexts')).content
 }
 
-export async function getCurrentContextName({ REPL }: { REPL: REPLType }) {
+export async function getCurrentContextName({ REPL }: { REPL: REPLType }): Promise<string> {
   const context = await REPL.qexec<string>('kubectl config current-context')
   return context ? context.trim() : context
 }
@@ -75,20 +75,30 @@ export async function getCurrentContextName({ REPL }: { REPL: REPLType }) {
 let currentDefaultNamespaceCache: string
 onKubectlConfigChangeEvents((type, namespace) => {
   if (type === 'SetNamespaceOrContext') {
-    if (typeof namespace === 'string') {
+    if (typeof namespace === 'string' && namespace.length > 0) {
       currentDefaultNamespaceCache = namespace
     } else {
       // invalidate cache
       currentDefaultNamespaceCache = undefined
     }
   }
 })
-export async function getCurrentDefaultNamespace({ REPL }: { REPL: REPLType }) {
+export async function getCurrentDefaultNamespace({ REPL }: { REPL: REPLType }): Promise<string> {
   if (currentDefaultNamespaceCache) {
     return currentDefaultNamespaceCache
   }
 
-  const ns = await REPL.qexec<string>(`kubectl config view --minify --output "jsonpath={..namespace}"`)
+  const cmdline = `kubectl config view --minify --output "jsonpath={..namespace}"`
+  const ns = await REPL.qexec<string>(cmdline)
+    .then(ns => {
+      if (typeof ns !== 'string' || ns.length === 0) {
+        // e.g. microk8s
+        console.error('Suspicious return value for current namespace', ns, cmdline)
+        return 'default'
+      } else {
+        return ns
+      }
+    })
     .then(ns => {
       currentDefaultNamespaceCache = ns
       return ns
@@ -100,12 +110,7 @@ export async function getCurrentDefaultNamespace({ REPL }: { REPL: REPLType }) {
       return 'default'
     })
 
-  if (typeof ns !== 'string') {
-    // e.g. microk8s
-    return 'default'
-  } else {
-    return ns ? ns.trim() : ns
-  }
+  return ns ? ns.trim() : ns
 }
 
 /**

plugins/plugin-kubectl/src/controller/kubectl/contexts.ts

@@ -35,7 +35,11 @@ const strings = i18n('plugin-kubectl', 'kustomize')
 function prepare(args: Arguments<KubeOptions>): string {
   const idx = args.argvNoOptions.indexOf('kustomize')
   const filepath = args.argvNoOptions[idx + 1]
-  return args.command.replace(new RegExp(filepath, 'g'), expandHomeDir(filepath))
+  if (!filepath) {
+    return args.command
+  } else {
+    return args.command.replace(new RegExp(filepath, 'g'), expandHomeDir(filepath))
+  }
 }
 
 function groupByKind(resources: KubeResource[], rawFull: string): Menu[] {