config: add etcd authentication #1506
Conversation
| #### Using systemd Drop-Ins | ||
|
|
||
| ```ini | ||
| [Service] |
There was a problem hiding this comment.
if you provide etcd_servers in examples below, put it here too. if you use https protocol, probably it worth to mention TLS keypairs described above.
|
@jipperinbham nice patch. could you also add extra info into Documentation/using-the-client.md? |
jipperinbham
force-pushed
the
etcd-basic-auth
branch
from
b1a957a to
811d555
Compare
|
@kayrus I made some additions to both Documentation/deployment-and-configuration.md and Documentation/using-the-client.md but let me know if the wording needs some adjustments. I also ditched the |
| Environment="FLEET_ETCD_PASSWORD=coreos" | ||
| ``` | ||
|
|
||
| #### Using CoreOS Cloud Config |
There was a problem hiding this comment.
This will need to actually be added to cloudconfig to be valid: https://github.com/coreos/coreos-cloudinit/blob/master/config/fleet.go
There was a problem hiding this comment.
@jonboulle thanks for the heads up, I've submitted coreos/coreos-cloudinit#418
There was a problem hiding this comment.
Would be nice, if the documentation could also show how to use TLS authentication by default.
Thanks!
There was a problem hiding this comment.
@tixxdz doc already contains info on how to use tls auth: https://github.com/coreos/fleet/blob/master/Documentation/deployment-and-configuration.md#tls-authentication
|
@jonboulle should we create tests for this PR? otherwise let's just close #1447 issue and merge this PR. |
fleetctl/fleetctl.go
Outdated
|
|
||
| globalFlagset.StringVar(&globalFlags.EtcdUsername, "etcd-username", "", "Username used to authenticate with etcd") | ||
| globalFlagset.StringVar(&globalFlags.EtcdPassword, "etcd-password", "", "Password used to authenticate with etcd") | ||
|
|
There was a problem hiding this comment.
Thanks for the documentation and the patch, but honestly I don't like the fact that username and password will show up on the cmdline where every process will just grab them... so could you please make just a one option where it reads the input from the config file ?
Thank you!
There was a problem hiding this comment.
hmm do we have any other config variables that can be set from the config file but not CLI?
There was a problem hiding this comment.
hmm, I'm not aware of any other variables that works like that!
but if I'm not mixing things this code touches the old etcd API which was deprecated right ? and in that case do we need to expose these information ? it will be world readable and having a vulnerable node will just expose the whole cluster...
In fleetd it's fine, since it is retrieved from the configuration file. IMO the only thing to do if you are writing systemd drop-ins with such content is perhaps to make sure how you store that information.
|
Bump |
|
Thanks for the bump. Overall it looks ok to me, but I also share with @tixxdz concerns about exposing username/password via cmdline. But actually I don't have strong opinions. |
jipperinbham
force-pushed
the
etcd-basic-auth
branch
from
811d555 to
7c219c4
Compare
|
@dongsupark @tixxdz I removed etcd username and password options from |
|
|
||
| ### Using etcd Authentication | ||
|
|
||
| If your `etcd` cluster is configured with authentication enabled, use the `--etcd-username` and `--etc-password` flags to provide credentials to the command-line tool. |
There was a problem hiding this comment.
One minor nit: is this description still valid, given that the options are now removed from fleetctl?
|
@jipperinbham BTW this PR needs a rebase. As soon as a minor document change gets done, I'd be ready to merge it. Functional test failed, but I suppose that's just an occasional failure, which has nothing to do with this PR. |
dongsupark
force-pushed
the
master
branch
2 times, most recently
from
14580b0 to
63b20dc
Compare
dongsupark
force-pushed
the
master
branch
3 times, most recently
from
23d4b13 to
6fb1256
Compare
dongsupark
force-pushed
the
master
branch
2 times, most recently
from
54409ab to
a1a21b8
Compare
|
@jipperinbham gentle ping. |
|
Closed in favor of #1630. Thanks. |
6 participants
expose username and password as options for configuration to etcd endpoints
Fixes #1505