Allow setting certificates validity period during installation #3581

php-coder · 2017-03-07T15:59:59Z

This PR is adding support for setting certificates validity period during installation. It's done by passing --expire-days and --signer-expire-days options to oc adm that were adding in v1.5 (see openshift/origin#11814)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1275176
Trello: https://trello.com/c/MV4uHYdW/367-leverage-the-new-expire-days-in-the-ansible-playbooks

PTAL @abutcher @tbielawa
CC @mfojtik

php-coder · 2017-03-07T16:00:12Z

Current state/open questions:

It isn't tested because at this moment installer fails with error Detected OpenShift version 1.3.1 does not match requested openshift_release 1.5 when I'm trying to use v1.5 I didn't find the repo for origin 1.5
It's not clear do I need some special code for a containerized installation
I don't know whether code for supporting external etcd should be added or not
I'm not sure that new parameters will affect procedure of updating of the existing certificates
~~It isn't documented~~

abutcher · 2017-03-07T16:20:29Z

It isn't tested because at this moment installer fails with error Detected OpenShift version 1.3.1 does not match requested openshift_release 1.5 when I'm trying to use v1.5 I didn't find the repo for origin 1.5

1.5 packages haven't been created yet afaik. I can get you configuration for testing 3.5 internally.

It's not clear do I need some special code for a containerized installation

This can be accomplished using a host level variable containerized=true per host in the inventory or containerized=true can be added under [OSEv3:vars] to globally set containerized true for all services.

For example,

...

[masters]
master1.abutcher.com containerized=true

[nodes]
master1.abutcher.com openshift_schedulable=true containerized=true

I don't know whether code for supporting external etcd should be added or not

We have an internal variable for configuring external etcd CA validity etcd_ca_default_days but no configurable for etcd peer and serving certificates. If we are allowing OpenShift certificate validity to be configured then I think we should also add variables for external etcd cert validity but that can be done separately.

I'm not sure that new parameters will affect procedure of updating of the existing certificates

This process uses the same code updated here so I expect that no changes will be required.

It isn't documented

Adding these variables to https://github.com/openshift/openshift-ansible/blob/master/inventory/byo/hosts.ose.example with a small explanation will be a good start.

tbielawa

Curious about default setting and the value for the default expiry days.

tbielawa · 2017-03-08T18:11:11Z

playbooks/common/openshift-cluster/redeploy-certificates/registry.yml

@@ -73,6 +73,9 @@
        --hostnames="{{ docker_registry_service_ip.results.clusterip }},docker-registry.default.svc.cluster.local,{{ docker_registry_route_hostname }}"
        --cert={{ openshift.common.config_base }}/master/registry.crt
        --key={{ openshift.common.config_base }}/master/registry.key
+        {% if openshift.common.version_gte_3_5_or_1_5 | bool %}
+        --expire-days={{ openshift_registry_cert_expire_days | default(730) }}


I think we issue 1 year certs now, you're suggesting a 2 year default expiry here. Do we have it defined somewhere how long we want certs to be valid for?

Below, in some vars file I'm seeing defaults for expire days being set.

roles/openshift_ca/vars/main.yml +openshift_ca_cert_expire_days: 1825 +openshift_master_cert_expire_days: 730 roles/openshift_master_certificates/vars/main.yml +openshift_master_cert_expire_days: 730 roles/openshift_node_certificates/vars/main.yml +openshift_node_cert_expire_days: 730

But here you are using default(730) in case openshift_registry_cert_expire_days is not defined. Should that not also be added to a roles vars//defaults/ file?

I think we issue 1 year certs now, you're suggesting a 2 year default expiry here.

I'm using oadm ca default values here. How it's possible that ansible installer uses 1 year certificates and in the same time uses oadm with its default values of 2 (and 5) years? It only possible if a) we have a modified oadm for some reason b) we're generating certs manually somewhere.

But here you are using default(730) in case openshift_registry_cert_expire_days is not defined. Should that not also be added to a roles vars//defaults/ file?

I'm setting default values for roles but here it's impossible because it's not a role but just a playbook, so I'm using explicit default() here. Do you know how it can be improved?

No idea how to improve that. Thanks for explaining!

tbielawa · 2017-03-08T18:15:58Z

roles/openshift_ca/tasks/main.yml

@@ -88,7 +88,7 @@
 # This should NOT replace the CA due to --overwrite=false when a CA already exists.
 - name: Create the master certificates if they do not already exist
  command: >
-    {{ hostvars[openshift_ca_host].openshift.common.client_binary }} adm create-master-certs
+    {{ hostvars[openshift_ca_host].openshift.common.client_binary }} adm ca create-master-certs


Just curious, did this not work in the past without the ca sub-sub command there?

There was a warning about using deprecated command.

php-coder · 2017-03-08T18:58:09Z

Ok, I've updated the code and tested it against OSE v3.5 on RHEL. It works fine on 1 node with embedded etcd. I was checking an expiration dates of all *.crt files inside /etc/origin.

To @abutcher @tbielawa:

Are you OK with these changes? If yes, then I'll add a bit documentation before merging.
Do we have certificates in some non-standard places that I'm not aware of?
Do we have QEs who can test this along with corner cases (like using containerized environment)?
I also tested redeploy-certificates.yml, it updates all the certificates except ca.crt. Is it a feature?

php-coder · 2017-03-08T19:06:44Z

I don't know whether code for supporting external etcd should be added or not

We have an internal variable for configuring external etcd CA validity etcd_ca_default_days but no configurable for etcd peer and serving certificates. If we are allowing OpenShift certificate validity to be configured then I think we should also add variables for external etcd cert validity but that can be done separately.

@mfojtik Is it still in the scope of my task? Technically it's not because these certificates are generated by openssl command and it doesn't use oadm at all. Practically it could be in scope, in terms of a general task for setting the certificates validity.

abutcher · 2017-03-08T19:21:04Z

Are you OK with these changes? If yes, then I'll add a bit documentation before merging.

These changes LGTM.

Do we have certificates in some non-standard places that I'm not aware of?

All are covered here afaict.

Do we have QEs who can test this along with corner cases (like using containerized environment)?

QE will pick up when we move our cluster lifecycle card to complete.

I also tested redeploy-certificates.yml, it updates all the certificates except ca.crt. Is it a feature?

There is a separate playbook for rolling CA redeployment playbooks/byo/openshift-cluster/redeploy-openshift-ca.yml.

tbielawa · 2017-03-08T19:27:54Z

@php-coder said

Ok, I've updated the code and tested it against OSE v3.5 on RHEL. It works fine on 1 node with embedded etcd. I was checking an expiration dates of all *.crt files inside /etc/origin.

To @abutcher @tbielawa:

Are you OK with these changes? If yes, then I'll add a bit documentation before merging.
Do we have certificates in some non-standard places that I'm not aware of?
Do we have QEs who can test this along with corner cases (like using containerized environment)?
I also tested redeploy-certificates.yml, it updates all the certificates except ca.crt. Is it a feature?

If you want a thorough test of the new depoyment I suggest you use the cert expiry checker we have now

Using your existing inventory file you can run:

$ ansible-playbook -v -i <INVENTORY_FILE> ./playbooks/certificate_expiry/easy-mode.yaml
$ xdg-open /tmp/cert-expiry-report.html
$ xdg-open /tmp/cert-expiry-report.json

php-coder · 2017-03-09T16:10:15Z

@abutcher @tbielawa It's ready to merge. PTAL.

abutcher · 2017-03-09T16:54:02Z

[merge]

openshift-bot · 2017-03-09T16:55:18Z

[test]ing while waiting on the merge queue

abutcher · 2017-03-09T19:42:40Z

aos-ci-test

openshift-bot · 2017-03-09T21:46:17Z

3b87f81 - State: success - All Test Contexts: aos-ci-jenkins/OS_unit_tests - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-2-unit-tests-1089/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-09T22:10:45Z

3b87f81 - State: error - All Test Contexts: aos-ci-jenkins/OS_3.4_containerized - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_containerized,OSE_VER=3.4,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster-containerized,TargetBranch=master,nodes=openshift-ansible-slave-1093/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-09T22:11:38Z

3b87f81 - State: success - All Test Contexts: "aos-ci-jenkins/OS_3.4_NOT_containerized, aos-ci-jenkins/OS_3.4_NOT_containerized_e2e_tests" - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_NOT_containerized,OSE_VER=3.4,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster,TargetBranch=master,nodes=openshift-ansible-slave-1093/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-09T22:17:56Z

3b87f81 - State: success - All Test Contexts: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_NOT_containerized,OSE_VER=3.5,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster,TargetBranch=master,nodes=openshift-ansible-slave-1093/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-09T22:20:18Z

3b87f81 - State: success - All Test Contexts: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_containerized,OSE_VER=3.5,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster-containerized,TargetBranch=master,nodes=openshift-ansible-slave-1093/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

php-coder · 2017-03-13T14:30:30Z

The first one is very strange because it could not compile a test:

23:21:08 ++ Building release v1.5.0-alpha.3+4cbbecd-244
23:21:16 rm -rf _output
23:21:16 hack/build-cross.sh
23:21:17 ++ Building go targets for linux/amd64: images/pod cmd/dockerregistry cmd/gitserver pkg/sdn/plugin/sdn-cni-plugin vendor/github.com/containernetworking/cni/plugins/ipam/host-local vendor/github.com/containernetworking/cni/plugins/main/loopback examples/hello-openshift examples/deployment
23:22:31 ++ Building go targets for linux/amd64: cmd/openshift cmd/oc
23:25:46 ++ Building go targets for linux/amd64: test/extended/extended.test
23:27:26 # github.com/openshift/origin/test/extended/registry
23:27:26 test/extended/registry/registry.go:52: cannot use "github.com/openshift/origin/pkg/image/api".ResourceImageStreams (type "github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/api".ResourceName) as type string in argument to "github.com/openshift/origin/pkg/image/api".Resource

The second one is a test-flake openshift/origin#12797

@abutcher could you re-run merge process, please?

sdodson · 2017-03-13T14:37:17Z

flake openshift/origin#12797
[merge]

sdodson · 2017-03-13T14:40:21Z

aos-ci-test is failing on 3.4 because --expire-days hasn't been backported there. Though, why is it only failing for containerized installs?

php-coder · 2017-03-13T14:54:59Z

aos-ci-test is failing on 3.4 because --expire-days hasn't been backported there. Though, why is it only failing for containerized installs?

I suspect that openshift.common.version_gte_3_5_or_1_5 is evaluating to true for some strange reason. Is it possible?

sdodson · 2017-03-13T15:17:57Z

oh, that's probably right.

openshift-bot · 2017-03-13T16:07:17Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_extended_conformance_install/84/) (Base Commit: 403b5c5)

php-coder · 2017-03-14T10:59:41Z

How we'll merge it? How I can help?

php-coder · 2017-03-20T12:48:58Z

Ping.

sdodson · 2017-03-20T13:08:07Z

aos-ci-test

openshift-bot · 2017-03-20T13:16:32Z

3b87f81 - State: success - All Test Contexts: aos-ci-jenkins/OS_unit_tests - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-2-unit-tests-1163/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-20T13:40:26Z

3b87f81 - State: error - All Test Contexts: aos-ci-jenkins/OS_3.4_containerized - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_containerized,OSE_VER=3.4,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster-containerized,TargetBranch=master,nodes=openshift-ansible-slave-1166/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

openshift-bot · 2017-03-20T13:50:30Z

3b87f81 - State: success - All Test Contexts: "aos-ci-jenkins/OS_3.4_NOT_containerized, aos-ci-jenkins/OS_3.4_NOT_containerized_e2e_tests" - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-3-test-matrix-CONTAINERIZED=_NOT_containerized,OSE_VER=3.4,PYTHON=System-CPython-2.7,TOPOLOGY=openshift-cluster,TargetBranch=master,nodes=openshift-ansible-slave-1166/3b87f8180e751e6bebe8c9227eeb4100ffef8eba.txt

php-coder · 2017-03-21T12:37:59Z

Containerized installation fails because openshift.common.version_gte_3_5_or_1_5 is evaluating to true. I feel that it's a bug in condition evaluation. I don't see another way of not using new option on old versions.

abutcher · 2017-03-22T20:30:06Z

@php-coder I'm looking into how we can address this. openshift.common.version_gte_3_5_or_1_5 is defaulting to true because we have nothing to base the version on (as written) but we do know the image tag that we intend to use prior to creating certs.

abutcher · 2017-03-28T17:08:48Z

@php-coder Once #3769 merges the version comparisons can use the new filters https://gist.github.com/abutcher/c60a27f6fa9abf4ae365fc24738349ee.

php-coder · 2017-03-28T17:12:31Z

@abutcher Thank you! I'm watching and waiting :)

…meter.

…parameter.

…t_expire_days parameter.

…cert_expire_days parameters.

…meter.

…ters.

php-coder · 2017-03-29T13:53:31Z

@abutcher Updated and ready to be tested/merged.

openshift-bot · 2017-03-29T13:55:28Z

Evaluated for openshift ansible test up to 638e419

openshift-bot · 2017-03-29T15:15:33Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_extended_conformance_install/84/) (Base Commit: 403b5c5)

abutcher · 2017-03-29T17:36:51Z

aos-ci-test

openshift-bot · 2017-03-29T19:14:47Z

success: aos-ci-jenkins/OS_unit_tests for 638e419 (logs)

openshift-bot · 2017-03-29T19:40:44Z

success: "aos-ci-jenkins/OS_3.4_NOT_containerized, aos-ci-jenkins/OS_3.4_NOT_containerized_e2e_tests" for 638e419 (logs)

openshift-bot · 2017-03-29T19:43:11Z

success: "aos-ci-jenkins/OS_3.4_containerized, aos-ci-jenkins/OS_3.4_containerized_e2e_tests" for 638e419 (logs)

openshift-bot · 2017-03-29T19:46:10Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 638e419 (logs)

openshift-bot · 2017-03-29T19:48:38Z

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for 638e419 (logs)

abutcher · 2017-03-29T19:55:03Z

[merge]

openshift-bot · 2017-03-29T19:55:24Z

Evaluated for openshift ansible merge up to 638e419

tbielawa reviewed Mar 8, 2017

View reviewed changes

php-coder force-pushed the set_cert_expiration_date branch from 295ab3c to 436cf09 Compare March 8, 2017 18:41

php-coder force-pushed the set_cert_expiration_date branch from 436cf09 to 3b87f81 Compare March 9, 2017 16:04

abutcher approved these changes Mar 9, 2017

View reviewed changes

php-coder added 8 commits March 29, 2017 15:50

openshift_node_certificates: add openshift_node_cert_expire_days para…

b463c61

…meter.

openshift_master_certificates: add openshift_master_cert_expire_days …

3944a99

…parameter.

redeploy-certificates/registry.yml: add openshift_hosted_registry_cer…

c5da9d9

…t_expire_days parameter.

openshift_ca: add openshift_ca_cert_expire_days and openshift_master_…

5ae588b

…cert_expire_days parameters.

oc_adm_ca_server_cert: add expire_days parameter.

850fed7

oc_adm_ca_server_cert.py: re-generate.

f2f6713

openshift_hosted: add openshift_hosted_registry_cert_expire_days para…

cf7eef7

…meter.

Update example inventory files to mention certificate validity parame…

638e419

…ters.

php-coder force-pushed the set_cert_expiration_date branch from 3b87f81 to 638e419 Compare March 29, 2017 13:50

openshift-bot merged commit 6f0c060 into openshift:master Mar 29, 2017

php-coder deleted the set_cert_expiration_date branch March 29, 2017 22:02

adellape mentioned this pull request May 31, 2017

Add cert validity Ansible vars openshift/openshift-docs#4504

Merged

purplexa mentioned this pull request Jun 22, 2017

Missing default value for openshift_master_cert_expire_days on release-1.5 #4552

Closed

abutcher mentioned this pull request Jun 28, 2017

[1.5] Allow setting certificates validity period during installation #4629

Merged

Allow setting certificates validity period during installation #3581

Allow setting certificates validity period during installation #3581

Conversation

php-coder commented Mar 7, 2017

php-coder commented Mar 7, 2017 • edited

abutcher commented Mar 7, 2017

tbielawa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

php-coder commented Mar 8, 2017

php-coder commented Mar 8, 2017

abutcher commented Mar 8, 2017

tbielawa commented Mar 8, 2017

php-coder commented Mar 9, 2017

abutcher commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

abutcher commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

openshift-bot commented Mar 9, 2017

php-coder commented Mar 13, 2017 • edited

sdodson commented Mar 13, 2017

sdodson commented Mar 13, 2017

php-coder commented Mar 13, 2017

sdodson commented Mar 13, 2017

openshift-bot commented Mar 13, 2017 • edited

php-coder commented Mar 14, 2017

php-coder commented Mar 20, 2017

sdodson commented Mar 20, 2017

openshift-bot commented Mar 20, 2017

openshift-bot commented Mar 20, 2017

openshift-bot commented Mar 20, 2017

php-coder commented Mar 21, 2017

abutcher commented Mar 22, 2017

abutcher commented Mar 28, 2017

php-coder commented Mar 28, 2017

php-coder commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

abutcher commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

abutcher commented Mar 29, 2017

openshift-bot commented Mar 29, 2017

php-coder commented Mar 7, 2017 •

edited

php-coder commented Mar 13, 2017 •

edited

openshift-bot commented Mar 13, 2017 •

edited