Closed etopeter closed 6 years ago
@etopeter I've merged in the new OpenSSL only branch that also supports named secrets and the new home folder structure. When you have a chance, please update this pull request to use the new methods such as get_secret and set_secret. Thanks!
@etopeter This is looking really good! I ran through the tests both locally and through docker and it worked. There is a small issue right now due to calling checks within set_secret now. When set_secret gets invoked in the get_secret flow it will get called by the get_abs_secret_name method...
get_secret_abs_name() {
SECRET_ABS_NAME="$ENCPASS_HOME_DIR/secrets/$LABEL/$SECRET_NAME.enc"
if [ ! -f "$SECRET_ABS_NAME" ]; then
set_secret
fi
}
As you can see right now, no parameters are passed in, so $1 and $2 are null when the checks method gets run the second time, which causes $LABEL and $SECRET_NAME to be overridden with the default values. The simple fix here is to pass $LABEL and $SECRET_NAME when the secret method is called in this flow. It would be nice if we could avoid running the checks method multiple times in the get_secret flow. Perhaps we could add a variable in the checks method that gets set on the first run and then add an if condition inside the checks method (wrapping the existing code) that doesn't allow the code to run if that variable is set?
Also, we need to enhance test_cleanup to remove the keys and secrets it adds during the test run to the .encpass folder. Alternatively, we could opt to just test this only docker and do away with the support for testing locally on a machine. I tend to think we can simulate different environments better using docker anyway.
@ahnick I think I got all your points covered. checks are running once defined by variable tests running in docker only using volume this time for speed
Speaking of variables, maybe all variables should be prefixed ENCPASS? Since its used inside other scripts it could collide with something else.
@etopeter Your changes look good, but can we put the check
+ if [ ! -n "$ENCPASS_CHECKS" ]; then
+ checks $1 $2
+ fi
inside the checks function, instead of having it in set_secret? There may be other places where we want to call checks at some point and I don't think we would want to add this conditional in multiple places either. Yes, I agree with your point about prefixing with ENCPASS to avoid conflicts.
@ahnick Moved Checks to the same function. Using return to exit early instead giant if statement. Could be done in one liner but didn't see any usages of short if statements so kept long version.
Test suite:
Run all:
Run only 1 interpreter
Use Makefile
Test with Docker