Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bats v1.2.1 testing issue - preserve exported bash functions #17

Closed
psellars-hyprnz opened this issue Aug 9, 2020 · 10 comments
Closed

Comments

@psellars-hyprnz
Copy link

psellars-hyprnz commented Aug 9, 2020

I am trying to test a dojo image and when I run bats am seeing the following issue

 ✗ hadolint
   (from function `assert_equal' in file /opt/bats-assert/src/assert_equal.bash, line 40,
    in test file test/integration/test.bats, line 7)
     `assert_equal "$status" 0' failed
   output: 2020/08/10 01:49:52 [ 1]  INFO: (main.main) Dojo version 0.8.0
   2020/08/10 01:49:52 [ 7]  INFO: (main.DockerDriver.HandleRun) docker command will be:
    docker run --rm -v /home/cato/Code/docker-docker-image-dojo:/dojo/work -v /home/cato:/dojo/identity:ro -v /tmp/dojo-environment-multiline-dojo-docker-docker-image-dojo-2020-08-10_01-49-52-67789255:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/dojo-environment-dojo-docker-docker-image-dojo-2020-08-10_01-49-52-67789255 -v /tmp/.X11-unix:/tmp/.X11-unix --name=dojo-docker-docker-image-dojo-2020-08-10_01-49-52-67789255 catosplace/docker-docker-image-dojo:latest "hadolint --version"
   09-08-2020 13:49:52 Dojo entrypoint info: Sourcing: /etc/dojo.d/variables/00-multiline-vars.sh
   /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `DOJO_BASH_FUNC_bats_readlinkf%%=()': not a valid identifier
   /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `{': not a valid identifier
   /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `-f': not a valid identifier
   /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `"$1"': not a valid identifier
   /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `}': not a valid identifier
   
   -- values do not equal --
   expected : 0
   actual   : 1
   --

Wondering if anyone may be able to shed some light on this - when I dojo into the container the file is empty, and when I run the command using dojo "hadolint --version" it works fine. Only difference is running it using bats from CLI in test.

Am using bats version 1.2.1

@xmik
Copy link
Member

xmik commented Aug 10, 2020

Hi. I reproduced your problem! I will try to find the cause.

@psellars-hyprnz
Copy link
Author

psellars-hyprnz commented Aug 10, 2020

I think I have found the cause.

I ran using the --debug flag and see this, which I believe is part of the multiline variables work from what I can work out.

   2020/08/10 21:38:43 [ 4] DEBUG: (main.FileService.WriteToFile) Written file /tmp/test-dojo-environment-multiline-testdojorunid, contents:
    export DOJO_BASH_FUNC_bats_readlinkf%%=$(echo KCkgeyAgcmVhZGxpbmsgLWYgIiQxIgp9 | base64 -d)

In bats v1.2.1 in the bats script at the end is a

export -f bats_readlinkf

I reverted to v1.2.0 which does not have this line and am able to run, so tend to think this is the cause. Unsure how to fix it.

I also would like to take the time to say - thanks for sharing this work, I am trying to build a Dojo container for a docker image pre-commit hook. I really like this work and appreciate your examples. I will tag my Dojo images so that they can be found for use by others.

@psellars-hyprnz psellars-hyprnz changed the title bats testing issue bats 1.2.1 testing issue Aug 10, 2020
@psellars-hyprnz psellars-hyprnz changed the title bats 1.2.1 testing issue bats v1.2.1 testing issue Aug 10, 2020
@xmik
Copy link
Member

xmik commented Aug 10, 2020

Thanks for your investigation. I agree with the cause and will do more investigation in order to make Dojo work with latest Bats. Can't promise, however, that I will fix it today.

Thanks for the nice words! :)

@psellars-hyprnz
Copy link
Author

You are most welcome.

No rush on the fix, I will work with the v1.2.0 for now and update when it is addressed. Sorry I could not be more helpful in fixing the issue at this time.

I noticed you had switched to use Python test instead of bats - is that the path you will use going forward for Dojo and Dojo images? Just wondering which you recommend, or is a mixture appropriate?

I also use aws-vault when using AWS, I was able to get this working passing a value to the docker options, just wondered if this is a good pattern with Dojo.

@xmik
Copy link
Member

xmik commented Aug 10, 2020

We decided to move away from bats, because there was some time when it was unmanaged sstephenson/bats#150. We had some problems running the dojo bats tests (I remember for sure that the exit status was invalid for Alpine Linux). Perhaps it is already fixed in bats-core.

All our machines had Python installed, so it was easy to switch. For Dojo we will continue to use the Python tests, but for Dojo images you can use whatever framework suits you. Sometimes you may think that adding another language (Python) just for tests is too much for your stack and sometimes you may want to use PyCharm and be able to debug in IDE. Sometimes you can run Dojo image tests in another Dojo image (so in result it does not matter what your machine has installed, because you can just docker pull a ready environment - a Dojo image to run tests in it) and sometimes it is strenuous to do (e.g. because of mounting the volumes and file permissions).

Great found with the aws-vault! I have not used it before, but I worked with AWS and know about STS. Other ideas are listed in the readme https://github.com/kudulab/dojo#secrets. The most secure solution I've used so far is with Vault from Hashicorp. This solution requires a standalone server to host the secrets but I read that aws-vault also needs a local EC2 instance.

@xmik
Copy link
Member

xmik commented Aug 10, 2020

If you already have credentials in a file in your home directory, then they are already mounted (read-only) into /dojo/identity directory. You can then easily copy the credentials file into the /home/dojo directory. There is an example for AWS credentials: https://github.com/kudulab/docker-k8s-aws-dojo/blob/master/image/etc_dojo.d/scripts/99-dojo-copy-aws-cred.sh.

@psellars-hyprnz
Copy link
Author

If you already have credentials in a file in your home directory, then they are already mounted (read-only) into /dojo/identity directory. You can then easily copy the credentials file into the /home/dojo directory. There is an example for AWS credentials: https://github.com/kudulab/docker-k8s-aws-dojo/blob/master/image/etc_dojo.d/scripts/99-dojo-copy-aws-cred.sh.

I use the 99Designs aws-vault (https://github.com/99designs/aws-vault) - which stores your credentials in a keystore on your host machine. Was able to get the AWS credentials into the container when run using the following syntax:

dojo --docker-options "--env-file <(aws-vault --debug exec devops -- env | grep ^AWS_)" -- <CMD>

Often run terraform with different profiles, so can pass the relevant profiles and set up some aliases to simplify the command.

Am exploring lots of ways to use this awesome work. Once again many thanks for all your efforts.

@xmik
Copy link
Member

xmik commented Aug 14, 2020

This issue will be solved in Dojo 0.9.0. PR is already created.

Reason of this issue

Whenever a bash function is exported in the following way:

my_bash_func() {
  echo "hello"
}
export -f my_bash_func

Bash creates an environment variable, in this case:

BASH_FUNC_my_bash_func%%=() {  echo "hello"
}

So far, when there was any bash function exported, Dojo resulted in an error like:

13-08-2020 19:53:43 Dojo entrypoint info: Sourcing: /etc/dojo.d/variables/00-multiline-vars.sh
/etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `DOJO_BASH_FUNC_my_bash_func%%=()': not a valid identifier
/etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `{': not a valid identifier
/etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `"hello"': not a valid identifier
/etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `}': not a valid identifier

This was made visible when using Bats-core v1.2.1, because they exported a bash function in this PR. Dojo interpreted this bash environment variable as a multiline variable and attempted to serialize it with base64. It was fine, but deserialization lead to the error above.

Solution

Dojo 0.9.0 treats all the environment variables which names start with "BASH_FUNC_" prefix and which values start with "()" as exported bash functions and serializes them into /etc/dojo.d/variables/01-bash-functions.sh file. This file is sourced at a container start. However, the bash functions are not preserved later, because sudo -E (used in Dojo entrypoint) does not preserve exported bash functions after ShellShock. See this and this.

It was decided that Dojo 0.9.0 should follow the sudo's practice and not try to add a work around leading to bash functions being preserved. But, it was made easy for the end user to preserve them with:

source /etc/dojo.d/variables/01-bash-functions.sh

Usage instructions are added to readme.

@xmik xmik changed the title bats v1.2.1 testing issue bats v1.2.1 testing issue - preserve exported bash functions Aug 14, 2020
@psellars-hyprnz
Copy link
Author

@xmik thanks for the awesome explanation of the issue and fix implementation.

@xmik
Copy link
Member

xmik commented Aug 15, 2020

@psellars-hyprnz - I'm glad that you found this problem and we could make Dojo better :). Do not hesitate to find more :)

Dojo 0.9.0 is released, thus I'm closing this issue.

@xmik xmik closed this as completed Aug 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants