Support user configs, user secrets and separate environments for cassandra and sidecar #218

alourie · 2019-08-02T06:09:09Z

Fixes #213

Signed-off-by: Alex Lourie <[email protected]>

Fixes instaclustr#213 Fixes instaclustr#208 * Fixed some comments from PR review * Added Env to CRD to allow specifying environment for containers (exists in java version) * Added userConfigMap handling (instaclustr#213) * Restored PrivelegedSupported handling (instaclustr#208) * Backup secret volume allows providing GOOGLE_APPLICATION_CREDENTIALS in a secret * Cloud providers creds can be set using Env field in CRD Signed-off-by: Alex Lourie <[email protected]>

Signed-off-by: Alex Lourie <[email protected]>

examples/go/example-datacenter.yaml

zegelin · 2019-08-02T17:38:44Z

examples/go/example-datacenter.yaml

+        path: creds.json
+  env:
+    - name: GOOGLE_APPLICATION_CREDENTIALS
+      value: "/etc/google/creds.json"


Is this the "default"? If so, do we need to set it as an environment variable?

There does not seem to be any default https://cloud.google.com/docs/authentication/getting-started

i checked the source code, there is not any default location

yep, no defaults for that one. This is just an example of where to place that file, I literally look it up in the env and if it's there - we use it. If not defined, we'll use /etc/gcp, but I might change that to something more generic.

Looks like the default process is this: https://cloud.google.com/docs/authentication/production#finding_credentials_automatically

So yes, we need a way to define the environment variable, but that's probably also going to be the more uncommon approach, as on GKE the credentials will be automatically discovered.

C* clusters running on GKE would most likely backup to Google Cloud Storage. Same on Amazon EKS -- S3 is the most likely destination. But we don't want to prevent clusters running on other K8s environments from accessing cloud storage, so having the option is always nice. Could also be useful for GKE -> EKS migrations for example.

Yea, it's a convenient option, and it's optional. Also, might be that even on GKE they might want to use some different account for backups, and that's when it would be handy.

examples/go/example-datacenter.yaml

pkg/controller/cassandradatacenter/statefulset.go

pkg/apis/cassandraoperator/v1alpha1/cassandradatacenter.go

pkg/controller/cassandradatacenter/statefulset.go

Signed-off-by: Alex Lourie <[email protected]>

zegelin · 2019-08-05T20:35:16Z

pkg/controller/cassandradatacenter/statefulset.go

@@ -24,21 +23,9 @@ const (
 	OperatorConfigVolumeMountPath = "/tmp/operator-config"
 	UserConfigVolumeMountPath     = "/tmp/user-config"
 	UserSecretVolumeMountPath     = "/tmp/user-secret-config"
+	BackupSecretVolumeMountPath   = "/tmp/backup-secret"


What's the difference between BackupSecret and UserSecret?

The difference is in fact that BackupSecret would hold the credentials for the backup functionality (i.e. GCP and maybe AWS/Azure should they choose to) and will be mounted in the sidecar container, while UserSecret is for C* encryption/auth credentials and to be mounted in the Cassandra container. Having backup stuff mounted in the cassandra container or cassandra credentials in the sidecar container, while not horrible, feels pretty hacky and dirty. That's why the split.

Maybe change to sidecarSecret/cassandraSecret? So it relates to the pod name

I don't mind the C* and sidecar containers sharing secrets to be honest.

Well, I can rename it, but I think it's better called a "backup" secret as it hints the usage for backups only.

Actually, the only other "secrets" usage would be for secure communications as far as I can tell, and we're going to resolve it differently anyway. So I'd just keep it as "backup" for now, and if ever we combine "user" and "backup" secrets, we can change the naming convention.

zegelin · 2019-08-05T20:35:41Z

pkg/controller/cassandradatacenter/statefulset.go

@@ -24,21 +23,9 @@ const (
 	OperatorConfigVolumeMountPath = "/tmp/operator-config"
 	UserConfigVolumeMountPath     = "/tmp/user-config"
 	UserSecretVolumeMountPath     = "/tmp/user-secret-config"


This probably should just be /tmp/user-secret for consistency.

zegelin · 2019-08-05T20:45:35Z

docker/cassandra/entry-point

@@ -12,4 +12,12 @@ do
 done
 )

+# Copy over cqlshrc to a default location if defined as ENV


I think we can make this nicer. It IMO shouldn't be a requirement that users have to define an environment variable and add a file to a config map to get cqlsh to function.

Instead I suggest that we write our own wrapper for cqlsh that detects if SSL is enabled, and fetches the CA cert (or just disables trust checking). This will also be necessary once we support authn/authz, since we need to implement shared-secret support.

As a side question, should cqlsh be installed the C* container at all? If people want to connect to the cluster, should they instead start a new pod/container that just contains cqlsh?

@zegelin we currently use cqlsh now in cql-readiness-probe, and this is easily replaceable with either python script or, say, a statically compiled gocql simple program. So we don't have an inherent requirement to have it in the container at all.

Also, if people want to use cqlsh to connect to the cluster, they can either spin a new container with all the certificates and custom configs as they need to, or have it installed locally if they have the network access to the cluster. So, if you think that's a good path to follow, I can replace the readiness script with something else pretty quickly and stop the dependency on cqlsh.

I did rush it a bit. Clearly, any "script" we use will have to know about SSL configuration to be able to connect to the node. So having a wrapper around cqlsh might be a useful and simpler solution.

In order to get the PR closed, I think we should create a new issue/PR for the wrapper script and solve that one separately.

yea, I'm going to remove this part and deal with that in #241

Signed-off-by: Alex Lourie <[email protected]>

zegelin · 2019-08-08T19:05:41Z

Is this ready to be merged?

alourie · 2019-08-09T06:39:54Z

@zegelin it can be merged, but the "secure" Cassandra needs more work. I'd prefer merging this, as it fixes a bunch of tickets/features/bugs, and continuing work on proper TLS support separately. If we do it, I'll rename the merge to "Support backups secrets, user secrets and user maps" to avoid misunderstanding.

benbromhead

Overall lgtm - I think wrap up the rebase on existing changes and the above nits and lets get this merged

benbromhead · 2019-08-19T19:01:15Z

pkg/controller/cassandradatacenter/statefulset.go

@@ -24,21 +23,9 @@ const (
 	OperatorConfigVolumeMountPath = "/tmp/operator-config"
 	UserConfigVolumeMountPath     = "/tmp/user-config"
 	UserSecretVolumeMountPath     = "/tmp/user-secret-config"
+	BackupSecretVolumeMountPath   = "/tmp/backup-secret"


Maybe change to sidecarSecret/cassandraSecret? So it relates to the pod name

I don't mind the C* and sidecar containers sharing secrets to be honest.

benbromhead · 2019-08-19T19:02:58Z

docker/cassandra/entry-point

@@ -12,4 +12,12 @@ do
 done
 )

+# Copy over cqlshrc to a default location if defined as ENV


In order to get the PR closed, I think we should create a new issue/PR for the wrapper script and solve that one separately.

…assandra-operator into keystore_178

smiklosovic · 2019-08-20T07:02:49Z

looks good to me

Signed-off-by: Alex Lourie <[email protected]>

alourie · 2019-08-20T14:23:13Z

Now can be squashed and merged @benbromhead

alourie added 3 commits July 27, 2019 00:48

Initial work for clouds secrets and user-defined configmap

c8cdba9

Signed-off-by: Alex Lourie <[email protected]>

Support TLS certificates for internal communication

e7109f6

Signed-off-by: Alex Lourie <[email protected]>

alourie requested review from zegelin and benbromhead August 2, 2019 06:10

zegelin reviewed Aug 2, 2019

View reviewed changes

alourie added 2 commits August 6, 2019 01:16

PR comments, docs updates

f3f49bb

Signed-off-by: Alex Lourie <[email protected]>

Cleanups

a38eb32

Signed-off-by: Alex Lourie <[email protected]>

alourie marked this pull request as ready for review August 5, 2019 15:57

zegelin reviewed Aug 5, 2019

View reviewed changes

alourie mentioned this pull request Aug 6, 2019

Initial work for clouds secrets and user-defined configmap #205

Closed

Path naming update

1af32c3

Signed-off-by: Alex Lourie <[email protected]>

alourie mentioned this pull request Aug 6, 2019

Setting sysctl parameters on nodes #208

Closed

alourie changed the title ~~Support TLS certificates~~ Support user configs, user secrets and separate environments for cassandra and sidecar Aug 16, 2019

alourie mentioned this pull request Aug 19, 2019

Don't require IPC_LOCK and SYS_RESOURCE capabilities by default #237

Closed

benbromhead approved these changes Aug 19, 2019

View reviewed changes

alourie added 2 commits August 20, 2019 15:24

Cleanups

fa47f90

Merge branch 'superdupertopsecretrewrite' of github.com:instaclustr/c…

a860bbe

…assandra-operator into keystore_178

go fmt

aff1123

Signed-off-by: Alex Lourie <[email protected]>

benbromhead merged commit df5ef00 into instaclustr:superdupertopsecretrewrite Aug 21, 2019

This was referenced Aug 22, 2019

Port user config map support. #213

Closed

Allow running on unprivileged clusters #116

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support user configs, user secrets and separate environments for cassandra and sidecar #218

Support user configs, user secrets and separate environments for cassandra and sidecar #218

alourie commented Aug 2, 2019 •

edited

Loading

zegelin Aug 2, 2019

smiklosovic Aug 4, 2019

smiklosovic Aug 4, 2019

alourie Aug 5, 2019

zegelin Aug 5, 2019

alourie Aug 6, 2019

zegelin Aug 5, 2019

alourie Aug 6, 2019 •

edited

Loading

benbromhead Aug 19, 2019

alourie Aug 20, 2019

zegelin Aug 5, 2019

alourie Aug 6, 2019

zegelin Aug 5, 2019

alourie Aug 6, 2019

alourie Aug 6, 2019 •

edited

Loading

benbromhead Aug 19, 2019

alourie Aug 20, 2019

zegelin commented Aug 8, 2019

alourie commented Aug 9, 2019

benbromhead left a comment

benbromhead Aug 19, 2019

benbromhead Aug 19, 2019

smiklosovic commented Aug 20, 2019

alourie commented Aug 20, 2019

Support user configs, user secrets and separate environments for cassandra and sidecar #218

Support user configs, user secrets and separate environments for cassandra and sidecar #218

Conversation

alourie commented Aug 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alourie Aug 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alourie Aug 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zegelin commented Aug 8, 2019

alourie commented Aug 9, 2019

benbromhead left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smiklosovic commented Aug 20, 2019

alourie commented Aug 20, 2019

alourie commented Aug 2, 2019 •

edited

Loading

alourie Aug 6, 2019 •

edited

Loading

alourie Aug 6, 2019 •

edited

Loading