-
Notifications
You must be signed in to change notification settings - Fork 85
Conversation
359bede
to
39d50a4
Compare
/assign @mikehelmick |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just nits and questions
docs/production.md
Outdated
base64, for example: | ||
|
||
```sh | ||
export COOKIE_KEYS="ARLaFwAqBGIkm5pLjAveJuahtCnX2NLoAUz2kCZKrScUaUkEaxHSvJLVYb5yAPCc441Cho5n5yp8jdEmy6hyig==,RLjcRZeqc07s6dh3OK4CM1POjHDZHC+usNU1w/XNTjM=" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could. you add a command to turn this from base64? Just to make this as dumb simple as possible to follow in case of a breach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand. The openssl
commands output base64, and the envvar accepts base64. Why would we want something that decodes it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, that's not how I read this. I thought this value was a base64 encoded value of an array of base64 strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified.
@@ -11,3 +12,180 @@ configurations are available: | |||
| ----------------------- | ------------------------------- | ----------- | |||
| OpenCensus Agent | `OCAGENT` | Use OpenCensus. | |||
| Stackdriver\* | `STACKDRIVER` | Use Stackdriver. | |||
|
|||
|
|||
## Rotating secrets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot, I look forward to the tooling. Maybe add a TODO for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was toying with the tooling yesterday. It's actually somewhat annoying because Terraform is managing the secret and the envvars.
39d50a4
to
c998a5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Thanks for clarifying!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: icco, sethvargo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This isn't 100% complete, but it's about 95% toward GH-429. I'd eventually like to write a utility in
tools/
that generates the secrets and puts them in Secret Manager so a human never sees them, but didn't want to muddle that with the actual docs/implementation.Release Note