driplineorg / dripline-python

python implementation of project8/dripline
Apache License 2.0
3 stars 6 forks source link

Feature/chart auth rename #91

Closed raphaelcervantes closed 4 years ago

raphaelcervantes commented 4 years ago

I renamed some fields in the helm charts to make it more obvious that authentications.json can have postgres credentials. For example, rabbitmq-authentications-secret became dl-authentications-secret.

laroque commented 4 years ago

Hey @raphaelcervantes,

I think that this change makes sense with the code as is, but we're hoping to remove this usage as soon as https://github.com/project8/scarab/pull/62 is merged.

The postgres interface class does not use the top-level auth file natively, you have to give that Service its own path to a file with postgres auth details, and the recommendation would be for that to be a different file from the AMQP details with jsut the postgres info (in k8s you'd probably want to make that a second secret). Because extra fields in the json are just ignored, there's nothing to stop you from putting it all in the same file, but this pattern isn't recommended as it means that if you want to update the postgres credentials, you'll end up changing the secret used by services which don't talk to postgres, causing a lifecycle interruption to all running services.

laroque commented 4 years ago

One other note: I don't think it makes sense to put a default value into the secret name. This is a value that the user needs to provide and which needs to match what exists in their mesh. If it doesn't already provide a useful error, I'd like to find a way to make it clear that specifying that value is required.

laroque commented 4 years ago

per the last comment, we don't want to change this field name. The image tag version bump was valuable but has now been outpaced (we're on v4.4.0) and that has been merged.

Hopefully I'll be able to expand/update the second-mesh tutorial in the relative near term to demo the preferred usage pattern.