charts icon indicating copy to clipboard operation
charts copied to clipboard

Add missing HTTPS configs

Open Darrekt opened this issue 2 years ago • 8 comments

While trying to follow the official instructions for TLS/HTTPS, I realised that a lot of the required flags in config.properties for the coordinator were not yet implemented.

This PR attempts to implement them to the best of my knowledge. This is my very first open-source contribution, so if I did something wrong I am happy to receive feedback and fix my mistakes / convention breaks accordingly.

The following changes were made:

  • secretMounts was incorrectly implemented: In deployment-coordinator.yaml, the initContainers clause was incorrectly in the middle of the volumes clause, interrupting the place where the secretMounts would be added. I've fixed this to permit correct volume addition for certificates which I assume would be mounted using this mechanism.
  • keystore.keystorePassword was previously not implemented to support keystores with passwords.
  • keystore.keymanagerPassword was similarly missing.
  • Added support for the allowInsecureOverHttp flag.

Edit: Would also like some help on getting around the CLA checks. I've submitted my signed CLA a few days ago, and signed my commits with my email using git rebase --signoff HEAD~2 after setting my email in my git client, but still getting the error.

Darrekt avatar Mar 10 '23 08:03 Darrekt

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Darrick Lau. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot[bot] avatar Mar 10 '23 08:03 cla-bot[bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Darrick Lau. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot[bot] avatar Mar 10 '23 08:03 cla-bot[bot]

@cla-bot check

Darrekt avatar Mar 10 '23 08:03 Darrekt

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Darrick Lau. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot[bot] avatar Mar 10 '23 08:03 cla-bot[bot]

The cla-bot has been summoned, and re-checked this pull request!

cla-bot[bot] avatar Mar 10 '23 08:03 cla-bot[bot]

Hi! We were about to embark on making similar changes to the chart but stumbled upon the PR. Any chance someone can merge this?

mach-kernel avatar Apr 21 '23 15:04 mach-kernel

Yep. I'm happy to integrate changes if I get some feedback. Also, some help on how to appease the cla-bot would be appreciated. I've signed my commits, but it's still complaining.

Darrekt avatar Apr 25 '23 04:04 Darrekt

I'm having some problems when trying have an https connection to my cluster as well, how did you managed to do it, can you give me some guidance in it please?

Torugo avatar Jan 19 '24 03:01 Torugo

Adding the missing properties is a good change, but this PR is abandoned, so I'm closing it.

nineinchnick avatar Aug 24 '24 08:08 nineinchnick