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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAM Support: phase II: token management #156

Merged
merged 1 commit into from
Apr 22, 2019
Merged

IAM Support: phase II: token management #156

merged 1 commit into from
Apr 22, 2019

Conversation

srl295
Copy link
Contributor

@srl295 srl295 commented Apr 20, 2019

followon to #151

  • make internal client.swaggerClient a getter instead of a property.
    (original property at _swaggerClient).
    All internal clients must go through this getter.

  • support IAM token. No API change.

  • Internal env vars:

    • GP_USE_APIKEY=true to not use tokens,
    • IAM_TOKEN_EXPIRY_THRESHOLD_PROP_KEY to change the expiry derating from
      0.85 to something else.

Fixes: #150

@srl295 srl295 requested review from pgaglani1 and yumaoka April 20, 2019 01:28
@srl295 srl295 self-assigned this Apr 20, 2019
- make internal client.swaggerClient a getter instead of a property.
(original property at _swaggerClient).
All internal clients must go through this getter.

- support IAM token. No API change.

- Internal env vars:
    - GP_USE_APIKEY=true to not use tokens,
    - IAM_TOKEN_EXPIRY_THRESHOLD_PROP_KEY to change the expiry derating from
0.85 to something else.

Fixes: #150
@coveralls
Copy link

coveralls commented Apr 20, 2019

Coverage Status

Coverage decreased (-2.8%) to 84.605% when pulling c0f6940 on iam-token into 9e25239 on master.

@sid41
Copy link

sid41 commented Apr 22, 2019

LGTM.
(The continuous-integration/travis-ci/push build is failing)

@@ -29,12 +42,59 @@ const GpIAM = function GpIAM(credentials) {
if(!this.credentials || !this.credentials.apikey || !this.credentials.iam_endpoint) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are allowing both api key and token for iam auth, you might want to keep check for iam endpoint conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sid41 the API states that the iam_endpoint is required, I wanted to keep the credential requirement consistent. Using the API-KEY header is not intended to be a high runner use case.

@srl295
Copy link
Contributor Author

srl295 commented Apr 22, 2019

@sid41 may be an intermittent issue with the test run not related to this issue

@srl295 srl295 merged commit 0e649da into master Apr 22, 2019
@srl295 srl295 deleted the iam-token branch April 22, 2019 18:07
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.

js: IAM support phase II (token management)
3 participants