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

Add new driver for trustme #136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moisesguimaraes
Copy link
Contributor

The trustme driver provides TLS certificates for both client and
server side on the go. The certificates and keys are created in
the tempdir and their path pushed to environment variables.

Signed-off-by: Moisés Guimarães de Medeiros guimaraes@pm.me

The trustme driver provides TLS certificates for both client and
server side on the go. The certificates and keys are created in
the tempdir and their path pushed to environment variables.

Signed-off-by: Moisés Guimarães de Medeiros <guimaraes@pm.me>
Copy link
Owner

@jd jd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some tests?

@@ -185,7 +185,7 @@ def expand_urls_var(url):

putenv("PID", str(os.getpid()))
putenv("DAEMON", daemon)
url = os.getenv(driver.env_prefix + "_URL")
url = os.getenv(driver.env_prefix + "_URL", "")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated to this PR. If it's needed, can you put that into another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do, it was crashing cause the new driver didn't put an env variable prefixed with _URL.

@@ -251,7 +251,7 @@ def _cleanup(signum, frame):
signal.signal(signal.SIGPIPE, signal.SIG_IGN)
signal.pause()
else:
url = driver.env['%s_URL' % driver.env_prefix]
url = driver.env.get('%s_URL' % driver.env_prefix, "")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -4,4 +4,5 @@ pbr
jinja2
fixtures
psutil
trustme
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should come with a flavor, not in base deps.

def get_options(cls):
return [
{
"param_decls": ["--client_identity"],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"param_decls": ["--client_identity"],
"param_decls": ["--client-identity"],

sounds more common

"to use when checking identity.",
},
{
"param_decls": ["--server_identity"],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"param_decls": ["--server_identity"],
"param_decls": ["--server-identity"],

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

Successfully merging this pull request may close these issues.

2 participants