Skip to content
This repository has been archived by the owner on Dec 1, 2020. It is now read-only.

IN-105: Enabled SSL and HTTPS for Elasticsearch #69

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

IN-105: Enabled SSL and HTTPS for Elasticsearch #69

wants to merge 32 commits into from

Conversation

michaelamiethyst
Copy link

No description provided.

@mikeln
Copy link
Member

mikeln commented Nov 15, 2018

First thing,
you also need to fix/update the tests

Copy link
Member

@mikeln mikeln left a comment

Choose a reason for hiding this comment

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

do not check in the values.yaml file that is different from the one in the repo...
unless you are adding new required values.

@michaelamiethyst
Copy link
Author

Fixed tests. Incremented .version in Chart.yaml. Reverted changes in values.yaml.

charts/elasticsearch-chart/Chart.yaml Show resolved Hide resolved
charts/elasticsearch-chart/values.yaml Outdated Show resolved Hide resolved
charts/elasticsearch-chart/values.yaml Outdated Show resolved Hide resolved
@michaelamiethyst
Copy link
Author

Fixed all issues flagged, including rebasing the entire fork.

Copy link
Member

@mikeln mikeln left a comment

Choose a reason for hiding this comment

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

need to pass in certificate/keys. i.e. via --set off helm command line. (see chart-fluentd as an example).

@@ -11,10 +11,13 @@ data:

# run until either cluster status is green or 10 minute timeout
timeout=0
url=https://${ELASTICSEARCH_PORT_9200_TCP_ADDR}:9200/_cat/health
Copy link
Member

Choose a reason for hiding this comment

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

this may work, but I would prefer you left this one how it was {{ .Values.name }}.{{ .Release.Namespace }} because CI may run many tests and they each have a different namespace and name...so they don't clash.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I don't see this set anyplace ELASTICSEARCH_PORT_9200_TCP_ADDR

Copy link
Author

Choose a reason for hiding this comment

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

The test was failing locally with the original values. We could have two URLs (e.g. $url_second or some such) and only try the second if we can't connect to the first.

@@ -114,7 +114,7 @@ master_storage_azure_kind: Managed

#Cluster StatefulSet
# Need a storage class for CI testing
data_create_storage_class: false
data_create_storage_class: true
Copy link
Member

Choose a reason for hiding this comment

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

do not set this here.
Always use the --set on the helm command line instead.

Copy link
Author

Choose a reason for hiding this comment

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

Will fix. That one line always slips through the cracks, I mean to only change it locally and not check it in.

unencode-certificates.sh: |-
{{- include "unencode-certificates.sh.tpl" . | indent 4}}
elastic-certificates.p12.b64: |-
MIINbwIBAzCCDSgGCSqGSIb3DQEHAaCCDRkEgg0VMIINETCCBW0GCSqGSIb3DQEHAaCCBV4EggVaMIIFVjCCBVIGCyqGSIb3DQEMCgECoIIE+zCCBPcwKQYKKoZIhvcNAQwBAzAbBBSrTzyNwO6NPNKPDqQWk829cJnTWgIDAMNQBIIEyAnqtJHxp5lW2fg8J5DzKjAMjKQnY0lZ+M/Q6Bm0bNgQ+9QVRo+JjVXfIUTN7lFrLthPFNEmPToJmCWATTdy4j8B7mSvIXxk7VoOwCtwvxwMDgAGP/t7faSMx3F2pU7OADok8bv/BSS9QJx5P5ZJ/NsK/3ahPsxFJLWkxS6ZeWyfQympo7SkB8uyfMoBdf8OqjFtSU9INB9OM3V6dSt/KFg6wdEOXcJXt6Boplkj0SG/wa8HYQ8RO8wt8J7kEjM1NnRpP+o5SmkaS10Tn9m97EgdmOXd/ARYyqZBQSie6xUIL0b1yFqHK4y4XI0TlPb3cS372u8MyuBole1vEFfEqGu6goVquL7mx5YkK41vS/2Sed1gtzb5wpDKstblPgSS8ZaN5+DPeU2WMk0nuoKMZdNs0FPK33ZqkKKsy0asPdeIhCcPE+qPVRj9cdC1yw0wFJTyPs5Gh3l2Zsz3o15SeyWY9DyGnOninTZ0vz4eqv/Nlmc8A1b9XChkQReUDoYvFo8t24CrOPWyNySNb8P2NGqKF2Zbyw9K54u75QE9704BU+DeUhqB5VjtUCBDt8ZTCPWBut38J0f/IzqCY3pw5nVCsmyRqQpziFnvxBuIESrODjtt5v6of+C/BuZQUhRcGeq6P/s8WbhbvD/GfQCQtt+rNugWUnO5ZUARzS80Z+aen7YqDlnTTsHAc3ZMI1BQjUynyH8qcC+YBX0vVnGv8l0XrNN+MmJt6MKQ/M/VFsupFhXIvlaVZzoqZQMlc8yRgISDK55s8wNRwlpWvSWgLjxNZNqrZUU504dRgWfhH7fCqVWkN4ZJ2/5jSUDxBCWST4qIZMZtU9xnwW6KF17WcuQWjVDYb3ibL7jV0+PC640qb4mjwZudEMYYRWqPFkeUknl8USRjWiN/rknFLdF+8G9quUYFHUTjxgLR2642wprRFHXckXDZ5ThiwbhDKpmUZQHCe1b/R/4ShV8XDaqYYHuQQgnUsmhgW+96jXbXQL3FLlQmbZg5TmvqjtGJGYbHvPX0l5LKgKEJZUO1Vou5M3qPRUqUOyuXGU0XOwBRL/qay9ZDhJM1oSX1SPIcnO+UqbceS4EWfpn3TGiVskU0HHs81lP+iP4ERA10F7gwu7kyLPs6BsPkqJQZaZrrLbgMCkxBjOdG3NOd5/OcjhAJiadwfSL3A/YHoISN2OhNZ6vWl8+axYfeMp/u21/5c03pdoxzHCUI031EkhsMvlKLZetNy90nRZHbfzcq/KAVfdMMDeax+JYZ2n1s/QBC93Qzoh8gQ0RCh7nmVLGNXB2cw2Qx7I5tWjd/Ax9M73xigyo+FML7Ah5CXyiqBZBEuJHb/VdPod77arvS46lnS2+BhqzCQVmsenkulyjKplQMSLRpwdceBXkD47cP5Ygzf9FUWzkNXTDAgzdQ+UgFtMVuZ4tQeLIwtcvCZeGhypuaNXvdY25E0YBTyZjFggwG17fPV2W42mN4lrMKQy7+Ky9Hjz5wQ5xBrfT+nKedeCpLioZ8VNwcbXt+TlLMx/7E+pYq4U13XialissOr/+C0vRE9abtVj6631LibNC8HoPNh3ctoWE3slLj6ZN4g+rwrQLJYRa7h2VKglD19JSN97BXNHgyxc7DBFv1sTFEMB8GCSqGSIb3DQEJFDESHhAAaQBuAHMAdABhAG4AYwBlMCEGCSqGSIb3DQEJFTEUBBJUaW1lIDE1NDIyMTkzMDIyMzUwggecBgkqhkiG9w0BBwagggeNMIIHiQIBADCCB4IGCSqGSIb3DQEHATApBgoqhkiG9w0BDAEGMBsEFP3F4mWbIY/C9o1br/nRVIkjgmvnAgMAw1CAggdILKswTlrRTnVSbL8aDUbrEhE2krJ+EyNXwPL/TPBvaCRho+UzcJ/dZMl/T/WT51ClM6dzu6v8OHxLwIvMHMmvVbnUKj5ilSi3bOur63wv4N92Hk0RYLT0iMhlpLIkQ9+8ciECw0udg00gtoJpTE/TG1uGN5YLudAYA2JfcBhOaXCiSgYjlbfZ43FqNH9/OAXY7stxcgoL2KAY+TVNFT4uDC+zqeW3m2DKeIsX6bGs1zEdiTvP5CZet8rKt2Wux3P883/QIhyXTop9jc3w4RHW04BTUppvO3RxYiEpTJXXWcOJnt4sOEKK/HDe4um0CXhY9Fal9fFLq/jOmNzWYY7dxWlspFTCooreeRq/m/83AqSe5FgkGGiHyPvAop6HsMzglHA6SU5WUN2PY6FFMaM4CWDRz3vmI6f+MD9unngQ+w6MZJkvHMZop1EvVyluLbKbB6T8IE51fgMa4lbdgcIukyczNITKWaawaziaPbMjRY2YhI+6s3YUeEHdNRthFPsh3YW2G1U/EVV74c5kqgFNvHgB1C/FfTiOkfv6zze9TsO+Jnt41IZ1Sv+ju8DJVf7DeHtuf7z/Uo3MEZ6f3/v8bcwdROGo6R0Lq24MVC/wFl3kgm9ZTeTR2iBiQUY/fteHK6l1g04eBNPSfD8I0QrYNwzRWsrNoT+j2PXW41ile7ZmxtGYpBBsYusDplz0/Za/jqY5FIl672J+SXJN4MEc0ujOg6QuxZ8/C/JuLCOXerBo3iKwOyrTO2pQkPf0sZArBakyWSt5dgLlzp3jhbJohP3SMZIymHifPrdt7JrUPcR2iTi1FjPSX5QmQR9zmeQPo61pyWWj++vDMbXDErmm4u1ofTFFJ3B1wAQjaobcGdDJh5BV3vJsES8ZUhGep2MMpBW7GzDscjgq9VuMoKtYBrRvGcKs0+SObau0Z0Xl+N1i6t2hgLYDyH/njqjeiOCvyezWWBby0XcRKHz8GJYz6hR80c9z6ANhlaENyZOWuhVHS4IYnNjSKKRFV9fepyXbFwp0BqABxdo2yEeDOGxGF1h/JrfTrloWIZnWl2TEzP5om45msTG2yXToFYnmYF4AWDTXYrJHZcnrjnxXv+OdqsT5C0ghY/atLdhWP4XOUIsI7p5/D3Fg4gDz9YXFtKsJssGikWELqUoamf7IHhr38XwUtfaAyURXavXLxQAATFMI6kfkp/TJrfaDXVfy1hhe9PFoy4wdtXm8GPm2RjtZSWdAp/DIxuQKaQ3tMkRSxHXCcQLDDylL264cTvuT2SWiTWqRYDdVLCloCdcLj4kFCfCkd2NXiWhUhT57zRx0jFxhDa6D9Hqggi8jS4H8mXvvdVp/JBvLxu6dTSO6R6iGng4yaJQaOb9uZ3XnowJ7gRoUAVrz7b8kOWwSXiH0fPjRgqQ3GsNaa5/QqjdLQHt1VwVc1DTOJ/4ugnYRzcADAaI2+KEsTWVadUgaYCLM29erG6+hI5ohepitQHUCaEbSZTlYAC4wr1/id2yUgnVeupHvOqdDVjyoxd+b9vsZaPSbJoTtVSUpQR++AizuusmUiPmRbI3LEc8Ojf11+QNoNlxo37ur7OX5xq3LlSU+A9V65KBQYNlCaBVqSraYQ0hLUSs1H6rOBz/+hT/CMaCMbWD4ZNgjRNOERAOepMFaHCIzyZJRSPztMsAof7zExYrFkSZ3HuUsylhM+CxGqKZWe90AYQOvRlOlHVUevyV+BGczhqPiHNcB9q52Dzrf/CwIZtdfHIwYxRGm7V1aExE0bBuZ535b8mkbF1fr7ePfdZ3bKTNv/pSipcHJfQpda8FlKjyKZ2JEZypwZAxxtIYcyfB8/9vXYQFbAuRvTCe+4dW76/HzKH12AfNFmq+EDwl/Nf0tc1vFjOUCJkMQsuBROEu2Wyku2ZI1M8w7cff4cIxYtR54kg3cN3V6zdR3dHObjFRmqYMZrTS5jO9RTHcORkOUGvGh8RA79DLsVQFiX0AONwn6oPib6Wv2sGNTf1LvIFc31J8Mu7ddPwGPxViJg38HFgZhUwnVCCS7wBNa9igRcM+ZvyZmsz4cKxBw1ZCDM58CfBsrD/F1MIY+XzMpKM76eFQXmA1teevNHfuDyCNQkM1IujG6zr1M2qoqpmkBDPdtC3KQ3aXrLbJfreFtlwLz1a3VGJU4opn/oxfZxfS7DSVH1En0ss3FAyUDuvB7bFiQHNUjsvg9MptHpIKTnCxDsgzVmvcTnbwBueJWvND+od+Pc5TOSMhkDGQPzRZ74FGllgr28B5f3AJCnijbNZ7l3iTxJWB+13Eyrw66bDWwdPVlE3RtHi2ZHB0MiVsYJ6xHXxWE5uP1EFRJ3BNiaWE+gStZ6nqRes3h5UVfcmQgasT1mClsUa+BPMpfDRInIwGUhSedN72rcTaCpNBHoGrKSQYqVqDiPr0LSgv8OZ7eJ8xeNOikBqd8wbX0v46aCF2gl4FmwsCKgvxAwnh3rrU6Fz5aMFxCUTA+MCEwCQYFKw4DAhoFAAQUWNwdo2TTaG/pJA13ZtfytFwyQt0EFAWBr+BamgLh50h4PIbRd6I6s/UMAgMBhqA=
Copy link
Member

Choose a reason for hiding this comment

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

We can't store this cert like this. Anyone has access, security issue.
we need to have logic that 1) checks for a stored cert (in the vault) and uses that, or 2) request a new cert if none is present and store that in the vault.

Copy link
Author

Choose a reason for hiding this comment

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

Bugger. It was a really elegant solution, but security trumps beauty.
I'll look into using Hashicorp Vault again.

@mikeln
Copy link
Member

mikeln commented Dec 18, 2018

I think for now, change all this to allow cert and key files to be read off a local disk with the filenames specified via --set , , etc on the chart install.

Keep the original non-SSL tests as they were. Add placeholders for SSL tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants