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

refs #15931 - allow passing the cname parameter when generating certs #120

Merged
merged 2 commits into from
Jan 17, 2017

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Dec 29, 2016

This PR does not completely fix http://projects.theforeman.org/issues/15931 aka https://bugzilla.redhat.com/show_bug.cgi?id=1160344, but it at least allows to actually generate certificates with multiple names in them which would have prevented the mentioned issue.

The cert resource already knows how to handle katello-ssl-tools --set-cname, so we just have to pass the right value to it. This is done by creating a default [] cname parameter and passing this along to all cert invocations. Kafo then can override the value via the command line.

@@ -7,11 +7,15 @@
#
Copy link
Member

Choose a reason for hiding this comment

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

This file is not being used anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then it should not exist? ;-)
But it was quite handy, as it allowed me to test on a plain Katello 3.2 install.

Copy link
Member

Choose a reason for hiding this comment

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

I agree it should not :)

@@ -16,6 +16,7 @@
$ssl_build_dir = '/root/ssl-build'

$node_fqdn = $::fqdn
$node_cname = []
Copy link
Member

Choose a reason for hiding this comment

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

Could we just change this to cname ? The node noemclature was there for pulp nodes and the old node-installer and I think it helps to not use it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will do.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

this enables Kafo to set the subjectAltName of the certificates via
--certs-node-cname. The option can be given multiple times to add
multple cnames.
…e cli

this enables Kafo to set the subjectAltName of the certificates via
--foreman-proxy-cname. The option can be given multiple times to add
multple cnames.
@ehelms
Copy link
Member

ehelms commented Jan 17, 2017

Thanks @evgeni !

@ehelms ehelms merged commit db68211 into theforeman:master Jan 17, 2017
@iNecas
Copy link
Member

iNecas commented Jan 24, 2017

What's missing to fix 1160344? The BZ mentions ability to set the CNAME on certs as alternative way of addressing the issue

@evgeni
Copy link
Member Author

evgeni commented Jan 24, 2017

@iNecas this only does the katello certs, so missing for 1160344 would be (IMHO):

  • puppet master certs (RHBZ#1415139)
  • the ability for foreman/katello to actually use the alternative names, currently all templates will use the primary name

@iNecas
Copy link
Member

iNecas commented Jan 24, 2017

My point is, I see this already bringing value to the users, and makes their lives easier when needing this. I would suggest treating the other issues as ones blocked by this, but we could start delivering this sooner, and for earlier releases, this could become a documentation bug.

@ehelms
Copy link
Member

ehelms commented Jan 24, 2017

@evgeni before I open a full Redmine issue, I hit this while testing a smart proxy, do you think its related to this change or something else entirely?

foreman-proxy-certs-generate
--foreman-proxy-fqdn
bye-puppet-proxy.example.com
--certs-tar
/root/bye-puppet-proxy.example.com.tar.gz

start: 2017-01-24 20:49:08.779575

end: 2017-01-24 20:49:14.890633

delta: 0:00:06.111058

stdout: Error during configuration, exiting

stderr: stty: standard input: Inappropriate ioctl for device
Parameter foreman-proxy-cname invalid: nil is not a valid array

@evgeni
Copy link
Member Author

evgeni commented Jan 24, 2017

@ehelms possible. is that puppet 3 or puppet 4?

@ehelms
Copy link
Member

ehelms commented Jan 26, 2017

Puppet 4

@evgeni
Copy link
Member Author

evgeni commented Jan 26, 2017

@ehelms can you try with 3? I think my test box for that only ever had 3.

@ekohl
Copy link
Member

ekohl commented Jan 26, 2017

@ehelms maybe katello-installer needs a migration?

@stbenjam
Copy link
Member

foreman-proxy-certs-generate doesn't have migrations, the answers are embedded in the script. Maybe certs::params isn't getting loaded somehow?

$certs_tar = $certs::params::certs_tar
$parent_fqdn = $fqdn,
$foreman_proxy_fqdn = $certs::node_fqdn,
$foreman_proxy_cname = $certs::cname,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe $certs name space isn't loaded when this class is processed in foreman-proxy-certs-generate?

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.

5 participants