Skip to content
This repository was archived by the owner on Apr 13, 2020. It is now read-only.

Commit c3c06f9

Browse files
dennisseahyradsmikham
dennisseah
and
yradsmikham
authored
[REFACTORING] making code more compact (#192)
* replace any with interface * correct the type definition for IVariableGroupDataVariable[] * [REFACTOR] making code compact * add test to create-variable-group command * breaking larger function in scaffold class into smaller one, so we can add tests * simplier commander interface * using type guard in hasValue function * incorporate Evan's feedback * fix jsdoc * fixed the problem where definition.yaml file is not created Co-authored-by: Yvonne Radsmikham <yvonne.radsmikham@gmail.com>
1 parent f0b3a1d commit c3c06f9

File tree

11 files changed

+653
-328
lines changed

11 files changed

+653
-328
lines changed

‎src/commands/infra/scaffold.test.ts‎

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,22 @@
1+
jest.mock("./generate");
2+
3+
import uuid = require("uuid");
4+
import { disableVerboseLogging, enableVerboseLogging } from "../../logger";
5+
import { validateRemoteSource } from "./generate";
16
import {
2-
disableVerboseLogging,
3-
enableVerboseLogging,
4-
logger
5-
} from "../../logger";
6-
import { generateClusterDefinition, parseVariablesTf } from "./scaffold";
7+
constructSource,
8+
execute,
9+
generateClusterDefinition,
10+
parseVariablesTf,
11+
validateValues
12+
} from "./scaffold";
13+
14+
const mockYaml = {
15+
azure_devops: {
16+
access_token: "token123",
17+
infra_repository: "repoABC"
18+
}
19+
};
720

821
beforeAll(() => {
922
enableVerboseLogging();
@@ -13,6 +26,55 @@ afterAll(() => {
1326
disableVerboseLogging();
1427
});
1528

29+
describe("test validate function", () => {
30+
it("empty config yaml", () => {
31+
const result = validateValues(
32+
{},
33+
{
34+
name: uuid(),
35+
source: "http://example@host",
36+
template: uuid(),
37+
version: uuid()
38+
}
39+
);
40+
expect(result).toBe(true); // because source if defined
41+
});
42+
it("empty config yaml and source is missing", () => {
43+
const result = validateValues(
44+
{},
45+
{
46+
name: uuid(),
47+
template: uuid(),
48+
version: uuid()
49+
}
50+
);
51+
expect(result).toBe(false); // because source if defined
52+
});
53+
});
54+
55+
describe("test constructSource function", () => {
56+
it("validate result", () => {
57+
const source = constructSource(mockYaml);
58+
expect(source).toBe("https://spk:token123@repoABC");
59+
});
60+
});
61+
62+
describe("test execute function", () => {
63+
it("missing config yaml", async () => {
64+
const exitFn = jest.fn();
65+
await execute({}, {}, exitFn);
66+
expect(exitFn).toBeCalledTimes(1);
67+
expect(exitFn.mock.calls).toEqual([[1]]);
68+
});
69+
it("missing opt ", async () => {
70+
(validateRemoteSource as jest.Mock).mockReturnValue(true);
71+
const exitFn = jest.fn();
72+
await execute(mockYaml, {}, exitFn);
73+
expect(exitFn).toBeCalledTimes(1);
74+
expect(exitFn.mock.calls).toEqual([[1]]);
75+
});
76+
});
77+
1678
describe("Validate parsing of sample variables.tf file", () => {
1779
test("Validate that a variables.tf sample can be parsed into an object", async () => {
1880
const sampleVarTf =

‎src/commands/infra/scaffold.ts‎

Lines changed: 99 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,88 @@ import yaml from "js-yaml";
55
import path from "path";
66
import { Config } from "../../config";
77
import { logger } from "../../logger";
8+
import { IConfigYaml } from "../../types";
89
import { validateRemoteSource } from "./generate";
910
import * as infraCommon from "./infra_common";
1011

12+
const DEFINITION_YAML = "definition.yaml";
13+
14+
interface ICommandOptions {
15+
[key: string]: string;
16+
}
17+
18+
export const validateValues = (
19+
config: IConfigYaml,
20+
opts: ICommandOptions
21+
): boolean => {
22+
if (
23+
!config.azure_devops ||
24+
!config.azure_devops.access_token ||
25+
!config.azure_devops.infra_repository
26+
) {
27+
logger.warn(
28+
"The infrastructure repository containing the remote terraform template repo and access token was not specified. Checking passed arguments."
29+
);
30+
31+
if (!opts.name || !opts.source || !opts.version || !opts.template) {
32+
logger.error(
33+
"You must specify each of the variables 'name', 'source', 'version', 'template' in order to scaffold out a deployment."
34+
);
35+
return false;
36+
}
37+
logger.info(
38+
"All required options are configured via command line for scaffolding, expecting public remote repository for terraform templates or PAT embedded in source URL."
39+
);
40+
}
41+
return true;
42+
};
43+
44+
// Construct the source based on the the passed configurations of spk-config.yaml
45+
export const constructSource = (config: IConfigYaml) => {
46+
const devops = config.azure_devops!;
47+
const source = `https://spk:${devops.access_token}@${devops.infra_repository}`;
48+
logger.info(
49+
`Infrastructure repository detected from initialized spk-config.yaml.`
50+
);
51+
return source;
52+
};
53+
54+
export const execute = async (
55+
config: IConfigYaml,
56+
opts: ICommandOptions,
57+
exitFn: (status: number) => void
58+
) => {
59+
if (!validateValues(config, opts)) {
60+
exitFn(1);
61+
} else {
62+
opts.source = opts.source || constructSource(config);
63+
64+
try {
65+
/* scaffoldDefinition will take in a definition object with a
66+
null configuration. Hence, the first index is "" */
67+
const scaffoldDefinition = ["", opts.source, opts.template, opts.version];
68+
const sourceFolder = await infraCommon.repoCloneRegex(opts.source);
69+
const sourcePath = path.join(infraCommon.spkTemplatesPath, sourceFolder);
70+
await validateRemoteSource(scaffoldDefinition);
71+
await copyTfTemplate(
72+
path.join(sourcePath, opts.template),
73+
opts.name,
74+
false
75+
);
76+
await validateVariablesTf(
77+
path.join(sourcePath, opts.template, "variables.tf")
78+
);
79+
await scaffold(opts.name, opts.source, opts.version, opts.template);
80+
await removeTemplateFiles(opts.name);
81+
exitFn(0);
82+
} catch (err) {
83+
logger.error("Error occurred while generating scaffold");
84+
logger.error(err);
85+
exitFn(1);
86+
}
87+
}
88+
};
89+
1190
/**
1291
* Adds the init command to the commander command object
1392
*
@@ -31,67 +110,9 @@ export const scaffoldCommandDecorator = (command: commander.Command): void => {
31110
"-t, --template <path to variables.tf> ",
32111
"Location of the variables.tf for the terraform deployment"
33112
)
34-
.action(async opts => {
35-
try {
36-
const config = Config();
37-
if (
38-
!config.azure_devops ||
39-
!config.azure_devops.access_token ||
40-
!config.azure_devops.infra_repository
41-
) {
42-
logger.warn(
43-
"The infrastructure repository containing the remote terraform template repo and access token was not specified. Checking passed arguments."
44-
);
45-
if (opts.name && opts.source && opts.version && opts.template) {
46-
logger.info(
47-
"All required options are configured via command line for scaffolding, expecting public remote repository for terraform templates or PAT embedded in source URL."
48-
);
49-
} else {
50-
logger.error(
51-
"You must specify each of the variables 'name', 'source', 'version', 'template' in order to scaffold out a deployment."
52-
);
53-
}
54-
} else {
55-
if (!opts.source) {
56-
// Construct the source based on the the passed configurations of spk-config.yaml
57-
opts.source =
58-
"https://spk:" +
59-
config.azure_devops.access_token +
60-
"@" +
61-
config.azure_devops.infra_repository;
62-
logger.info(
63-
`Infrastructure repository detected from initialized spk-config.yaml.`
64-
);
65-
}
66-
}
67-
/* scaffoldDefinition will take in a definition object with a
68-
null configuration. Hence, the first index is "" */
69-
const scaffoldDefinition = [
70-
"",
71-
opts.source,
72-
opts.template,
73-
opts.version
74-
];
75-
const sourceFolder = await infraCommon.repoCloneRegex(opts.source);
76-
const sourcePath = path.join(
77-
infraCommon.spkTemplatesPath,
78-
sourceFolder
79-
);
80-
await validateRemoteSource(scaffoldDefinition);
81-
await copyTfTemplate(
82-
path.join(sourcePath, opts.template),
83-
opts.name,
84-
false
85-
);
86-
await validateVariablesTf(
87-
path.join(sourcePath, opts.template, "variables.tf")
88-
);
89-
await scaffold(opts.name, opts.source, opts.version, opts.template);
90-
await removeTemplateFiles(opts.name);
91-
} catch (err) {
92-
logger.error("Error occurred while generating scaffold");
93-
logger.error(err);
94-
}
113+
.action(async (opts: ICommandOptions) => {
114+
const config = Config();
115+
await execute(config, opts, process.exit);
95116
});
96117
};
97118

@@ -111,7 +132,7 @@ export const validateVariablesTf = async (
111132
return false;
112133
}
113134
logger.info(
114-
`Terraform variables.tf file found. Attempting to generate definition.yaml file.`
135+
`Terraform variables.tf file found. Attempting to generate ${DEFINITION_YAML} file.`
115136
);
116137
} catch (_) {
117138
logger.error(`Unable to validate Terraform variables.tf.`);
@@ -207,18 +228,18 @@ export const copyTfTemplate = async (
207228
*
208229
* @param envPath path so the directory of Terraform templates
209230
*/
210-
export const removeTemplateFiles = async (envPath: string): Promise<void> => {
231+
export const removeTemplateFiles = async (envPath: string) => {
211232
// Remove template files after parsing
212-
fs.readdir(envPath, (err, files) => {
213-
if (err) {
214-
throw err;
215-
}
216-
for (const file of files) {
217-
if (file !== "definition.yaml") {
218-
fs.unlinkSync(path.join(envPath, file));
219-
}
220-
}
221-
});
233+
try {
234+
const files = await fs.readdirSync(envPath);
235+
files
236+
.filter(f => f !== DEFINITION_YAML)
237+
.forEach(f => {
238+
fs.unlinkSync(path.join(envPath, f));
239+
});
240+
} catch (e) {
241+
logger.error(`cannot read ${envPath}`);
242+
}
222243
};
223244

224245
/**
@@ -371,15 +392,12 @@ export const scaffold = async (
371392
);
372393
const definitionYaml = yaml.safeDump(baseDef);
373394
if (baseDef) {
374-
fs.mkdir(name, (e: any) => {
375-
const confPath: string = path.format({
376-
base: "definition.yaml",
377-
dir: name,
378-
root: "/ignored"
379-
});
380-
fs.writeFileSync(confPath, definitionYaml, "utf8");
381-
return true;
395+
const confPath: string = path.format({
396+
base: DEFINITION_YAML,
397+
dir: name,
398+
root: "/ignored"
382399
});
400+
fs.writeFileSync(confPath, definitionYaml, "utf8");
383401
} else {
384402
logger.error(`Unable to generate cluster definition.`);
385403
}

‎src/commands/project/create-variable-group.decorator.json‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"command": "create-variable-group <variable-group-name>",
3+
"alias": "cvg",
4+
"description": "Create a new variable group in Azure DevOps project with specific variables (ACR name, HLD Repo name, Personal Access Token, Service Principal id, Service Principal password, and Azure AD tenant id)",
5+
"options": [
6+
{
7+
"arg": "-r, --registry-name <registry-name>",
8+
"description": "The name of the existing Azure Container Registry.",
9+
"required": true
10+
},
11+
{
12+
"arg": "-d, --hld-repo-url <hld-repo-url>",
13+
"description": "The high level definition (HLD) git repo url; falls back to azure_devops.org in spk config.",
14+
"required": true
15+
},
16+
{
17+
"arg": "-u, --service-principal-id <service-principal-id>",
18+
"description": "Azure service principal id with `contributor` role in Azure Container Registry.",
19+
"required": true
20+
},
21+
{
22+
"arg": "-p, --service-principal-password <service-principal-password>",
23+
"description": "The Azure service principal password.",
24+
"required": true
25+
},
26+
{
27+
"arg": "-t, --tenant <tenant>",
28+
"description": "The Azure AD tenant id of service principal.",
29+
"required": true
30+
},
31+
{
32+
"arg": "--org-name <organization-name>",
33+
"description": "Azure DevOps organization name; falls back to azure_devops.org in spk config.",
34+
"required": true
35+
},
36+
{
37+
"arg": "--project <project>",
38+
"description": "Azure DevOps project name; falls back to azure_devops.project in spk config.",
39+
"required": true
40+
},
41+
{
42+
"arg": "--personal-access-token <personal-access-token>",
43+
"description": "Azure DevOps Personal access token; falls back to azure_devops.access_token in spk config.",
44+
"required": true
45+
}
46+
]
47+
}

0 commit comments

Comments
 (0)