cli: fix default certs-dir#87450
Conversation
|
This change is |
|
I'm not sure where to test this. do the |
pkg/security/certnames/locator.go
Outdated
| // MakeLocator initializes a Locator. | ||
| func MakeLocator(certsDir string) Locator { | ||
| return Locator{certsDir: certsDir} | ||
| return Locator{certsDir: os.ExpandEnv(certsDir)} |
There was a problem hiding this comment.
This doesn't seem right. The expansion should be done by the shell, not crdb.
Where is this $HOME default defined?
There was a problem hiding this comment.
oh I found it - base/config.go: DefaultCertsDirectory = "${HOME}/.cockroach-certs"
|
The proper fix is to change the or alternatively, keep the constant as-is, but un-export it, then define a new function What's important is to only apply the expansion on the default, but not on the string provided by the user on the command line. |
Why is that desired? That would be a regression (or at least, a backwards incompatible change) from the behavior of v22.1. |
|
That behavior was a misdesign, as discussed in #81298. |
|
Also, the bug your PR fixes here was an oversight in #81298 and so would like to apologize for the inconvenience. |
d4def44 to
4dd2562
Compare
cockroachdb#82020 accidentally broke the logic for the certs-dir defaulting to ${HOME}/.cockroach-certs/ No release note since this was not released. Release note: None Release justification: low risk bug fix to new functionality.
4dd2562 to
a78fc8b
Compare
|
tftr! bors r=knz |
|
Build succeeded: |
fixes #87191
#82020 accidentally broke
the logic for the certs-dir defaulting to ${HOME}/.cockroach-certs/
No release note since this bug was not released.
Release note: None
Release justification: low risk bug fix to new functionality.