From 5762d5f20ed2e53f0fc115201aa92b55e447d6e1 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 16 Sep 2020 09:09:18 -0400 Subject: [PATCH 1/2] Add option to filter invalid items when loading a config - Default is "Throw" - Could be later extended to return a list of errors along side the config --- package-lock.json | 6 +- package.json | 2 +- src/config.ts | 31 ++--- src/config_test.ts | 27 +++- src/config_types.ts | 163 ++++++++++++++++--------- testdata/empty-cluster-kubeconfig.yaml | 43 +++++++ testdata/empty-context-kubeconfig.yaml | 43 +++++++ 7 files changed, 236 insertions(+), 79 deletions(-) create mode 100644 testdata/empty-cluster-kubeconfig.yaml create mode 100644 testdata/empty-context-kubeconfig.yaml diff --git a/package-lock.json b/package-lock.json index 38fe3f9b0b..d8813d112c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3352,9 +3352,9 @@ } }, "underscore": { - "version": "1.9.1", - "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.9.1.tgz", - "integrity": "sha512-5/4etnCkd9c8gwgowi5/om/mYO5ajCaOgdzj/oW+0eQV9WxKBDZw5+ycmKmeaTXjInS/W0BzpGLo2xR2aBwZdg==" + "version": "1.11.0", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.11.0.tgz", + "integrity": "sha512-xY96SsN3NA461qIRKZ/+qox37YXPtSBswMGfiNptr+wrt6ds4HaMw23TP612fEyGekRE6LNRiLYr/aqbHXNedw==" }, "universalify": { "version": "0.1.2", diff --git a/package.json b/package.json index 754c0f956c..df8ad8a508 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "tar": "^6.0.2", "tmp-promise": "^3.0.2", "tslib": "^1.9.3", - "underscore": "^1.9.1", + "underscore": "^1.11.0", "ws": "^7.3.1" }, "devDependencies": { diff --git a/src/config.ts b/src/config.ts index e190aeed81..868a23b332 100644 --- a/src/config.ts +++ b/src/config.ts @@ -13,6 +13,7 @@ import { Authenticator } from './auth'; import { CloudAuth } from './cloud_auth'; import { Cluster, + ConfigOptions, Context, exportCluster, exportContext, @@ -31,9 +32,9 @@ function fileExists(filepath: string): boolean { try { fs.accessSync(filepath); return true; - // tslint:disable-next-line:no-empty - } catch (ignore) {} - return false; + } catch (ignore) { + return false; + } } export class KubeConfig { @@ -121,9 +122,9 @@ export class KubeConfig { return findObject(this.users, name, 'user'); } - public loadFromFile(file: string) { + public loadFromFile(file: string, opts?: Partial) { const rootDirectory = path.dirname(file); - this.loadFromString(fs.readFileSync(file, 'utf8')); + this.loadFromString(fs.readFileSync(file, 'utf8'), opts); this.makePathsAbsolute(rootDirectory); } @@ -155,11 +156,11 @@ export class KubeConfig { } } - public loadFromString(config: string) { - const obj = yaml.safeLoad(config) as any; - this.clusters = newClusters(obj.clusters); - this.contexts = newContexts(obj.contexts); - this.users = newUsers(obj.users); + public loadFromString(config: string, opts?: Partial) { + const obj = yaml.safeLoad(config); + this.clusters = newClusters(obj.clusters, opts); + this.contexts = newContexts(obj.contexts, opts); + this.users = newUsers(obj.users, opts); this.currentContext = obj['current-context']; } @@ -279,13 +280,13 @@ export class KubeConfig { this.contexts.push(ctx); } - public loadFromDefault() { + public loadFromDefault(opts?: Partial) { if (process.env.KUBECONFIG && process.env.KUBECONFIG.length > 0) { const files = process.env.KUBECONFIG.split(path.delimiter); - this.loadFromFile(files[0]); + this.loadFromFile(files[0], opts); for (let i = 1; i < files.length; i++) { const kc = new KubeConfig(); - kc.loadFromFile(files[i]); + kc.loadFromFile(files[i], opts); this.mergeConfig(kc); } return; @@ -294,7 +295,7 @@ export class KubeConfig { if (home) { const config = path.join(home, '.kube', 'config'); if (fileExists(config)) { - this.loadFromFile(config); + this.loadFromFile(config, opts); return; } } @@ -303,7 +304,7 @@ export class KubeConfig { try { const result = execa.sync('wsl.exe', ['cat', shelljs.homedir() + '/.kube/config']); if (result.code === 0) { - this.loadFromString(result.stdout); + this.loadFromString(result.std, opts); return; } } catch (err) { diff --git a/src/config_test.ts b/src/config_test.ts index 59c2e8b24d..c04fb96903 100644 --- a/src/config_test.ts +++ b/src/config_test.ts @@ -12,7 +12,7 @@ import { fs } from 'mock-fs'; import * as os from 'os'; import { CoreV1Api } from './api'; import { bufferFromFileOrString, findHomeDir, findObject, KubeConfig, makeAbsolutePath } from './config'; -import { Cluster, newClusters, newContexts, newUsers, User } from './config_types'; +import { Cluster, newClusters, newContexts, newUsers, User, ActionOnInvalid } from './config_types'; import { isUndefined } from 'util'; const kcFileName = 'testdata/kubeconfig.yaml'; @@ -22,6 +22,8 @@ const kcDupeContext = 'testdata/kubeconfig-dupe-context.yaml'; const kcDupeUser = 'testdata/kubeconfig-dupe-user.yaml'; const kcNoUserFileName = 'testdata/empty-user-kubeconfig.yaml'; +const kcInvalidContextFileName = 'testdata/empty-context-kubeconfig.yaml'; +const kcInvalidClusterFileName = 'testdata/empty-cluster-kubeconfig.yaml'; /* tslint:disable: no-empty */ describe('Config', () => {}); @@ -192,9 +194,26 @@ describe('KubeConfig', () => { validateFileLoad(kc); }); it('should fail to load a missing kubeconfig file', () => { - // TODO: make the error check work - // let kc = new KubeConfig(); - // expect(kc.loadFromFile("missing.yaml")).to.throw(); + const kc = new KubeConfig(); + expect(kc.loadFromFile.bind('missing.yaml')).to.throw(); + }); + + describe('filter vs throw tests', () => { + it('works for invalid users', () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcNoUserFileName, { onInvalidEntry: ActionOnInvalid.FILTER }); + expect(kc.getUsers().length).to.be.eq(2); + }); + it('works for invalid contexts', () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcInvalidContextFileName, { onInvalidEntry: ActionOnInvalid.FILTER }); + expect(kc.getContexts().length).to.be.eq(2); + }); + it('works for invalid clusters', () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcInvalidClusterFileName, { onInvalidEntry: ActionOnInvalid.FILTER }); + expect(kc.getClusters().length).to.be.eq(1); + }); }); }); diff --git a/src/config_types.ts b/src/config_types.ts index d31108b564..c54ca20a3b 100644 --- a/src/config_types.ts +++ b/src/config_types.ts @@ -1,5 +1,20 @@ import * as fs from 'fs'; -import * as u from 'underscore'; +import * as _ from 'underscore'; + +export enum ActionOnInvalid { + THROW = 'throw', + FILTER = 'filter', +} + +export interface ConfigOptions { + onInvalidEntry: ActionOnInvalid; +} + +function defaultNewConfigOptions(): ConfigOptions { + return { + onInvalidEntry: ActionOnInvalid.THROW, + }; +} export interface Cluster { readonly name: string; @@ -9,8 +24,10 @@ export interface Cluster { readonly skipTLSVerify: boolean; } -export function newClusters(a: any): Cluster[] { - return u.map(a, clusterIterator()); +export function newClusters(a: any, opts?: Partial): Cluster[] { + const options = Object.assign(defaultNewConfigOptions(), opts || {}); + + return _.compact(_.map(a, clusterIterator(options.onInvalidEntry))); } export function exportCluster(cluster: Cluster): any { @@ -25,24 +42,34 @@ export function exportCluster(cluster: Cluster): any { }; } -function clusterIterator(): u.ListIterator { - return (elt: any, i: number, list: u.List): Cluster => { - if (!elt.name) { - throw new Error(`clusters[${i}].name is missing`); - } - if (!elt.cluster) { - throw new Error(`clusters[${i}].cluster is missing`); +function clusterIterator(onInvalidEntry: ActionOnInvalid): _.ListIterator { + return (elt: any, i: number, list: _.List): Cluster | null => { + try { + if (!elt.name) { + throw new Error(`clusters[${i}].name is missing`); + } + if (!elt.cluster) { + throw new Error(`clusters[${i}].cluster is missing`); + } + if (!elt.cluster.server) { + throw new Error(`clusters[${i}].cluster.server is missing`); + } + return { + caData: elt.cluster['certificate-authority-data'], + caFile: elt.cluster['certificate-authority'], + name: elt.name, + server: elt.cluster.server, + skipTLSVerify: elt.cluster['insecure-skip-tls-verify'] === true, + }; + } catch (err) { + switch (onInvalidEntry) { + case ActionOnInvalid.FILTER: + return null; + default: + case ActionOnInvalid.THROW: + throw err; + } } - if (!elt.cluster.server) { - throw new Error(`clusters[${i}].cluster.server is missing`); - } - return { - caData: elt.cluster['certificate-authority-data'], - caFile: elt.cluster['certificate-authority'], - name: elt.name, - server: elt.cluster.server, - skipTLSVerify: elt.cluster['insecure-skip-tls-verify'] === true, - }; }; } @@ -59,8 +86,10 @@ export interface User { readonly password?: string; } -export function newUsers(a: any): User[] { - return u.map(a, userIterator()); +export function newUsers(a: any, opts?: Partial): User[] { + const options = Object.assign(defaultNewConfigOptions(), opts || {}); + + return _.compact(_.map(a, userIterator(options.onInvalidEntry))); } export function exportUser(user: User): any { @@ -80,23 +109,33 @@ export function exportUser(user: User): any { }; } -function userIterator(): u.ListIterator { - return (elt: any, i: number, list: u.List): User => { - if (!elt.name) { - throw new Error(`users[${i}].name is missing`); +function userIterator(onInvalidEntry: ActionOnInvalid): _.ListIterator { + return (elt: any, i: number, list: _.List): User | null => { + try { + if (!elt.name) { + throw new Error(`users[${i}].name is missing`); + } + return { + authProvider: elt.user ? elt.user['auth-provider'] : null, + certData: elt.user ? elt.user['client-certificate-data'] : null, + certFile: elt.user ? elt.user['client-certificate'] : null, + exec: elt.user ? elt.user.exec : null, + keyData: elt.user ? elt.user['client-key-data'] : null, + keyFile: elt.user ? elt.user['client-key'] : null, + name: elt.name, + token: findToken(elt.user), + password: elt.user ? elt.user.password : null, + username: elt.user ? elt.user.username : null, + }; + } catch (err) { + switch (onInvalidEntry) { + case ActionOnInvalid.FILTER: + return null; + default: + case ActionOnInvalid.THROW: + throw err; + } } - return { - authProvider: elt.user ? elt.user['auth-provider'] : null, - certData: elt.user ? elt.user['client-certificate-data'] : null, - certFile: elt.user ? elt.user['client-certificate'] : null, - exec: elt.user ? elt.user.exec : null, - keyData: elt.user ? elt.user['client-key-data'] : null, - keyFile: elt.user ? elt.user['client-key'] : null, - name: elt.name, - token: findToken(elt.user), - password: elt.user ? elt.user.password : null, - username: elt.user ? elt.user.username : null, - }; }; } @@ -118,8 +157,10 @@ export interface Context { readonly namespace?: string; } -export function newContexts(a: any): Context[] { - return u.map(a, contextIterator()); +export function newContexts(a: any, opts?: Partial): Context[] { + const options = Object.assign(defaultNewConfigOptions(), opts || {}); + + return _.compact(_.map(a, contextIterator(options.onInvalidEntry))); } export function exportContext(ctx: Context): any { @@ -129,22 +170,32 @@ export function exportContext(ctx: Context): any { }; } -function contextIterator(): u.ListIterator { - return (elt: any, i: number, list: u.List): Context => { - if (!elt.name) { - throw new Error(`contexts[${i}].name is missing`); - } - if (!elt.context) { - throw new Error(`contexts[${i}].context is missing`); - } - if (!elt.context.cluster) { - throw new Error(`contexts[${i}].context.cluster is missing`); +function contextIterator(onInvalidEntry: ActionOnInvalid): _.ListIterator { + return (elt: any, i: number, list: _.List): Context | null => { + try { + if (!elt.name) { + throw new Error(`contexts[${i}].name is missing`); + } + if (!elt.context) { + throw new Error(`contexts[${i}].context is missing`); + } + if (!elt.context.cluster) { + throw new Error(`contexts[${i}].context.cluster is missing`); + } + return { + cluster: elt.context.cluster, + name: elt.name, + user: elt.context.user || undefined, + namespace: elt.context.namespace || undefined, + }; + } catch (err) { + switch (onInvalidEntry) { + case ActionOnInvalid.FILTER: + return null; + default: + case ActionOnInvalid.THROW: + throw err; + } } - return { - cluster: elt.context.cluster, - name: elt.name, - user: elt.context.user || undefined, - namespace: elt.context.namespace || undefined, - }; }; } diff --git a/testdata/empty-cluster-kubeconfig.yaml b/testdata/empty-cluster-kubeconfig.yaml new file mode 100644 index 0000000000..c4bcc1cbc1 --- /dev/null +++ b/testdata/empty-cluster-kubeconfig.yaml @@ -0,0 +1,43 @@ +apiVersion: v1 +clusters: +- cluster: + certificate-authority-data: Q0FEQVRB + server: http://example.com + name: "" +- cluster: + certificate-authority-data: Q0FEQVRBMg== + server: http://example2.com + insecure-skip-tls-verify: true + name: cluster2 + +contexts: +- context: + cluster: cluster1 + user: user1 + name: context1 +- context: + cluster: cluster2 + namespace: namespace2 + user: user2 + name: context2 +- context: + cluster: cluster2 + user: user3 + name: passwd + +current-context: context2 +kind: Config +preferences: {} +users: +- name: user1 + user: + client-certificate-data: VVNFUl9DQURBVEE= + client-key-data: VVNFUl9DS0RBVEE= +- name: user2 + user: + client-certificate-data: VVNFUjJfQ0FEQVRB + client-key-data: VVNFUjJfQ0tEQVRB +- name: user3 + user: + username: foo + password: bar diff --git a/testdata/empty-context-kubeconfig.yaml b/testdata/empty-context-kubeconfig.yaml new file mode 100644 index 0000000000..8d75378c99 --- /dev/null +++ b/testdata/empty-context-kubeconfig.yaml @@ -0,0 +1,43 @@ +apiVersion: v1 +clusters: +- cluster: + certificate-authority-data: Q0FEQVRB + server: http://example.com + name: cluster1 +- cluster: + certificate-authority-data: Q0FEQVRBMg== + server: http://example2.com + insecure-skip-tls-verify: true + name: cluster2 + +contexts: +- context: + cluster: "" + user: "" + name: "" +- context: + cluster: cluster2 + namespace: namespace2 + user: user2 + name: context2 +- context: + cluster: cluster2 + user: user3 + name: passwd + +current-context: context2 +kind: Config +preferences: {} +users: +- name: user1 + user: + client-certificate-data: VVNFUl9DQURBVEE= + client-key-data: VVNFUl9DS0RBVEE= +- name: user2 + user: + client-certificate-data: VVNFUjJfQ0FEQVRB + client-key-data: VVNFUjJfQ0tEQVRB +- name: user3 + user: + username: foo + password: bar From ce450c85e1608ead48611351750c1fc152c25236 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 16 Sep 2020 09:32:14 -0400 Subject: [PATCH 2/2] revert unnecessary dep ver bump --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index d8813d112c..38fe3f9b0b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3352,9 +3352,9 @@ } }, "underscore": { - "version": "1.11.0", - "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.11.0.tgz", - "integrity": "sha512-xY96SsN3NA461qIRKZ/+qox37YXPtSBswMGfiNptr+wrt6ds4HaMw23TP612fEyGekRE6LNRiLYr/aqbHXNedw==" + "version": "1.9.1", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.9.1.tgz", + "integrity": "sha512-5/4etnCkd9c8gwgowi5/om/mYO5ajCaOgdzj/oW+0eQV9WxKBDZw5+ycmKmeaTXjInS/W0BzpGLo2xR2aBwZdg==" }, "universalify": { "version": "0.1.2", diff --git a/package.json b/package.json index df8ad8a508..754c0f956c 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "tar": "^6.0.2", "tmp-promise": "^3.0.2", "tslib": "^1.9.3", - "underscore": "^1.11.0", + "underscore": "^1.9.1", "ws": "^7.3.1" }, "devDependencies": {