gateway error detail is not passed along in raised exception

[Dan Streetman]

[impact]

when the gateway reports an error, it is not passed along in the exception raised by python-etcd3gw

[test case]

Upstream added unit tests for this bug, which are included in the backport, so a successful package build (which runs the unit tests at build time) is enough to verify the fix.

[regression potential]

* Any regression would likely occur in handling errors sent from the gateway to python-etcd3gw, or in handling or later processing the exception(s) generated from the gateway error(s).

* This patchset introduces 'import mock' in a test unit which requires python3-mock as a Build-Depends.

For now, it is indirectly covered by python3-oslotest as follows:

d/control:
     18 Build-Depends-Indep:
     19 python-coverage,
     20 python-futurist,
     21 python-hacking,
     22 python-nose,
     23 python-oslosphinx,
     24 python-oslotest,
     25 python-pytest,
     26 python-requests,
     27 python-six,
     28 python-testscenarios,
     29 python-testtools,
     30 python-urllib3,
     31 python3-futurist,
     32 python3-nose,
=> 33 python3-oslotest,

~# apt-cache depends python3-oslotest
python3-oslotest
  Depends: python3-fixtures
=> Depends: python3-mock

Ideally, it would be best to have an implicit instruction for it, but since in Stable release, adding new build dependencies is not the preference and that python3-oslotest will unlikely be removed from the list. It shouldn't have any effect in the future. Worse case, python-etcd3gw will stop building, and we will have to implicitly instruct d/control to have python3-mock as Build-Depends.

[scope]

three commits are needed for b/f/g, and one commit is needed in h.

this is fixed upstream with commits:
483a37e28a59e29239dcae7eeabf6f24c1f0b440
19abd85b710682b326702e2290a30d084fb0af71
which are included in v2.5, and commit:
5a3157a122368c2314c7a961f61722e47355f981
which is included in v2.6

Debian currently has v2.5, and MR to add the last commit needed is:
https://salsa.debian.org/openstack-team/python/python-etcd3gw/-/merge_requests/1

Linking to related LP https://bugs.launchpad.net/ubuntu/+source/python-etcd3gw/+bug/1820083

[Heather Lemon (hypothetical-lemon)]

attached debdiff

[Heather Lemon (hypothetical-lemon)]

python-etcd3gw groovy debdiff

[Heather Lemon (hypothetical-lemon)]

python-etcd3gw bionic

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-etcd3gw - 0.2.5-1ubuntu1

---------------
python-etcd3gw (0.2.5-1ubuntu1) hirsute; urgency=medium

  * d/p/lp1900617-Include-resp.text-as-detail-in-all-etcd-exceptions.patch:
    include resp.text as exception detail (LP: #1900617)

 -- Dan Streetman <email address hidden> Tue, 09 Mar 2021 09:46:53 -0500

[Brian Murray (brian-murray)]

The Groovy Gorilla has reached end of life, so this bug will not be fixed for that release

[Heather Lemon (hypothetical-lemon)]

Sorry, I should have added my notes here sooner.

pull-lp-source python-etcd3gw focal
replaced client.py and base.py
ran `pytest`
--- include screenshot
pulled down source code to run unit tests
the tests include the fix and it passes

correct order for patches
=====
0001_reproducible-build.patch
0001-When-gateway-sends-failure-response-include-text-in-.patch
0001-Include-resp.text-as-detail-in-all-etcd-exceptions.patch
0001-Fix-exception-signature.patch

Testing output method:
TLS params test *This applies to both LPs
----
There are two test workflows to follow.
- testing the patch with self signed certs and etcd server running locally
- running newly created unit tests for TLS params
-----
# Create self signed certs

openssl req -x509 -out localhost.crt -keyout localhost.key -newkey rsa:4096 -nodes -sha256 -out localhost.csr
*make sure the key has an empty password

#download binaries & launch etcd locally with TLS enabled

wget https://github.com/etcd-io/etcd/releases/download/v3.3.13/etcd-v3.3.13-linux-amd64.tar.gz

tar -zxvf etcd-v3.3.14-linux-amd64.tar.gz

cd etcd-v3.3.14-linux-amd64/
sudo cp etcd etcdctl /usr/bin/

# spin up ectd server
etcd --name infra0 --data-dir infra0 --cert-file=localhost.crt --key-file=localhost.key --advertise-client-urls=https://127.0.0.1:2379 --listen-client-urls=https://127.0.0.1:2379
*note I named my directory infra0

#test connection with health endpoint:

curl --cacert localhost.crt --key localhost.key --cert localhost.crt https://127.0.0.1:2379/health

#if successful, the etcd server is configured with https
{"health": "true"}

View test changes inside of ~/python-etcd3gw-0.2.1/etcd3gw/tests/test_client.py

Run the newly added unit test, or run the whole test suite with:
python3 unittest

python3 -m unittest test_client.TestEtcd3Gateway.test_client_tls

We get an error in both the unit test and an error from the etcd server unit test error we are looking for:

OpenSSL.SSL.Error: [('SSL routines', 'tls_process_server_certificate', 'certificate verify failed')] related etcd error: I | embed: rejected connection from "127.0.0.1:44244" (error "remote error: tls: bad certificate", ServerName "")

If you are testing with the added unit test, then make sure there is no etcd server running already.

Unit test console output:

python3 -m unittest test_client.TestEtcd3Gateway.test_client_tls
    def test_client_tls(self):

       client = Etcd3Client(host="localhost", protocol="https", ca_cert="~/localhost.crt",cert_key="~/localhost.key", cert_cert="~/user.crt", timeout=10)
       response = client.get("/health")
       print(response)

/home/.local/lib/python3.8/site-packages/urllib3/connection.py:455: SubjectAltNameWarning: Certificate for 127.0.0.1 has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818. (See https://github.com/urllib3/urllib3/issues/497 for details.)
  warnings.warn(
127.0.0.1 - - [25/Feb/2021 16:43:48] "GET /health HTTP/1.1" 200 -
.
----------------------------------------------------------------------
Ran 1 test in 0.107s

OK

def test_client_tls(self):
            client = Etcd3Client(host="...

[Heather Lemon (hypothetical-lemon)]

pastebin of added code https://pastebin.canonical.com/p/45jWbygmSV/

[Heather Lemon (hypothetical-lemon)]

patch file for 0001-When-gateway-sends-failure-response-include-text-in-.patch

[Heather Lemon (hypothetical-lemon)]

patch file Include-resp.text-as-detail-in-all-etcd-exceptions.patch

[Heather Lemon (hypothetical-lemon)]

patch file Fix-exception-signature.patch

[Eric Desrochers (slashd)]

[sts-sponsors]

* The patch '0002-lp1900617-When-gateway-sends-failure-response-include-text-in-.patch'[0] introduce a 'import mock', but python-mock nor python3-mock is part of the build-Depends: in d/control.

Which make me doubt that this part of the code wasn't probably tested ?
Or is it simply an oversight when finalizing the patch for sponsorship ?

[0] -
0002-lp1900617-When-gateway-sends-failure-response-include-text-in-.patch:43:+import mock
0002-lp1900617-When-gateway-sends-failure-response-include-text-in-.patch:57:+ with mock.patch.object(client, "session") as mock_session:

Ideally, we try to avoid unless very good rational is provided to add 'Build-Depends' for a stable package.

2 Options I can think of, I'll let it to your judgment since you are more aware of this case than I do:

* Remove the test unit
* Add 'python3-mock' as a Build-Depends:'. If you take that path, make sure to have good justification that we can provide to the SRU team.

I'm still reviewing, I'll update the bug, If I find anything else.

- Eric

[Eric Desrochers (slashd)]

After more analysis it should be fine:

     18 Build-Depends-Indep:
     19 python-coverage,
     20 python-futurist,
     21 python-hacking,
     22 python-nose,
     23 python-oslosphinx,
     24 python-oslotest,
     25 python-pytest,
     26 python-requests,
     27 python-six,
     28 python-testscenarios,
     29 python-testtools,
     30 python-urllib3,
     31 python3-futurist,
     32 python3-nose,
=> 33 python3-oslotest,

~# apt-cache depends python3-oslotest
python3-oslotest
  Depends: python3-fixtures
=> Depends: python3-mock

[Eric Desrochers (slashd)]

In theory, python3-mock should be installed by the fact that python3-oslotest as a runtime dependency for it.

[Eric Desrochers (slashd)]

[sts-sponsors]

Uploaded in Focal and Bionic upload queues.

It is now waiting for the SRU verification team to approve the src package to start building and become available in the -proposed pockets for the testing/verification phase.

Thanks for your contribution, Heather.

- Eric & Dariusz

[Brian Murray (brian-murray)]

Hello Dan, or anyone else affected,

Accepted python-etcd3gw into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-etcd3gw/0.2.1-3ubuntu1.20.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Heather Lemon (hypothetical-lemon)]

Hello,

I am currently still working on trying to get a good test around this.

## 1900617 gateway error detail is not passed along in raised exception
# TEST CASE
1. use focal series lxc container
2. Enable proposed repo in /etc/apt/sources.list
3. deb http://archive.ubuntu.com/ubuntu focal-proposed main universe
4. sudo apt-get update
5. apt install etcd-client
6. apt-get install python3-etcd3gw
7. wget https://github.com/etcd-io/etcd/releases/download/v3.3.13/etcd-v3.3.13-linux-amd64.tar.gz
8. tar -xvf etcd-v3.3.13-linux-amd64.tar.gz
9. openssl req -x509 -keyout localhost.key -newkey rsa:4096 -nodes -sha256 -out localhost.crt
10. ./etcd --name infra0 --data-dir infra0 --cert-file=/root/localhost.crt --key-file=/root/localhost.key --advertise-client-urls=https://127.0.0.1:2379 --listen-client-urls=https://127.0.0.1:2379
The first test is from the code with auth enabled, and a response+status code is returned which i believe is the expected result. I am still trying to confirm this by just using the the default etcd server setup locally. The second output is from etcd server without python3-etcd3gw.

root@focal:~# etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended role list
start to sync cluster using endpoints(https://127.0.0.1:2379)
cURL Command: curl -X GET https://127.0.0.1:2379/v2/members
got endpoints(https://127.0.0.1:2379) after sync
Cluster-Endpoints: https://127.0.0.1:2379
Cluster-Endpoints: https://127.0.0.1:2379
cURL Command: curl -X GET https://127.0.0.1:2379/v2/auth/roles
unexpected status code 401

heather@:~$ etcdctl --endpoints http://127.0.0.1:2379 --debug role list
start to sync cluster using endpoints(http://127.0.0.1:2379)
cURL Command: curl -X GET http://127.0.0.1:2379/v2/members
got endpoints(http://localhost:2379) after sync
Cluster-Endpoints: http://localhost:2379
Cluster-Endpoints: http://localhost:2379
cURL Command: curl -X GET http://localhost:2379/v2/auth/roles
root

Thanks,
Heather Lemon

[Eric Desrochers (slashd)]

Can you build a reproducer out of the unit tests for this bug while using the binary package ?

[Heather Lemon (hypothetical-lemon)]

I made a few attempts at running the unit tests from binary but no luck.

[Eric Desrochers (slashd)]

Not having a good test for the binary package would be a blocker here for the release of the package.

[Eric Desrochers (slashd)]

Is there any reproducer that could exercise the code path you have modified ?

[Heather Lemon (hypothetical-lemon)]

https://bugs.launchpad.net/ubuntu/focal/+source/python-etcd3gw/+bug/1900617/comments/17

But I am still trying to confirm if this touched the modified code in the client.py

[Heather Lemon (hypothetical-lemon)]

## 1900617 gateway error detail is not passed along in raised exception
# TEST CASE FOCAL
1. use focal series lxc container
2. Enable proposed repo in /etc/apt/sources.list
3. deb http://archive.ubuntu.com/ubuntu focal-proposed main universe
4. sudo apt-get update
5. apt install etcd-client
6. apt-get install python3-etcd3gw
7. wget https://github.com/etcd-io/etcd/releases/download/v3.3.13/etcd-v3.3.13-linux-amd64.tar.gz
8. tar -xvf etcd-v3.3.13-linux-amd64.tar.gz
9. openssl req -x509 -keyout localhost.key -newkey rsa:4096 -nodes -sha256 -out localhost.crt
10. ./etcd --name infra0 --data-dir infra0 --cert-file=/root/localhost.crt --key-file=/root/localhost.key --advertise-client-urls=https://127.0.0.1:2379 --listen-client-urls=https://127.0.0.1:2379 --debug=true
11. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended user add root
12. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended auth enable
13. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended member list
14. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended member remove 8e9e05c52164694d
start to sync cluster using endpoints(https://127.0.0.1:2379)
cURL Command: curl -X GET https://127.0.0.1:2379/v2/members
got endpoints(https://127.0.0.1:2379) after sync
Cluster-Endpoints: https://127.0.0.1:2379
cURL Command: curl -X GET https://127.0.0.1:2379/v2/members
cURL Command: curl -X DELETE https://127.0.0.1:2379/v2/members/8e9e05c52164694d
Received an error trying to remove member 8e9e05c52164694d: unexpected status code 401

The status code is what is being checked and send back to the console from code client.py
```
if resp.status_code != requests.codes['ok']:
      raise exceptions.Etcd3Exception(resp.reason, resp.text)
```
If we're not a 200 response, then post the Exception to console.

VERIFICATION DONE
Exception response codes are posted to console.

Troubleshooting -
cURL Command: curl -X GET https://127.0.0.1:2379/v2/auth/roles/admin
cURL Command: curl -X PUT https://127.0.0.1:2379/v2/auth/roles/admin -d "{\"role\":\"admin\",\"permissions\":{\"kv\":{\"read\":null,\"write\":null}}}"
client: etcd cluster is unavailable or misconfigured; error #0: x509: cannot validate certificate for 127.0.0.1 because it doesn't contain any IP SANs

### add this to /etc/ssl/openssl.cnf
[ v3_ca ]
subjectAltName=IP:127.0.0.1

[Heather Lemon (hypothetical-lemon)]

## 1900617 gateway error detail is not passed along in raised exception
# TEST CASE BIONIC
Version tested - 0.2.1-1ubuntu0.18.04.1

1. use Bionic series lxc container
2. Enable proposed repo in /etc/apt/sources.list
3. deb http://archive.ubuntu.com/ubuntu focal-proposed main universe
4. sudo apt-get update
5. apt install etcd-client
6. apt-get install python-etcd3gw
7. wget https://github.com/etcd-io/etcd/releases/download/v3.3.13/etcd-v3.3.13-linux-amd64.tar.gz
8. tar -xvf etcd-v3.3.13-linux-amd64.tar.gz
9. openssl req -x509 -keyout localhost.key -newkey rsa:4096 -nodes -sha256 -out localhost.crt
10. ./etcd --name infra0 --data-dir infra0 --cert-file=/root/localhost.crt --key-file=/root/localhost.key --advertise-client-urls=https://127.0.0.1:2379 --listen-client-urls=https://127.0.0.1:2379 --debug=true
11. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended user add root
12. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended auth enable
13. etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended member list
14. root@bionic:~# etcdctl --endpoints https://127.0.0.1:2379 --ca-file=localhost.crt --cert-file=localhost.crt --key-file=localhost.key --debug --output extended member remove 8e9e05c52164694d
start to sync cluster using endpoints(https://127.0.0.1:2379)
cURL Command: curl -X GET https://127.0.0.1:2379/v2/members
got endpoints(https://127.0.0.1:2379) after sync
Cluster-Endpoints: https://127.0.0.1:2379
cURL Command: curl -X GET https://127.0.0.1:2379/v2/members
cURL Command: curl -X DELETE https://127.0.0.1:2379/v2/members/8e9e05c52164694d
Received an error trying to remove member 8e9e05c52164694d: unexpected status code 401

The status code is what is being checked and send back to the console from code client.py
```
if resp.status_code != requests.codes['ok']:
raise exceptions.Etcd3Exception(resp.reason, resp.text)
```
If we're not a 200 response, then post the Exception to console.

VERIFICATION DONE
Exception response codes are posted to console.

Troubleshooting -
cURL Command: curl -X GET https://127.0.0.1:2379/v2/auth/roles/admin
cURL Command: curl -X PUT https://127.0.0.1:2379/v2/auth/roles/admin -d "{\"role\":\"admin\",\"permissions\":{\"kv\":{\"read\":null,\"write\":null}}}"
client: etcd cluster is unavailable or misconfigured; error #0: x509: cannot validate certificate for 127.0.0.1 because it doesn't contain any IP SANs

### add this to /etc/ssl/openssl.cnf
[ v3_ca ]
subjectAltName=IP:127.0.0.1

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-etcd3gw - 0.2.1-3ubuntu1.20.04.1

---------------
python-etcd3gw (0.2.1-3ubuntu1.20.04.1) focal; urgency=medium

  * d/p/lp1820083-Set-transport-options-on-requests-session.patch
    - Sets TLS parameters for session (LP: #1820083)
  * d/p/0001-lp1900617-When-gateway-sends-failure-response-include-text-in-.patch
    - Include response text in raised exception
    d/p/0002-lp1900617-Include-resp.text-as-detail-in-all-etcd-exceptions.patch
    - Add new unit test for return exception
    d/p/0003-lp1900617-Fix-exception-signature.patch
    - Derived exceptions can use arguments again
    (LP: #1900617)

 -- Heather Lemon <email address hidden> Mon, 07 Dec 2020 12:21:25 -0700

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for python-etcd3gw has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-etcd3gw - 0.2.1-1ubuntu0.18.04.1

---------------
python-etcd3gw (0.2.1-1ubuntu0.18.04.1) bionic; urgency=medium

  * d/p/lp1820083-set-transport-options-on-requests-session.patch
    - Sets TLS parameters for session (LP: #1820083)

  * d/p/0001-lp1900617-When-gateway-sends-failure-response-include-text-in.patch
    - Include response text in raised exception
    d/p/0002-lp1900617-Include-resp.text-as-detail-in-all-etcd-exceptions.patch
    - Add new unit test for return exception
    d/p/0003-lp1900617-Fix-exception-signature.patch
    - Derived exceptions can use arguments again
    (LP: #1900617)

 -- Heather Lemon <email address hidden> Wed, 27 Oct 2021 15:59:44 +0000