Fix etcdv3 auth

[Dkhil]

What does this PR do?

fix authification for etcd module when using user/password based auth

Revolution1/etcd3-py#41
What issues does this PR fix or reference?

Fixes
auth when using user/passsword based authentification with etcdv3

[ggiesen]

Thanks for tackling this @Dkhil - the missing auth() call is a genuine gap, since etcd3-py doesn't authenticate until you call it (Revolution1/etcd3-py#41).

One concern with the current change: self.client.auth() is called unconditionally in EtcdClientV3.__init__, even when no etcd.username/etcd.password are configured, or when the etcd server doesn't have auth enabled. In that case etcd3-py's auth() raises:

etcd3.errors.go_etcd_rpctypes_error...ClientError: <ErrAuthNotEnabled error:'etcdserver: authentication is not enabled', code:9>

I reproduced that against etcd v3.5 with auth disabled (the default). As written, this makes EtcdClientV3 fail to construct against any no-auth etcd, which would regress the etcd execution module and state for everyone who isn't using etcd authentication.

Could the call be guarded so it only authenticates when credentials are present? e.g.

self.client = etcd3.Client(host=self.host, port=self.port, **self.xargs)
if self.xargs.get("username") and self.xargs.get("password"):
    self.client.auth()

That keeps your fix for the credentialed path while leaving the no-auth path working. (Heads-up that this constructor is also the connection path for an etcd3 cache backend I'm working on, so I'm keen to see this land solidly - happy to help test.)

[Dkhil]

@ggiesen thanks for catching that, calling auth() unconditionally is a clear regression for no-auth setups. I'll update the PR with the guarded check shortly. And good to know this also impacts the etcd3 cache backend you're working on — happy to coordinate if needed!

[ggiesen]

Guard is exactly the shape I had in mind, thanks for the quick turnaround. One thing on the new test_auth: it confirms the credentials land in xargs and reach etcd3.Client(...), but neither asserts that client.auth() is actually called when creds are present, nor exercises the no-creds path the guard is there to protect (where auth() must NOT be called). Worth adding mock_client_instance.auth.assert_called_once() to the existing test, plus a parallel test with creds-less opts asserting auth.assert_not_called(). Otherwise a future change to either branch of the guard would slip through silently. Happy to factor this into the etcd3 cache backend PR I'm prepping if it helps coordinate.