Check if environment variables that refer to external binaries exist before running them

[mergenci]

What happened?

TL;DR

I had a series of problems when I tried to use Uptest for the first time. The most important problem was that Uptest tried to run annotate command, because KUBECTL environment variable was unset. Unfortunately, annotate is a real command that is part of LibGD on my system. Fortunately, it wasn't invoked properly to have an effect. Though, such behavior could easily have nasty results.

Details

When I ran uptest for the first time, I got the following error:

bash: line 1: : command not found
uptest: error: cannot run e2e tests successfully: cannot execute tests: kuttl failed: exit status 127

I discovered that kuttl should have been installed. We better document it at the top of the Readme. The statement “Uptest comes as a binary which can be installed from the releases section.” gave me false confidence that the binary is self-sufficient.

I installed kuttl using brew, but I got the same error after running uptest. I navigated the source code and discovered that kuttl binary was reached through KUTTL environment variable. I set KUTTL environment variable:

export KUTTL='kubectl kuttl'

When I ran uptest again, I got the following error in an infinite loop:

    logger.go:42: 22:35:39 | case/0-apply | command failure, skipping 3 additional commands
    logger.go:42: 22:35:40 | case/0-apply | running command: [annotate managed --all upjet.upbound.io/test=true --overwrite]
    logger.go:42: 22:35:40 | case/0-apply | dyld[87940]: Library not loaded: /usr/local/opt/libtiff/lib/libtiff.5.dylib
    logger.go:42: 22:35:40 | case/0-apply |   Referenced from: <5B037668-7DA9-3463-B52D-9E582A3352AE> /usr/local/Cellar/gd/2.3.3_4/bin/annotate
    logger.go:42: 22:35:40 | case/0-apply |   Reason: tried: '/usr/local/opt/libtiff/lib/libtiff.5.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/opt/libtiff/lib/libtiff.5.dylib' (no such file), '/usr/local/opt/libtiff/lib/libtiff.5.dylib' (no such file), '/usr/local/lib/libtiff.5.dylib' (no such file), '/usr/lib/libtiff.5.dylib' (no such file, not in dyld cache), '/usr/local/Cellar/libtiff/4.5.0/lib/libtiff.5.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/Cellar/libtiff/4.5.0/lib/libtiff.5.dylib' (no such file), '/usr/local/Cellar/libtiff/4.5.0/lib/libtiff.5.dylib' (no such file), '/usr/local/lib/libtiff.5.dylib' (no such file), '/usr/lib/libtiff.5.dylib' (no such file, not in dyld cache)

Wondering why uptest would need libtiff, I upgraded my LibGD 😄🤦‍♂️. Then, running uptest gave the following error in an infinite loop:

    logger.go:42: 22:55:38 | case/0-apply | running command: [annotate managed --all upjet.upbound.io/test=true --overwrite]
    logger.go:42: 22:55:41 | case/0-apply | Usage: annotate imagein.jpg imageout.jpg
    logger.go:42: 22:55:41 | case/0-apply | 
    logger.go:42: 22:55:41 | case/0-apply | Standard input should consist of
    logger.go:42: 22:55:41 | case/0-apply | lines in the following formats:
    logger.go:42: 22:55:41 | case/0-apply | color r g b (0-255 each) [a (0-127, 0 is opaque)]
    logger.go:42: 22:55:41 | case/0-apply | font fontname
    logger.go:42: 22:55:41 | case/0-apply | size pointsize
    logger.go:42: 22:55:41 | case/0-apply | align (left|right|center)
    logger.go:42: 22:55:41 | case/0-apply | move x y
    logger.go:42: 22:55:41 | case/0-apply | text actual-output-text
    logger.go:42: 22:55:41 | case/0-apply | 
    logger.go:42: 22:55:41 | case/0-apply | If the file 'paris.ttf' exists in /usr/share/fonts/truetype or in a
    logger.go:42: 22:55:41 | case/0-apply | location specified in the GDFONTPATH environment variable, 'font paris' is
    logger.go:42: 22:55:41 | case/0-apply | sufficient. You may also specify the full, rooted path of a font file.
    logger.go:42: 22:55:41 | case/0-apply | command failure, skipping 3 additional commands

This time, I was convinced that there was something wrong and I searched for "annotate" in source code, which lead me to see that it was supposed to be kubectl annotate.

As a result, I propose that we:

1. check if KUTTL and KUBECTL environment variables are set, before calling them as commands (also ask users to set them, if unset),
2. tell users to install kuttl near the top of the Readme.

I would be happy to work on these.

How can we reproduce it?

Unset KUBECTL and KUTTL environment variables and run uptest with examples/iam/role.yaml:

unset KUBECTL
unset KUTTL
./uptest e2e examples/iam/role.yaml

What environment did it happen in?

• Uptest Version: 0.6.1
• Kubernetes Client Version: v1.28.2
• Kubernetes Server Version: v1.26.0
• Kubernetes distribution: kind version 0.20.0
• OS: macOS Sonoma 14.0

[ytsarev]

I think we have to be honest and document somewhere that currently uptest is not really usable without surrounding make targets and the build module :)

[ytsarev]

I noticed that my comment above is featured in the wonderful blogpost about Crossplane testing https://www.codecentric.de/wissens-hub/blog/testing-crossplane-compositions-kuttl as the argument to not use uptest :)

While I agree that we lack the documentation, I have to mention that we made some effort in explaining uptest concepts in presentations like https://resources.upbound.io/level-up/crossplane-testing-patterns-level-up-with-crossplane-presented-by-upbound-may-2024

Essentially, if you adopt build module and e2e target from any Upbound reference configuration like https://github.com/upbound/configuration-aws-network, you should be successful with the uptest setup 👍

[mergenci]

That blog post sounds a little bit harsh to me. Neither this issue nor @ytsarev's comment above states that Uptest is unusable. It's just that the documentation is not up-to-date. Otherwise, Uptest is definitely not a work-in-progress. To the contrary, it's essential in testing PRs. Just visit provider PRs to see how frequently it is used. Here's an example: crossplane-contrib/provider-upjet-aws#1367

Now that this thread has turned into comments section of the blog post 😄, I would like to note that Uptest is in the process of migrating from kuttl to chainsaw: crossplane/uptest#15