Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Update tls-init-job.yaml #329

Merged
merged 1 commit into from
Jan 14, 2020
Merged

Update tls-init-job.yaml #329

merged 1 commit into from
Jan 14, 2020

Conversation

tehmoon
Copy link
Contributor

@tehmoon tehmoon commented Jan 13, 2020

Missing domain for the tls-init-job

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Hey @tehmoon, thanks for the PR! I don't think the CA needs a domain, but curious what error you saw that led you to this change.

@@ -45,7 +45,8 @@ spec:
- "/bin/sh"
- "-ec"
- |
consul tls ca create
consul tls ca create \
-domain={{ .Values.global.domain }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the CA needs a domain. Are you seeing errors when you don't provide it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CA per se doesn't need one, the error lies in the fact that if a domain is set, the following command consul tls cert create -server which has domain set, will not be able to find the right file generated by consul tls ca create.
IE:

creator@infrastructure:/tmp/tmp.NtWBXsJ0Gc$ consul tls ca create
creator@infrastructure:/tmp/tmp.NtWBXsJ0Gc$ ls
consul-agent-ca-key.pem  consul-agent-ca.pem
creator@infrastructure:/tmp/tmp.NtWBXsJ0Gc$ consul tls cert create -server -domain test.tld
Error reading CA: open test.tld-agent-ca.pem: no such file or directory

If the domain flag is set for the CA:

creator@infrastructure:/tmp/tmp.NtWBXsJ0Gc$ consul tls ca create -domain test.tld
==> Saved test.tld-agent-ca.pem
==> Saved test.tld-agent-ca-key.pem

And thinking more about it, it could actually be solved with -ca:

  -ca=<string>
     Provide path to the ca. Defaults to #DOMAIN#-agent-ca.pem.

Like this:

creator@infrastructure:/tmp/tmp.NtWBXsJ0Gc$ consul tls cert create -server -ca consul-agent-ca.pem
==> WARNING: Server Certificates grants authority to become a
    server and access all state in the cluster including root keys
    and all ACL tokens. Do not distribute them to production hosts
    that are not server nodes. Store them as securely as CA keys.
==> Using consul-agent-ca.pem and consul-agent-ca-key.pem
==> Saved dc1-server-consul-0.pem
==> Saved dc1-server-consul-0-key.pem

Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch, thank you! I think it makes sense to provide the domain for consistency, even though this file will only exist during the duration of the job run.

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

💯 💯

@ishustava ishustava merged commit c2a4711 into hashicorp:master Jan 14, 2020
@tehmoon
Copy link
Contributor Author

tehmoon commented Jan 14, 2020

Thank you @ishustava great work for that #313 PR!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants