From 87097cca0085e57c373ccc8e829b46932c654368 Mon Sep 17 00:00:00 2001 From: armab Date: Fri, 8 Feb 2019 20:38:04 +0100 Subject: [PATCH 1/5] Fix API 'POST /api/v1/apikeys' wasn't creating new record with the provided ID At a high level fixes `st2api key load` that wasn't idempotent and failing to import same file twice with: ``` HTTPError: 409 Client Error: Conflict MESSAGE: Tried to save duplicate unique keys (E11000 duplicate key error collection: st2.api_key_d_b index: key_hash_1 dup key: { : "903b7f0761094265969761443f4f25fc87c780b6248c38ce8dbaadfa52a2b53fd72381ac141072cb53ae78ff02e16fdfc0e40001472b9f25585a7b2864c501db" }) for url: http://127.0.0.1:9101/v1/apikeys ``` because it couldn't import records with the requested ID. --- st2common/st2common/models/api/auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/models/api/auth.py b/st2common/st2common/models/api/auth.py index e1f9e39106..f51841e63d 100644 --- a/st2common/st2common/models/api/auth.py +++ b/st2common/st2common/models/api/auth.py @@ -143,11 +143,13 @@ def from_model(cls, model, mask_secrets=False): @classmethod def to_model(cls, instance): + # If PrimaryKey ID is provided, - we want to work with existing ST2 API key + id = getattr(instance, 'id', None) user = str(instance.user) if instance.user else None key_hash = getattr(instance, 'key_hash', None) metadata = getattr(instance, 'metadata', {}) enabled = bool(getattr(instance, 'enabled', True)) - model = cls.model(user=user, key_hash=key_hash, metadata=metadata, enabled=enabled) + model = cls.model(id=id, user=user, key_hash=key_hash, metadata=metadata, enabled=enabled) return model From 50489cdaf3cbd6265b75f819c5ae5238d4cac269 Mon Sep 17 00:00:00 2001 From: armab Date: Fri, 8 Feb 2019 21:14:11 +0100 Subject: [PATCH 2/5] Add Changelog record for #4542 --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c75117f82e..a866ee9531 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -48,6 +48,9 @@ Fixed Reported by @dswebbthg, @nickbaum. (bug fix) #4513 #4527 #4528 +* Fix CLI ``st2 apikey load`` not being idempotent and API endpoint ``/api/v1/apikeys`` not + honoring desired ``ID`` for the new record creation. #4542 + 2.10.0 - December 13, 2018 -------------------------- From 4995b8dfe5f973947ee3a4f7ab2421fad0225342 Mon Sep 17 00:00:00 2001 From: armab Date: Tue, 12 Feb 2019 13:29:21 +0100 Subject: [PATCH 3/5] Add ID's to apikey fixtures --- st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml | 1 + st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml b/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml index 49cc94e8a4..ffdaccad18 100644 --- a/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml +++ b/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml @@ -1,4 +1,5 @@ --- +id: 58e3f3330c0517062a3fda43 user: bill key_hash: "ec81d4a56f5987b0ae1cff6e152459986e873d6604637fc70d85c0a0daf131b0a830ccd5b6454cc0c95c0ba6e6655933c993325eb3a28bc43af6c1d801a7c1e8" # 1234 metadata: diff --git a/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml b/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml index fa2755052a..1fed0f928c 100644 --- a/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml +++ b/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml @@ -1,4 +1,5 @@ --- +id: 5c5ddd776cb8de530e0a1391 user: dilbert key_hash: "17f858ea0bb108feaa91b8eee524c7382e0218ff541783d45996a1149d50dfde4bc19f2e6a591028a2ea08de4211893b246d4eda61dd3c9cf294a2405184ac4b" # 5678 metadata: From 15d8c73e9e1041fc98468851adc2d96bcf448084 Mon Sep 17 00:00:00 2001 From: armab Date: Tue, 12 Feb 2019 13:37:18 +0100 Subject: [PATCH 4/5] Add 'ID' for POST API creation test case --- st2api/tests/unit/controllers/v1/test_auth_api_keys.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_auth_api_keys.py b/st2api/tests/unit/controllers/v1/test_auth_api_keys.py index 3a32dc03c1..069051e4cd 100644 --- a/st2api/tests/unit/controllers/v1/test_auth_api_keys.py +++ b/st2api/tests/unit/controllers/v1/test_auth_api_keys.py @@ -197,6 +197,7 @@ def test_post_delete_key(self): def test_post_delete_same_key_hash(self): api_key = { + 'id': '5c5dbb576cb8de06a2d79a4d', 'user': 'herge', 'key_hash': 'ABCDE' } @@ -207,6 +208,7 @@ def test_post_delete_same_key_hash(self): # drop into the DB since API will be masking this value. api_key_db = ApiKey.get_by_id(resp1.json['id']) + self.assertEqual(resp1.json['id'], api_key['id'], 'PK ID of created API should match.') self.assertEqual(api_key_db.key_hash, api_key['key_hash'], 'Key_hash should match.') self.assertEqual(api_key_db.user, api_key['user'], 'Key_hash should match.') From 1d012afe52e496cfd989e26a3960992fe1769bc1 Mon Sep 17 00:00:00 2001 From: armab Date: Tue, 12 Feb 2019 13:38:15 +0100 Subject: [PATCH 5/5] Fix a typo in older tests --- st2api/tests/unit/controllers/v1/test_auth_api_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2api/tests/unit/controllers/v1/test_auth_api_keys.py b/st2api/tests/unit/controllers/v1/test_auth_api_keys.py index 069051e4cd..3a6f8f6e02 100644 --- a/st2api/tests/unit/controllers/v1/test_auth_api_keys.py +++ b/st2api/tests/unit/controllers/v1/test_auth_api_keys.py @@ -210,7 +210,7 @@ def test_post_delete_same_key_hash(self): self.assertEqual(resp1.json['id'], api_key['id'], 'PK ID of created API should match.') self.assertEqual(api_key_db.key_hash, api_key['key_hash'], 'Key_hash should match.') - self.assertEqual(api_key_db.user, api_key['user'], 'Key_hash should match.') + self.assertEqual(api_key_db.user, api_key['user'], 'User should match.') resp = self.app.delete('/v1/apikeys/%s' % resp1.json['id']) self.assertEqual(resp.status_int, 204)