Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION #868

Merged
merged 2 commits into from
Jan 23, 2021

Conversation

themarcelor
Copy link
Contributor

@themarcelor themarcelor commented Jan 22, 2021

Our fence-create google-manage-keys CLI command is returning 400 FAILED_PRECONDITION while attempting to delete a key that no longer exists in GCP (only exists in fence's database).

The description from the official documentation is quite misleading... 馃
https://cloud.google.com/apis/design/errors

400 FAILED_PRECONDITION Resource xxx is a non-empty directory, so it cannot be deleted.

logs:

{
  "error": {
    "code": 400,
    "message": "Precondition check failed.",
    "status": "FAILED_PRECONDITION"
  }
}
>
ERROR: (gcloud.iam.service-accounts.keys.delete) FAILED_PRECONDITION: Precondition check failed.

Because it is trying to delete a key that no longer existing in Google Cloud. Hence we should just allow the CLI operation to delete the orphan key in fence's db and unblock the Continuous Integration pipeline.

@github-actions
Copy link

The style in this PR agrees with black. 鉁旓笍

This formatting comment was generated automatically by a script in uc-cdis/wool.

@themarcelor themarcelor changed the title chore(google-manage-keys): Handle key deletion with FAILED_PRECONDITION chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION Jan 22, 2021
@coveralls
Copy link

coveralls commented Jan 22, 2021

Pull Request Test Coverage Report for Build 10251

  • 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 68.717%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fence/scripting/fence_create.py 4 5 80.0%
Files with Coverage Reduction New Missed Lines %
fence/scripting/fence_create.py 1 53.68%
Totals Coverage Status
Change from base Build 10212: 0.02%
Covered Lines: 5430
Relevant Lines: 7902

馃挍 - Coveralls

@themarcelor themarcelor merged commit 1bcbc45 into master Jan 23, 2021
@themarcelor themarcelor deleted the chore/tweak_fence_create_manage_google_keys branch January 23, 2021 07:03
rolinge added a commit to rolinge/fence that referenced this pull request Mar 10, 2021
* annotate spot for fix

* try overwrite roles, if fail, then create

* test exception handling

* catch arborist error

* log info failed put role

* test gen3authz patch

* fix git requirement line

* debug requirements load from git

* specify test gen3authz by commit

* test no git

* debug gen3authz tag

* clear cache in quay

* debug import test gen3authz lib

* test

* test

* revert dockerfile and requirements to master

* hack test poetry install gen3authz

* debug poetry install

* debug test dockerfile

* clear quay cache

* comment out gen3authz from setup.py

* tell poetry we don't need a virtual env thanks

* add gen3authz name to setup.py package list

* bump cdiserrors version

* add cdiserrors to setup.py

* bump gen3config

* fix ArboristError import path

* await get users from arborist

* don't await; catch attribute error

* h4ck dockerfile

* revert dockerfile

* use new release of gen3authz lib

* update httplib2

* remove comment

* fix conftest mock arborist patch method

* fix conftest request import path

* async magic mock arborist client

* add asynctest to dev-requirements

* revert testing stuff..

* fix tests - replace gen3authz requests.request with httpx.request

* revert

* fix requests import path in tests

* revert

* update dep versions

* bump authutils..

* revert deps..

* update dep versions

* bump authutils

* delete old poetry lock file

* test import cdispyutils branch

* add test lock file

* update tests

* update tests

* update tests

* debug tests

* revert

* reset

* Parse RAS visas

* chore(fence): New parent image pybase3-1.4.2 (uc-cdis#853)

* parse all users

* feat(userinfo): include identity provider name in userinfo response for clients

* feat(userinfo): safely graph idp name even if relationship doesn't exist

* docs(openapi): add idp to userinfo response docs

* visa update cron job

* initialize

* notes

* fix(dependencies): Fix poetry lock command (uc-cdis#858)

* fix(dependencies): Fix poetry lock

* new lock file

* Update pyproject.toml

Cool

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* draft flow

* Dependency Fix New

* chore(builds): Speed up poetry commands by setting proper botocore version (uc-cdis#859)

* fix(dependencies): Fix poetry lock

* new lock file

* chore(build): Speed up docker build nby setting proper botocore version

* add new lock file

* removing line from the other PR

* more notes

* gen3authz use branch

* pkg

* async

* new fix

* new authz

* print

* stuff

* print

* new endpoint

* async

* prints

* updated

* remove prints

* update

* authutils

* testing

* testing

* update dependencies

* poetry update

* account for empty url list

* add window method

* updated as designed

* chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION (uc-cdis#868)

* chore(google-manage-keys): Handle key deletion with FAILED_PRECONDITION

* somem logging

* fence-create+logging

* fix asyncio

* fix asyncio for python<3.6

* db session fixes

* small fixes

* fixes

* try

* remove grant type

* remove try

* add not

* return for when no urls are present

* add variables

* fix

* check update

* int

* fix

* change small things

* add single visa processing

* loop through all urls

* rerun

* lock update

* add unit test

* fix parse_consent_code

* remove test

* finishing up + fence create update

* fix user yaml

* fix get

* add test cases

* comments

* feat(cookies): always set httponly flag on cookies (also update deps)

* add test case reformat

* remove redundent code

* adding fallback logic

* fix things

* add help

* feat(cookies): add httponly flag to missing spots

* test and such

* lock update

* fix things

* cahnge config

* lgos + black

* remove invalid visas

* db_session

* smoll fix

* add logs

* add logs

* #):
fix(no_force_sign): add regression test for urls with no_force_sign=False

* fix(no_force_sign): parse truthiness from no_force_sign param, rather than check if exists

* ras login

* fixes

* small fixes

* stop revoke

* revoke

* black

* small change

* sess

* init sync

* blargh

* add session

* revert

* add to config

* black + spaces

* fix tests

* add to config

* Update bin/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/scripting/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/job/visa_update_cronjob.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* review changes

* black

* resolve again

* remove comments

* black

* small fix

* login

* revoke control

* close session

* refactor

* PXP-7696 Fix/renew access token (uc-cdis#874)

* ix: always mint new token

* fix: wrong logic

* fix: add config

* run the hook

* set default to false

* resolve comments

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* fix(empty idp): always commit user/idp to db

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* update black

* fix typo

* Feat/refresh expires in (uc-cdis#848)

* store expires_in param from authz request in flask session

* take expires_in value from session if present for generating refresh tokens

* fix key check

* debug

* revert

* add exp col to auth code db table model

* extract expires_in param from authorize request and put in db record

* add refresh_token_expires_in logic to token response

* add code for db migration authorization_code table add col

* remove comment

* reformat

* handle pre-db migration case

* undo last change

* handle pre-db-migration case

* fix init auth code record

* undo last change

* revert

* try reflecting db to enable backwards compatibility with previous db schema

* add ref to relevant doc

* fix db reflection

* add scope handling

* reflect auth code table at token endpoint

* revert

* Accommodate variously named query params for expiration times

* expiry does not always refer to access token

* Refactor get-and-validate-requested-expiration pattern into util fn

* Clearer intentions, better names, less repetition, less cognitive load, etc

Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>

* test(login_user): ensure logged in users have idp

* docs(login_user): add a docstring

* format black

* fix(login_user): dont log in if already configured

* chore(login_user): remove unused flask.request arg

* chore(login_user): trigger build

* fix(oauth): allow fence oauth w/o LOGIN_OPTIONS

* docs(Accessing Data): Fixed broken link to #create-user-access-file (uc-cdis#881)

Co-authored-by: mattgarvin1 <mattgarvin1@gmail.com>
Co-authored-by: Alexander VanTol <Avantol13@users.noreply.github.com>
Co-authored-by: BinamB <numb.baj@gmail.com>
Co-authored-by: Marcelo R Costa <marceloc@uchicago.edu>
Co-authored-by: Alexander VT <alexander.m.vantol@gmail.com>
Co-authored-by: Binam Bajracharya <44302895+BinamB@users.noreply.github.com>
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
Co-authored-by: mattreex <mattreex.9@gail.com>
Co-authored-by: Mattreex <45504048+mattreex@users.noreply.github.com>
Co-authored-by: Matthew Cannalte <mcannalte@uchicago.edu>
Co-authored-by: Mingfei Shao <2475897+mfshao@users.noreply.github.com>
Co-authored-by: John McCann <johnmccann@uchicago.edu>
Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>
rolinge added a commit to rolinge/fence that referenced this pull request Mar 10, 2021
* annotate spot for fix

* try overwrite roles, if fail, then create

* test exception handling

* catch arborist error

* log info failed put role

* test gen3authz patch

* fix git requirement line

* debug requirements load from git

* specify test gen3authz by commit

* test no git

* debug gen3authz tag

* clear cache in quay

* debug import test gen3authz lib

* test

* test

* revert dockerfile and requirements to master

* hack test poetry install gen3authz

* debug poetry install

* debug test dockerfile

* clear quay cache

* comment out gen3authz from setup.py

* tell poetry we don't need a virtual env thanks

* add gen3authz name to setup.py package list

* bump cdiserrors version

* add cdiserrors to setup.py

* bump gen3config

* fix ArboristError import path

* await get users from arborist

* don't await; catch attribute error

* h4ck dockerfile

* revert dockerfile

* use new release of gen3authz lib

* update httplib2

* remove comment

* fix conftest mock arborist patch method

* fix conftest request import path

* async magic mock arborist client

* add asynctest to dev-requirements

* revert testing stuff..

* fix tests - replace gen3authz requests.request with httpx.request

* revert

* fix requests import path in tests

* revert

* update dep versions

* bump authutils..

* revert deps..

* update dep versions

* bump authutils

* delete old poetry lock file

* test import cdispyutils branch

* add test lock file

* update tests

* update tests

* update tests

* debug tests

* revert

* reset

* Parse RAS visas

* chore(fence): New parent image pybase3-1.4.2 (uc-cdis#853)

* parse all users

* feat(userinfo): include identity provider name in userinfo response for clients

* feat(userinfo): safely graph idp name even if relationship doesn't exist

* docs(openapi): add idp to userinfo response docs

* visa update cron job

* initialize

* notes

* fix(dependencies): Fix poetry lock command (uc-cdis#858)

* fix(dependencies): Fix poetry lock

* new lock file

* Update pyproject.toml

Cool

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* draft flow

* Dependency Fix New

* chore(builds): Speed up poetry commands by setting proper botocore version (uc-cdis#859)

* fix(dependencies): Fix poetry lock

* new lock file

* chore(build): Speed up docker build nby setting proper botocore version

* add new lock file

* removing line from the other PR

* more notes

* gen3authz use branch

* pkg

* async

* new fix

* new authz

* print

* stuff

* print

* new endpoint

* async

* prints

* updated

* remove prints

* update

* authutils

* testing

* testing

* update dependencies

* poetry update

* account for empty url list

* add window method

* updated as designed

* chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION (uc-cdis#868)

* chore(google-manage-keys): Handle key deletion with FAILED_PRECONDITION

* somem logging

* fence-create+logging

* fix asyncio

* fix asyncio for python<3.6

* db session fixes

* small fixes

* fixes

* try

* remove grant type

* remove try

* add not

* return for when no urls are present

* add variables

* fix

* check update

* int

* fix

* change small things

* add single visa processing

* loop through all urls

* rerun

* lock update

* add unit test

* fix parse_consent_code

* remove test

* finishing up + fence create update

* fix user yaml

* fix get

* add test cases

* comments

* feat(cookies): always set httponly flag on cookies (also update deps)

* add test case reformat

* remove redundent code

* adding fallback logic

* fix things

* add help

* feat(cookies): add httponly flag to missing spots

* test and such

* lock update

* fix things

* cahnge config

* lgos + black

* remove invalid visas

* db_session

* smoll fix

* add logs

* add logs

* #):
fix(no_force_sign): add regression test for urls with no_force_sign=False

* fix(no_force_sign): parse truthiness from no_force_sign param, rather than check if exists

* ras login

* fixes

* small fixes

* stop revoke

* revoke

* black

* small change

* sess

* init sync

* blargh

* add session

* revert

* add to config

* black + spaces

* fix tests

* add to config

* Update bin/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/scripting/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/job/visa_update_cronjob.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* review changes

* black

* resolve again

* remove comments

* black

* small fix

* login

* revoke control

* close session

* refactor

* PXP-7696 Fix/renew access token (uc-cdis#874)

* ix: always mint new token

* fix: wrong logic

* fix: add config

* run the hook

* set default to false

* resolve comments

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* fix(empty idp): always commit user/idp to db

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* update black

* fix typo

* Feat/refresh expires in (uc-cdis#848)

* store expires_in param from authz request in flask session

* take expires_in value from session if present for generating refresh tokens

* fix key check

* debug

* revert

* add exp col to auth code db table model

* extract expires_in param from authorize request and put in db record

* add refresh_token_expires_in logic to token response

* add code for db migration authorization_code table add col

* remove comment

* reformat

* handle pre-db migration case

* undo last change

* handle pre-db-migration case

* fix init auth code record

* undo last change

* revert

* try reflecting db to enable backwards compatibility with previous db schema

* add ref to relevant doc

* fix db reflection

* add scope handling

* reflect auth code table at token endpoint

* revert

* Accommodate variously named query params for expiration times

* expiry does not always refer to access token

* Refactor get-and-validate-requested-expiration pattern into util fn

* Clearer intentions, better names, less repetition, less cognitive load, etc

Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>

* test(login_user): ensure logged in users have idp

* docs(login_user): add a docstring

* format black

* fix(login_user): dont log in if already configured

* chore(login_user): remove unused flask.request arg

* chore(login_user): trigger build

* fix(oauth): allow fence oauth w/o LOGIN_OPTIONS

* docs(Accessing Data): Fixed broken link to #create-user-access-file (uc-cdis#881)

Co-authored-by: mattgarvin1 <mattgarvin1@gmail.com>
Co-authored-by: Alexander VanTol <Avantol13@users.noreply.github.com>
Co-authored-by: BinamB <numb.baj@gmail.com>
Co-authored-by: Marcelo R Costa <marceloc@uchicago.edu>
Co-authored-by: Alexander VT <alexander.m.vantol@gmail.com>
Co-authored-by: Binam Bajracharya <44302895+BinamB@users.noreply.github.com>
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
Co-authored-by: mattreex <mattreex.9@gail.com>
Co-authored-by: Mattreex <45504048+mattreex@users.noreply.github.com>
Co-authored-by: Matthew Cannalte <mcannalte@uchicago.edu>
Co-authored-by: Mingfei Shao <2475897+mfshao@users.noreply.github.com>
Co-authored-by: John McCann <johnmccann@uchicago.edu>
Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants