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

Deploy Foreman with cert not from puppet #135

Closed
wants to merge 1 commit into from

Conversation

amh-mw
Copy link
Contributor

@amh-mw amh-mw commented Dec 11, 2013

On CentOS 6, the following settings allow me to use a CA-provided cert instead of a puppet cert.

$tlsdir = "/etc/pki/tls"

class { '::foreman':
    client_ssl_ca   => "${tlsdir}/certs/IntermediateCA.crt",
    client_ssl_cert => "${tlsdir}/certs/star.domain.com.crt",
    client_ssl_key  => "${tlsdir}/private/star.domain.com.key",
}

class { "::foreman_proxy": }

class { "::puppet":
    # Generated per http://curl.haxx.se/docs/caextract.html
    server_foreman_ssl_ca => "${tlsdir}/certs/ca-bundle.crt",
}

SSLCertificateChainFile /var/lib/puppet/ssl/certs/ca.pem
SSLCertificateFile <%= @client_ssl_cert %>
SSLCertificateKeyFile <%= @client_ssl_key %>
SSLCertificateChainFile <%= @client_ssl_ca %>
Copy link
Member

Choose a reason for hiding this comment

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

Are these parameters available in the scope? Would you be willing to write a test for this?

@ekohl
Copy link
Member

ekohl commented Dec 11, 2013

I don't know why Travis failed, but it looks like a Travis issue more than the code.

This can be a good addition, especially since we already have the variables available in params.pp. I do wonder what PUP-57 stands for.

@amh-mw
Copy link
Contributor Author

amh-mw commented Dec 11, 2013

On Wed, Dec 11, 2013 at 11:57 AM, Ewoud Kohl van Wijngaarden
notifications@github.com wrote:

Are these parameters available in the scope?

Ah, no. I'm running puppet 2.7, so I haven't purged all my bad habits yet.

Would you be willing to write a test for this?

Sure, I should be able to get back to this later this week.

I do wonder what PUP-57 stands for.

It is just the issue number in JIRA here internally. I didn't think
to scrub it from the comment.

@ekohl
Copy link
Member

ekohl commented Dec 12, 2013

👍 from me. @domcleal?

Semi-related: I wish github would send me a notification if a new commit was added.

@domcleal
Copy link
Contributor

I'm uncomfortable using the client parameters for the server side certificates, since I think these params are designed for the puppetmaster certificates when talking to Foreman. Could we add new params for server certs instead?

SSLCertificateChainFile /var/lib/puppet/ssl/certs/ca.pem
SSLCertificateFile <%= scope.lookupvar 'foreman::client_ssl_cert' %>
SSLCertificateKeyFile <%= scope.lookupvar 'foreman::client_ssl_key' %>
SSLCertificateChainFile <%= scope.lookupvar 'foreman::client_ssl_ca' %>
SSLCACertificateFile /var/lib/puppet/ssl/certs/ca.pem
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this line also come from a variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally set it to the same value as SSLCertificateChainFile (which seems to be a fairly common pattern in Apache conf files), but @domcleal suggested that I not. Initial googling suggests that I can/should not use my CA-issued cert to issue client certs.

http://stackoverflow.com/questions/1899983/difference-between-sslcacertificatefile-and-sslcertificatechainfile

@amh-mw
Copy link
Contributor Author

amh-mw commented Dec 16, 2013

PR updated to use server_* class parameters.

@ekohl
Copy link
Member

ekohl commented Dec 23, 2013

Still unsure about the SSLCACertificateFile and it being static, but I don't mind merging it as is. @domcleal @GregSutcliffe?

@domcleal
Copy link
Contributor

domcleal commented Jan 2, 2014

ditto, that's good, thanks for the update @amh-mw.

One last thing, could you add some docs to the top of init.pp for the three new parameters which our installer parses out? Then we can merge. Cheers!

@amh-mw
Copy link
Contributor Author

amh-mw commented Jan 6, 2014

PR updated with documentation.

@domcleal
Copy link
Contributor

domcleal commented Jan 6, 2014

👍 pending Travis

@ekohl
Copy link
Member

ekohl commented Jan 6, 2014

Travis is still broken on 1.8.7, but 👍 from me as well.

@domcleal
Copy link
Contributor

domcleal commented Jan 6, 2014

Thanks for the contribution @amh-mw, merged as 222fb5e!

@domcleal domcleal closed this Jan 6, 2014
@amh-mw amh-mw deleted the vhost_params branch January 6, 2014 17:57
jturel pushed a commit to jturel/puppet-foreman that referenced this pull request May 1, 2018
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.

None yet

3 participants