-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Allowing a wildcard cert to be generated. #3821
Conversation
I'm back and forth on doing the task file include in the block. Since there are only two tasks, it seems reasonable to just put them in the router.yml vice a separate task file. The block functions the same either way. Travis is complaining about yamllint in that new file. |
# keep the logic in this playbook on how to handle certificates. | ||
- block: | ||
- include: wildcard_router_cert.yml | ||
when: openshift_hosted_router_create_certificate | default(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the default
needed here since it is in defaults/main.yml? If so, then it seems it should also be at line 32.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending the comment about the "when" which will prevent the tests from passing, too.
certfile: "{{ openshift_master_config_dir ~ '/openshift-router.crt' }}" | ||
keyfile: "{{ openshift_master_config_dir ~ '/openshift-router.key' }}" | ||
cafile: "{{ openshift_master_config_dir ~ '/ca.crt' }}" | ||
when: openshift_hosted_router_create_certificate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ansible complained about this line. I think it should be indented once and maybe lowered to be clear that it applies to both tasks in the block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@mtnbikenc, thanks. Travis failed on yamllint on py27 but not py35. I changed spacing again. I will pass the tests and then squash these commits. |
a1316e7
to
cd595ca
Compare
aos-ci-test |
1 similar comment
aos-ci-test |
[test] |
Evaluated for openshift ansible test up to cd595ca |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_extended_conformance_install/112/) (Base Commit: 76be31c) |
[merge] |
flake openshift/origin#10162 |
[merge] |
Evaluated for openshift ansible merge up to cd595ca |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/165/) (Base Commit: 9b0f294) |
No description provided.