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

Add a new value neo4j.passwordFromSecret #118

Merged
merged 11 commits into from
Nov 24, 2022

Conversation

ojhughes
Copy link
Collaborator

@ojhughes ojhughes commented Nov 23, 2022

Deploying a cluster using tools such as Terraform or Argo leads to a data race where the Helm chart creates a secret for Neo4j auth. Add a new value neo4j.passwordFromSecret so the user can provide the auth secret to be used. This is more secure and avoids the secret creation data race

  • update integration tests
  • Fix warning message
  • Polish NOTES.txt to output command to retrieve password when using passwordFromSecret

…sswordFromSecret

Remove password failure test as it cannot be checked when user provides the secret
@ojhughes ojhughes force-pushed the bugfix/secret-creation-data-race branch from 017d527 to ab81ac0 Compare November 23, 2022 10:21
@ojhughes ojhughes changed the title bugfix/secret creation data race Add a new value neo4j.passwordFromSecret Nov 23, 2022
remove PVC patching as it logs an error
Comment on lines 102 to 104
{{- if .Values.neo4j.passwordFromSecret -}}
{{- printf "$(kubectl get secret %s -o go-template='{{.data.NEO4J_AUTH | base64decode }}' | cut -d '/' -f2) " .Values.neo4j.passwordFromSecret -}}
{{- end -}}
Copy link

Choose a reason for hiding this comment

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

Considering this was provided independently, do we really want to print it, and risk having it exposed in logs for example ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. This only prints the command to run, not the actual value eg;

Your release "server1" has been installed  in namespace "neo4j".

The neo4j user's password has been set to "$(kubectl get secret test-auth -o go-template='{{.data.NEO4J_AUTH | base64decode }}' | cut -d '/' -f2) ".To view the progress of the rollout try:

  $ kubectl --namespace "neo4j" rollout status --watch --timeout=600s statefulset/server1

Once rollout is complete you can log in to Neo4j at "neo4j://server1.neo4j.svc.cluster.local:7687". Try:

  $ kubectl run --rm -it --namespace "neo4j" --image "neo4j:5.1.0-enterprise" cypher-shell \
     -- cypher-shell -a "neo4j://server1.neo4j.svc.cluster.local:7687" -u neo4j -p "$(kubectl get secret test-auth -o go-template='{{.data.NEO4J_AUTH | base64decode }}' | cut -d '/' -f2) "

Copy link

Choose a reason for hiding this comment

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

Indeed I read a bit fast the templating and got tricked by the comment in the description. Thanks !

neo4j/templates/_helpers.tpl Show resolved Hide resolved
neo4j/templates/_helpers.tpl Outdated Show resolved Hide resolved
neo4j/templates/_helpers.tpl Show resolved Hide resolved
internal/integration_tests/standalone_test.go Outdated Show resolved Hide resolved
internal/model/helm.go Outdated Show resolved Hide resolved
@ojhughes
Copy link
Collaborator Author

@harshitsinghvi22 thanks for the review, I made a few changes in response.
I added auth_tests.go as these are not related to standalone
I added a receiver function to the helm install function, so instead of HelmInstallFromStruct() we have helmClient.Install()

@ojhughes ojhughes merged commit 34cbd7c into dev Nov 24, 2022
@ojhughes ojhughes deleted the bugfix/secret-creation-data-race branch November 24, 2022 12:44
@EvertonSA
Copy link

hi colleagues, when can we get this on 4.4?

This pull request was closed.
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.

5 participants