Skip to content

Commit a6ce136

Browse files
prachirpfrankfarzan
prachirp
authored and
frankfarzan
committed
Address review comments
1 parent bc5b5f2 commit a6ce136

File tree

4 files changed

+81
-89
lines changed

4 files changed

+81
-89
lines changed

‎tests/e2e.sh‎

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -133,40 +133,6 @@ assert_dir_exists payments-dev
133133
assert_dir_exists payments-prod
134134
grep -q allowPrivilegeEscalation podsecuritypolicy_psp.yaml
135135

136-
helm_testcase "node_helm_template_command_line_args"
137-
node "${DIST}"/helm_template_run.js -i /dev/null -d name=my-chart -d chart_path=charts/bitnami/redis/ >out.yaml
138-
assert_contains_string out.yaml "my-chart"
139-
140-
helm_testcase "node_helm_template_function_config_args"
141-
cat >fc.yaml <<EOF
142-
apiVersion: v1
143-
kind: ConfigMap
144-
metadata:
145-
name: my-config
146-
annotations:
147-
config.k8s.io/function: |
148-
container:
149-
image: gcr.io/kpt-functions/helm-template
150-
config.kubernetes.io/local-config: "true"
151-
data:
152-
name: my-chart
153-
chart_path: charts/bitnami/redis
154-
EOF
155-
node "${DIST}"/helm_template_run.js -i /dev/null -f fc.yaml >out.yaml
156-
assert_contains_string out.yaml "my-chart"
157-
158-
helm_testcase "node_helm_template_undefined_args"
159-
node "${DIST}"/helm_template_run.js -i /dev/null 2>err.txt || true
160-
assert_contains_string err.txt "Error: functionConfig expected, instead undefined"
161-
162-
helm_testcase "node_helm_template_transform_funct"
163-
node "${DIST}"/read_yaml_run.js -i /dev/null -d source_dir="$(pwd)"/example-configs |
164-
node "${DIST}"/helm_template_run.js -i /dev/null -d name=my-chart -d chart_path=charts/bitnami/redis/ |
165-
node "${DIST}"/write_yaml_run.js -o /dev/null -d sink_dir="$(pwd)"/example-configs -d overwrite=true
166-
assert_dir_exists example-configs/shipping-dev
167-
assert_dir_exists example-configs/default
168-
grep -q "name: my-chart-redis-master" example-configs/default/service_my-chart-redis-master.yaml
169-
170136
############################
171137
# Docker Tests
172138
############################
@@ -211,14 +177,52 @@ assert_dir_exists payments-dev
211177
assert_dir_exists payments-prod
212178
grep -q allowPrivilegeEscalation podsecuritypolicy_psp.yaml
213179

180+
helm_testcase "docker_helm_template_undefined_args"
181+
docker run -u "$(id -u)" -v "$(pwd)/${CHARTS_SRC}":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null 2>err.txt || true
182+
assert_contains_string err.txt "Error: functionConfig expected, instead undefined"
183+
184+
helm_testcase "docker_helm_template_empty_fc"
185+
cat >fc.yaml <<EOF
186+
apiVersion: v1
187+
kind: ConfigMap
188+
metadata:
189+
name: empty-config
190+
annotations:
191+
config.k8s.io/function: |
192+
container:
193+
image: gcr.io/kpt-functions/helm-template
194+
config.kubernetes.io/local-config: "true"
195+
data:
196+
EOF
197+
docker run -u "$(id -u)" -v "$(pwd)":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null -f /source/fc.yaml 2>err.txt || true
198+
assert_contains_string err.txt "Error: functionConfig expected to contain data, instead empty"
199+
200+
helm_testcase "docker_helm_template_invalid_fc"
201+
cat >fc.yaml <<EOF
202+
apiVersion: v1
203+
kind: ConfigMap
204+
metadata:
205+
name: invalid-config
206+
annotations:
207+
config.k8s.io/function: |
208+
container:
209+
image: gcr.io/kpt-functions/helm-template
210+
config.kubernetes.io/local-config: "true"
211+
data:
212+
name: invalid-fc
213+
chart_path: /path/to/helm/chart
214+
EOF
215+
docker run -u "$(id -u)" -v "$(pwd)":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null -f /source/fc.yaml >out.yaml || true
216+
assert_contains_string out.yaml "${HELM_ERROR_SNIPPET}"
217+
218+
helm_testcase "docker_helm_template_too_few_args"
219+
docker run -u "$(id -u)" -v "$(pwd)/${CHARTS_SRC}":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null -d name=too-few-args >out.yaml || true
220+
assert_contains_string out.yaml "${HELM_ERROR_SNIPPET}"
221+
214222
helm_testcase "docker_helm_template_expected_args"
215223
docker run -u "$(id -u)" -v "$(pwd)/${CHARTS_SRC}":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null -d name=expected-args -d chart_path=/source/redis >out.yaml
216224
assert_contains_string out.yaml "expected-args"
217225

218-
helm_testcase "docker_helm_template_error_too_few_args"
219-
docker run -u "$(id -u)" -v "$(pwd)/${CHARTS_SRC}":/source gcr.io/kpt-functions/helm-template:"${TAG}" -i /dev/null -d name=too-few-args 2>err.txt || true
220-
assert_contains_string err.txt "${HELM_ERROR_SNIPPET}"
221-
222226
helm_testcase "docker_helm_template_extra_args"
223227
cat >fc.yaml <<EOF
224228
apiVersion: v1

‎ts/demo-functions/build/helm_template.Dockerfile‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
FROM node:10-alpine as builder
22

3+
RUN apk add bash curl git
4+
RUN apk update
5+
6+
ENV HELM_LATEST_VERSION="v3.2.1"
7+
RUN curl -fsSL -o /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz https://get.helm.sh/helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
8+
tar -zxvf /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
9+
mv /linux-amd64/helm /usr/local/bin/helm && \
10+
rm -f /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
11+
rm -rf /linux-amd64
12+
13+
RUN curl -fsSL -o /usr/local/bin/kpt https://storage.googleapis.com/kpt-dev/latest/linux_amd64/kpt && \
14+
chmod +x /usr/local/bin/kpt
15+
316
RUN mkdir -p /home/node/app && \
417
chown -R node:node /home/node/app
518

@@ -22,23 +35,17 @@ RUN npm run build && \
2235

2336
FROM node:10-alpine
2437

25-
RUN apk add bash curl git
26-
RUN apk update
27-
28-
ENV HELM_LATEST_VERSION="v3.2.1"
29-
RUN curl -fsSL -o /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz https://get.helm.sh/helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
30-
tar -zxvf /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
31-
mv /linux-amd64/helm /usr/local/bin/helm && \
32-
rm -f /helm-${HELM_LATEST_VERSION}-linux-amd64.tar.gz && \
33-
rm -rf /linux-amd64
34-
35-
RUN curl -fsSL -o /usr/local/bin/kpt https://storage.googleapis.com/kpt-dev/latest/linux_amd64/kpt && \
36-
chmod +x /usr/local/bin/kpt
37-
38-
ENV PATH /usr/local/bin:$PATH
38+
# Run as non-root user as a best-practices:
39+
# https://github.com/nodejs/docker-node/blob/master/docs/BestPractices.md
40+
USER node
3941

4042
WORKDIR /home/node/app
4143

4244
COPY --from=builder /home/node/app /home/node/app
4345

46+
COPY --from=builder /usr/local/bin /usr/local/bin
47+
48+
ENV PATH /usr/local/bin:$PATH
49+
ENV HELM_PATH_CACHE /tmp
50+
4451
ENTRYPOINT ["node", "/home/node/app/dist/helm_template_run.js"]

‎ts/demo-functions/src/helm_template.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ import {
1919
Configs,
2020
FunctionConfigError,
2121
isKubernetesObject,
22-
ConfigError,
22+
generalResult,
2323
} from 'kpt-functions';
2424
import { spawnSync } from 'child_process';
2525
import { isConfigMap } from './gen/io.k8s.api.core.v1';
2626

27-
const NAME = 'name';
27+
const CHART_NAME = 'name';
2828
const CHART_PATH = 'chart_path';
2929
const VALUES_PATH = '--values';
3030

@@ -34,27 +34,28 @@ export async function helmTemplate(configs: Configs) {
3434
const args = readArguments(configs);
3535
args.unshift('template');
3636

37-
let child;
3837
let error;
3938
try {
40-
child = spawnSync('helm', args);
39+
const child = spawnSync('helm', args);
4140
error = child.stderr;
4241
let objects = safeLoadAll(child.stdout);
4342
objects = objects.filter(o => isKubernetesObject(o));
4443
configs.insert(...objects);
4544
} catch (err) {
46-
throw new ConfigError(err);
45+
configs.addResults(generalResult(err, 'error'));
4746
}
4847
if (error && error.length > 0) {
49-
throw new ConfigError(`Helm template command results in error: ${error}`);
48+
configs.addResults(
49+
generalResult(`Helm template command results in error: ${error}`, 'error')
50+
);
5051
}
5152
}
5253

5354
function readArguments(configs: Configs) {
5455
const args: string[] = [];
5556

5657
// Helm template expects name then chart path then remaining flags
57-
const name = configs.getFunctionConfigValue(NAME);
58+
const name = configs.getFunctionConfigValue(CHART_NAME);
5859
if (name) {
5960
args.push(name);
6061
}
@@ -66,7 +67,7 @@ function readArguments(configs: Configs) {
6667
// Remaining flags
6768
const data = readConfigDataOrThrow(configs);
6869
for (const key in data) {
69-
if (key !== NAME && key !== CHART_PATH) {
70+
if (key !== CHART_NAME && key !== CHART_PATH) {
7071
args.push(key);
7172
args.push(data[key]);
7273
}
@@ -94,15 +95,14 @@ function readConfigDataOrThrow(configs: Configs) {
9495
}
9596

9697
helmTemplate.usage = `
97-
Render chart templates locally using helm template.
98-
Display the output objects as a Kubernetes List. If piped a Kubernetes List in
98+
Render chart templates locally using helm template. If piped a Kubernetes List in
9999
addition to arguments then render the chart objects into the piped list,
100100
overwriting any chart objects that already exist in the list.
101101
102-
Configured using a ConfigMap with keys for ${NAME}, ${CHART_PATH}, and optional helm
102+
Configured using a ConfigMap with keys for ${CHART_NAME}, ${CHART_PATH}, and optional helm
103103
template flags like --values:
104104
105-
${NAME}: Name of helm chart.
105+
${CHART_NAME}: Name of helm chart.
106106
${CHART_PATH}: Chart templates directory.
107107
${VALUES_PATH}: [Optional] Path to values file.
108108
...
@@ -121,7 +121,7 @@ metadata:
121121
image: gcr.io/kpt-functions/helm-template
122122
config.kubernetes.io/local-config: "true"
123123
data:
124-
${NAME}: my-chart
124+
${CHART_NAME}: my-chart
125125
${CHART_PATH}: ../path/to/helm/chart
126126
${VALUES_PATH}: ./values.yaml
127127
`;

‎ts/demo-functions/src/helm_template_test.ts‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,45 +16,26 @@
1616

1717
import { Configs, TestRunner, FunctionConfigError } from 'kpt-functions';
1818
import { helmTemplate } from './helm_template';
19-
import { ConfigMap, Namespace } from './gen/io.k8s.api.core.v1';
19+
import { Namespace } from './gen/io.k8s.api.core.v1';
2020

2121
const RUNNER = new TestRunner(helmTemplate);
2222

2323
describe('helmTemplate', () => {
2424
it('outputs error given undefined function config', async () => {
25-
const input = new Configs([], undefined);
25+
const input = new Configs(undefined, undefined);
2626

2727
await RUNNER.assert(
2828
input,
29-
new Configs([]),
29+
new Configs(undefined),
3030
FunctionConfigError,
3131
'functionConfig expected, instead undefined'
3232
);
3333
});
3434

3535
const namespace = Namespace.named('namespace');
3636
it('outputs error given namespace function config', async () => {
37-
const input = new Configs([], namespace);
37+
const input = new Configs(undefined, namespace);
3838

39-
await RUNNER.assert(input, new Configs([]), Error);
40-
});
41-
42-
const emptyFunctionConfig = ConfigMap.named('empty-config');
43-
emptyFunctionConfig.data = {};
44-
it('outputs helm template error given empty function config', async () => {
45-
const input = new Configs([], emptyFunctionConfig);
46-
47-
await RUNNER.assert(input, new Configs([]), Error);
48-
});
49-
50-
const invalidFunctionConfig = ConfigMap.named('function-config');
51-
invalidFunctionConfig.data = {
52-
name: 'my-chart',
53-
chart_path: '../path/to/helm/chart',
54-
};
55-
it('outputs helm template error given invalid function config', async () => {
56-
const input = new Configs([], invalidFunctionConfig);
57-
58-
await RUNNER.assert(input, new Configs([]), Error);
39+
await RUNNER.assert(input, new Configs(undefined, namespace), Error);
5940
});
6041
});

0 commit comments

Comments
 (0)