make: Suppress symlink warnings on getting k8gb $VERSION #499

somaritane · 2021-05-20T19:08:08Z

This PR suppresses walk.go:74: found symbolic link... message noise during k8gb helm chart $VERSION extraction in Makefile by muting the stderr stream for that command.

NOTE: This change affects only helm show chart chart/k8gb/ command.
Other helm commands used in Makefile are deliberately left intact to not miss possible important helm errors.

Before:

❯ make                        
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/LICENSE resolves to /Users/abti021/src/absaoss/k8gb/LICENSE
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/README.md resolves to /Users/abti021/src/absaoss/k8gb/README.md
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/LICENSE resolves to /Users/abti021/src/absaoss/k8gb/LICENSE
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/README.md resolves to /Users/abti021/src/absaoss/k8gb/README.md
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/LICENSE resolves to /Users/abti021/src/absaoss/k8gb/LICENSE
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/README.md resolves to /Users/abti021/src/absaoss/k8gb/README.md
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/LICENSE resolves to /Users/abti021/src/absaoss/k8gb/LICENSE
walk.go:74: found symbolic link in path: /Users/abti021/src/absaoss/k8gb/chart/k8gb/README.md resolves to /Users/abti021/src/absaoss/k8gb/README.md
demo-failover        Execute failover demo
demo-roundrobin      Execute round-robin demo
...

After:

❯ make                        
demo-failover        Execute failover demo
demo-roundrobin      Execute round-robin demo
...

Signed-off-by: Timofey Ilinykh timofey.ilinykh@absa.africa

Suppress helm "walk.go:74: found symbolic link..." message noise during k8gb helm chart $VERSION extraction in Makefile by muting the shell command stderr. Other helm commands in Makefile are deliberately left intact to not miss possible helm errors. Signed-off-by: Timofey Ilinykh <timofey.ilinykh@absa.africa>

ytsarev · 2021-05-21T07:38:14Z

Makefile

@@ -58,7 +58,7 @@ NO_VALUE ?= no_value
 #		VARIABLES
 ###############################
 PWD ?=  $(shell pwd)
-VERSION ?= $(shell helm show chart chart/k8gb/|awk '/appVersion:/ {print $$2}')
+VERSION ?= $(shell helm show chart chart/k8gb/ 2> /dev/null | awk '/appVersion:/ {print $$2}')


What if stderr contains a valid error? it will be invisible right?

@ytsarev I've tried to minimize the silencing impact only to this command, but yes, errors will be suppressed.
Regarding possible valid errors for this command, I see issues like helm not installed, chart code missing, appVersion missing in the chart, malformed helm command. In all cases, except missing appVersion, the errors are produced, but the execution proceeds with the empty $(VERSION) variable. In the case of missing appVersion, no errors produced at all.

Suggested change

VERSION ?= $(shell helm show chart chart/k8gb/ 2> /dev/null | awk '/appVersion:/ {print $$2}')

VERSION ?= $(shell helm show chart chart/k8gb/ 2>&1 |grep -v 'found symbolic link in path' | awk '/appVersion:/ {print $$2}')

@ytsarev , since awk is "interested" in awkVersion: only, it will suppress the rest of stdout, including the redirected stderr with possible errors.

ytsarev · 2021-05-21T08:57:10Z

@somaritane which version of helm do you use?
Mine looks silent

 make
demo-failover        Execute failover demo
demo-roundrobin      Execute round-robin demo
deploy-candidate     Deploy test k8gb version together with CRs and test apps on top of existing clusters
deploy-full-local-setup Deploy full local multicluster setup (k3d >= 4.2.0)
deploy-gslb-cr       Apply Gslb Custom Resources
deploy-gslb-operator Deploy k8gb operator
deploy-test-apps     Deploy testing workloads
destroy-full-local-setup Destroy full local multicluster setup
dns-tools            Run temporary dnstools pod for debugging DNS issues
help                 Show this help
upgrade-candidate    Upgrade k8gb to the test version on existing clusters

helm version
version.BuildInfo{Version:"v3.3.4", GitCommit:"a61ce5633af99708171414353ed49547cf05013d", GitTreeState:"dirty", GoVersion:"go1.15.2"}

somaritane · 2021-05-21T09:42:33Z

@somaritane which version of helm do you use?
Mine looks silent

@ytsarev mine is v3.5.4 I can downgrade and check.

somaritane · 2021-05-21T09:57:54Z

@somaritane which version of helm do you use?
Mine looks silent

@ytsarev mine is v3.5.4 I can downgrade and check.

v3.3.4 version acts the same for me

ytsarev

Suggested the safer suppression method above

somaritane · 2021-05-21T10:10:24Z

After carefully checking helm issues:

Symlinks produce noise in templated output helm/helm#7019,
Why does helm package follow symlinks? helm/helm#8540
and related security audit report and post:
https://github.com/helm/community/blob/main/security-audit/HLM-01-report.pdf
https://helm.sh/blog/2019-10-30-helm-symlink-security-notice/
I think it would be good to not suppress helm symlink traversing warnings, but instead, replace symlinked License and README files in the k8gb chart directory, regardless of their apparent convenience.

README replacement is already tracked by separate issue #359, same should be done with LICENSE.

I'm going to close the PR.

somaritane requested review from donovanmuller, k0da, kuritka and ytsarev as code owners May 20, 2021 19:08

kuritka approved these changes May 20, 2021

View reviewed changes

ytsarev reviewed May 21, 2021

View reviewed changes

ytsarev requested changes May 21, 2021

View reviewed changes

somaritane closed this May 21, 2021

somaritane deleted the suppress-symlink-walk-messages branch October 14, 2021 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make: Suppress symlink warnings on getting k8gb $VERSION #499

make: Suppress symlink warnings on getting k8gb $VERSION #499

somaritane commented May 20, 2021

ytsarev May 21, 2021

somaritane May 21, 2021 •

edited

Loading

ytsarev May 21, 2021

somaritane May 21, 2021

ytsarev commented May 21, 2021

somaritane commented May 21, 2021

somaritane commented May 21, 2021

ytsarev left a comment

somaritane commented May 21, 2021 •

edited

Loading

	VERSION ?= $(shell helm show chart chart/k8gb/ 2> /dev/null \| awk '/appVersion:/ {print $$2}')
	VERSION ?= $(shell helm show chart chart/k8gb/ 2>&1 \|grep -v 'found symbolic link in path' \| awk '/appVersion:/ {print $$2}')

make: Suppress symlink warnings on getting k8gb $VERSION #499

make: Suppress symlink warnings on getting k8gb $VERSION #499

Conversation

somaritane commented May 20, 2021

ytsarev May 21, 2021

Choose a reason for hiding this comment

somaritane May 21, 2021 • edited Loading

Choose a reason for hiding this comment

ytsarev May 21, 2021

Choose a reason for hiding this comment

somaritane May 21, 2021

Choose a reason for hiding this comment

ytsarev commented May 21, 2021

somaritane commented May 21, 2021

somaritane commented May 21, 2021

ytsarev left a comment

Choose a reason for hiding this comment

somaritane commented May 21, 2021 • edited Loading

somaritane May 21, 2021 •

edited

Loading

somaritane commented May 21, 2021 •

edited

Loading