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

rabbitmq: Secret values are rotated on upgrade #980

Closed
kujenga opened this Issue Apr 30, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@kujenga
Contributor

kujenga commented Apr 30, 2017

After performing an upgrade of the RabbitMQ chart, the secrets get set to new values, breaking authentication where it is used elsewhere in the cluster.

Here is a simple proof of concept for the issue, the echoed value will be different after the upgrade, but the persistent state for RabbitMQ still has the original password stored.

helm install stable/rabbitmq --name amqp
echo $(kubectl get secret amqp-rabbitmq --template '{{index .data "rabbitmq-password"}}' | base64 -D)
helm upgrade amqp stable/rabbitmq
echo $(kubectl get secret amqp-rabbitmq --template '{{index .data "rabbitmq-password"}}' | base64 -D)

I could go the route of setting the password to a persistent value that I define at install time, but is there any way to make this work without adding external password management?

@meenie

This comment has been minimized.

Show comment
Hide comment
@meenie

meenie May 7, 2017

Contributor

I have this issue right now in the RethinkDB chart I've just created. I'm not really sure what would be a good fix...

Contributor

meenie commented May 7, 2017

I have this issue right now in the RethinkDB chart I've just created. I'm not really sure what would be a good fix...

@kujenga

This comment has been minimized.

Show comment
Hide comment
@kujenga

kujenga May 7, 2017

Contributor

Yeah, my original intention was to follow this up with a PR fixing the issue, but I couldn't find a path forward. There's a warning about this issue in the docs as well: https://github.com/kubernetes/helm/blob/master/docs/charts_tips_and_tricks.md#be-careful-with-generating-random-values

There is ongoing discussion within Helm [1] [2] to add first class support for secrets, hopefully that functionality will be able to solve this problem as well.

[1] helm/helm#2196
[2] helm/helm#2083

Contributor

kujenga commented May 7, 2017

Yeah, my original intention was to follow this up with a PR fixing the issue, but I couldn't find a path forward. There's a warning about this issue in the docs as well: https://github.com/kubernetes/helm/blob/master/docs/charts_tips_and_tricks.md#be-careful-with-generating-random-values

There is ongoing discussion within Helm [1] [2] to add first class support for secrets, hopefully that functionality will be able to solve this problem as well.

[1] helm/helm#2196
[2] helm/helm#2083

@meenie

This comment has been minimized.

Show comment
Hide comment
@meenie

meenie May 7, 2017

Contributor

Looks like I might have to stop doing a random password until this can be fixed. Also, leaking passwords in Tiller isn't great as well ><.

Contributor

meenie commented May 7, 2017

Looks like I might have to stop doing a random password until this can be fixed. Also, leaking passwords in Tiller isn't great as well ><.

meenie added a commit to meenie/charts that referenced this issue May 7, 2017

meenie added a commit to meenie/charts that referenced this issue May 7, 2017

prydonius added a commit that referenced this issue Jun 13, 2017

rethinkdb: Initial Commit (#1018)
* rethinkdb-cluster: Initial Commit

* Fix chart name.

* Make the NOTES.txt file more accurate.

* Use k8s 1.6 storageClassName notation for PVC.

* RethinkDB directory needs to be initially empty.

When using a provisioned volume from something like GCE, the mounted directory has a  folder inside of it. This breaks RethinkDB when initializing.

* Add an extra k8s script to create a StorageClass on GCE.

* Addressing PR Comments.

* Rename to Recursively removing directory /Users/codyl/dev-files/charts/rethinkdb_data/tmp
Initializing directory /Users/codyl/dev-files/charts/rethinkdb_data
Running rethinkdb 2.3.4 (CLANG 7.3.0 (clang-703.0.31))...
Running on Darwin 16.6.0 x86_64
Loading data from directory /Users/codyl/dev-files/charts/rethinkdb_data and move to stable.

* Shorten down .gitignore file.

* Make using the RethinkDB Admin Console easier.

* Fix some wrong names/paths.

* Due to a bug with secrets, don't auto-gen a password.

#980

* Remove extra file.

* Addressing PR comments and some other changes

 - Updated NOTES.txt to include more tailored instructions based on proxy service type.
 - Added an optional StorageClass.
 - Simplified rethink config to not use a ConfigMap.
 - Added optional RethinkDB Driver TLS configuration.
 - Updated README.md to reflect new config changes.

* Tiny fix in README.md.

* Use github username for Chart.yaml.

* Default to ClusterIP for proxy service.

flah00 added a commit to Adaptly/charts that referenced this issue Jun 28, 2017

rethinkdb: Initial Commit (#1018)
* rethinkdb-cluster: Initial Commit

* Fix chart name.

* Make the NOTES.txt file more accurate.

* Use k8s 1.6 storageClassName notation for PVC.

* RethinkDB directory needs to be initially empty.

When using a provisioned volume from something like GCE, the mounted directory has a  folder inside of it. This breaks RethinkDB when initializing.

* Add an extra k8s script to create a StorageClass on GCE.

* Addressing PR Comments.

* Rename to Recursively removing directory /Users/codyl/dev-files/charts/rethinkdb_data/tmp
Initializing directory /Users/codyl/dev-files/charts/rethinkdb_data
Running rethinkdb 2.3.4 (CLANG 7.3.0 (clang-703.0.31))...
Running on Darwin 16.6.0 x86_64
Loading data from directory /Users/codyl/dev-files/charts/rethinkdb_data and move to stable.

* Shorten down .gitignore file.

* Make using the RethinkDB Admin Console easier.

* Fix some wrong names/paths.

* Due to a bug with secrets, don't auto-gen a password.

helm#980

* Remove extra file.

* Addressing PR comments and some other changes

 - Updated NOTES.txt to include more tailored instructions based on proxy service type.
 - Added an optional StorageClass.
 - Simplified rethink config to not use a ConfigMap.
 - Added optional RethinkDB Driver TLS configuration.
 - Updated README.md to reflect new config changes.

* Tiny fix in README.md.

* Use github username for Chart.yaml.

* Default to ClusterIP for proxy service.

c-knowles added a commit to c-knowles/helm-charts that referenced this issue Jun 29, 2017

rethinkdb: Initial Commit (#1018)
* rethinkdb-cluster: Initial Commit

* Fix chart name.

* Make the NOTES.txt file more accurate.

* Use k8s 1.6 storageClassName notation for PVC.

* RethinkDB directory needs to be initially empty.

When using a provisioned volume from something like GCE, the mounted directory has a  folder inside of it. This breaks RethinkDB when initializing.

* Add an extra k8s script to create a StorageClass on GCE.

* Addressing PR Comments.

* Rename to Recursively removing directory /Users/codyl/dev-files/charts/rethinkdb_data/tmp
Initializing directory /Users/codyl/dev-files/charts/rethinkdb_data
Running rethinkdb 2.3.4 (CLANG 7.3.0 (clang-703.0.31))...
Running on Darwin 16.6.0 x86_64
Loading data from directory /Users/codyl/dev-files/charts/rethinkdb_data and move to stable.

* Shorten down .gitignore file.

* Make using the RethinkDB Admin Console easier.

* Fix some wrong names/paths.

* Due to a bug with secrets, don't auto-gen a password.

helm#980

* Remove extra file.

* Addressing PR comments and some other changes

 - Updated NOTES.txt to include more tailored instructions based on proxy service type.
 - Added an optional StorageClass.
 - Simplified rethink config to not use a ConfigMap.
 - Added optional RethinkDB Driver TLS configuration.
 - Updated README.md to reflect new config changes.

* Tiny fix in README.md.

* Use github username for Chart.yaml.

* Default to ClusterIP for proxy service.

yanns pushed a commit to yanns/charts that referenced this issue Jul 28, 2017

rethinkdb: Initial Commit (#1018)
* rethinkdb-cluster: Initial Commit

* Fix chart name.

* Make the NOTES.txt file more accurate.

* Use k8s 1.6 storageClassName notation for PVC.

* RethinkDB directory needs to be initially empty.

When using a provisioned volume from something like GCE, the mounted directory has a  folder inside of it. This breaks RethinkDB when initializing.

* Add an extra k8s script to create a StorageClass on GCE.

* Addressing PR Comments.

* Rename to Recursively removing directory /Users/codyl/dev-files/charts/rethinkdb_data/tmp
Initializing directory /Users/codyl/dev-files/charts/rethinkdb_data
Running rethinkdb 2.3.4 (CLANG 7.3.0 (clang-703.0.31))...
Running on Darwin 16.6.0 x86_64
Loading data from directory /Users/codyl/dev-files/charts/rethinkdb_data and move to stable.

* Shorten down .gitignore file.

* Make using the RethinkDB Admin Console easier.

* Fix some wrong names/paths.

* Due to a bug with secrets, don't auto-gen a password.

helm#980

* Remove extra file.

* Addressing PR comments and some other changes

 - Updated NOTES.txt to include more tailored instructions based on proxy service type.
 - Added an optional StorageClass.
 - Simplified rethink config to not use a ConfigMap.
 - Added optional RethinkDB Driver TLS configuration.
 - Updated README.md to reflect new config changes.

* Tiny fix in README.md.

* Use github username for Chart.yaml.

* Default to ClusterIP for proxy service.
@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Dec 24, 2017

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

fejta-bot commented Dec 24, 2017

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jan 23, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

fejta-bot commented Jan 23, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

@kujenga

This comment has been minimized.

Show comment
Hide comment
@kujenga

kujenga Jan 24, 2018

Contributor

As I understand it this is an issue more fundamental to Helm, rather than something specific to this chart, which might be better tracked here: helm/helm#2196

Contributor

kujenga commented Jan 24, 2018

As I understand it this is an issue more fundamental to Helm, rather than something specific to this chart, which might be better tracked here: helm/helm#2196

@kujenga kujenga closed this Jan 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment