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

always add es and es-ops hostname to the es server cert #7931

Merged
merged 1 commit into from Apr 13, 2018

Conversation

richm
Copy link
Contributor

@richm richm commented Apr 12, 2018

This will add X509v3 Subject Alternative Name items to the Elasticsearch
server cert like this:

DNS:es.openshift.public.host, DNS:es-ops.openshift.public.host

This allows external Elasticsearch clients using client cert auth
and the Elasticsearch service using externalIPs to be able to turn
on cert verification.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 12, 2018
@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

@ewolinetz

@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/test gcp

@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

Copy link
Contributor

@ewolinetz ewolinetz left a comment

Choose a reason for hiding this comment

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

Other than question regarding omit LGMT

We have to make sure when we want to regen the certs that we delete it from the host before rerunning the playbook also.

@@ -64,7 +64,7 @@
when: not elasticsearch_jks.stat.exists or not logging_es_jks.stat.exists or not system_admin_jks.stat.exists or not truststore_jks.stat.exists

- name: Run JKS generation script
local_action: script generate-jks.sh {{local_tmp.stdout}} {{openshift_logging_namespace}}
local_action: script generate-jks.sh {{local_tmp.stdout}} {{openshift_logging_namespace}} {{openshift_logging_es_hostname | default(omit)}} {{openshift_logging_es_ops_hostname | default(omit)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

does omit actually work? I've just done default() in the past to null it out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ewolinetz good question - there are many other places in the openshift-ansible code where default(omit) is used for a similar purpose

@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/retest

@ewolinetz
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 12, 2018
@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/test gcp

1 similar comment
@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/test gcp

@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/test travis

@richm
Copy link
Contributor Author

richm commented Apr 12, 2018

/test travis-ci

@richm
Copy link
Contributor Author

richm commented Apr 13, 2018

/retest

@richm
Copy link
Contributor Author

richm commented Apr 13, 2018

tests passing - can I get a /lgtm?

@openshift-merge-robot openshift-merge-robot merged commit 5d555fd into openshift:master Apr 13, 2018
@richm richm deleted the bug-1548154 branch April 13, 2018 03:09
@richm
Copy link
Contributor Author

richm commented Apr 13, 2018

/cherrypick release-3.9

@openshift-cherrypick-robot

@richm: new pull request created: #7940

In response to this:

/cherrypick release-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants