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

Added initial support for MySQL as a database backend #16

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

maugustosilva
Copy link
Contributor

Since it was added as subchart (from bitnami), the following steps are needed before deployment.

cd build/helm
helm dependency update keylime

In addition to it, tweaked a little bit the messages in NOTES.txt

@maugustosilva maugustosilva requested a review from mheese July 28, 2023 18:27
@maugustosilva
Copy link
Contributor Author

@mheese Just realized that I left the DB password in plaintext in values.yaml. Any advice on "best practices" on this is welcome (maybe create a "persistent" secret with a password?)

@mheese
Copy link
Contributor

mheese commented Aug 1, 2023

@maugustosilva awesome! great to see that this is coming together!

To come up with good defaults for secrets, I always opted to generate some as a Kubernetes secret with an init job (similarly to what we do with the CA right now). This prevents that there are some default passwords floating around that attackers could try to gain access to instances like this. Additionally, like in the other cases, one should be able to specify a secret though which will prevent it from being created. This allows preprovisioning of it as well.

To make things in the helm charts a bit easier, I was already thinking that we might simply want to eliminate the init job subchart and move it to the main chart. In general I used subcharts more for structural reasons and making it easier for others to look at the charts than anything else (well, and of course so that it is easy to enable/disable the deployment of them).

As for updating dependencies in the helm chart, I always felt so-so about that. As MySQL isn't the "default" though, I think it is probably okay, as long as that is not a necessary step to begin with. Alternatively, we can just package up the version that we know works - depends on the licensing of the other helm chart of course. Personally I don't like to have an external dependency on availability of another chart, but that might just be me being paranoid.

I will review the PR later this week! Unfortunately, this week I'm going to be busy with some other non-keylime things as well.

@maugustosilva
Copy link
Contributor Author

@mheese thanks for your answers! So, on my last update, which I just pushed, I tried my very best to have re-usable randomly generated passwords. The code looks syntactically correct (making use of the "lookup" function), but something is not quite right yet (i.e., every time I call with helm upgrade after helm uninstal, the secrets are deleted and re-generated).

I have attempted to use the trick described at helm/helm-www#1259 (comment), but couldn't finish it. Please take a look, and let me know. Here is what I would like to have:

global:
...
  # ca manages the "cv_ca" of keylime
  ca:
    # generate means that an initialization job will run in the pre-install phase which will generate the CV CA and create     # leave it empty, and new password, maintained accross multiple upgrades, will be generated
    password: ""
...
  database:
...
    mysql:
      # leave it empty, and a new password, maintained accross multiple upgrades, will be generated
      password: ""

Where an empty password would become a randomly generated password which is then stored into a Secret and constantly re-used. I am surprised on how difficult it is, judging from the answers :-)

@maugustosilva
Copy link
Contributor Author

maugustosilva commented Aug 2, 2023

Ok, managed to get something that works with auto-generated passwords. It survives multiple invocations of helm upgrade following the first helm install. The solution was to store the password into a dictionary inside .Release, given that it is available to all subcharts. Annoyingly it does not survive helm install -> helm uninstall -> helm install (the Secret containing the password are deleted at the beginning of the installation).

Since it was added as subchart (from bitnami), the following steps are
needed before deployment.

```
cd build/helm
helm dependency update keylime
```

In addition to it, tweaked a little bit the messages in NOTES.txt

This uses randomly generated password which persist accross upgrades

Here we also have a new Makefile, providing options to deploy/undeploy/update/debug

Signed-off-by: Marcio Silva <marcio.a.silva@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants