From 347d51fde946f30552d50a58d42d22d8ab69bb3e Mon Sep 17 00:00:00 2001 From: J Date: Fri, 26 Feb 2021 22:10:15 +0000 Subject: [PATCH] interface: cleaner API, robust GcpManager retry GcpApi now acts like other APIs. Since GcpManager can no longer get at the token exipry by inspecting the raw update, it must depend on the global store instead. This also means it can check whether the user has configured S3, and not try to refresh the token in that case. In the case where no storage is configured, this will spam the console with request failures since the thread returns 500 if there is no token. Perhaps this is a good argument for making the thread return a unit. --- pkg/interface/src/logic/api/gcp.ts | 8 --- pkg/interface/src/logic/lib/gcpManager.ts | 72 ++++++++++++++----- pkg/interface/src/logic/reducers/gcp-token.ts | 14 ++-- pkg/interface/src/types/gcp-state.ts | 2 +- pkg/interface/src/views/App.js | 2 +- 5 files changed, 65 insertions(+), 33 deletions(-) diff --git a/pkg/interface/src/logic/api/gcp.ts b/pkg/interface/src/logic/api/gcp.ts index 728b711782..b86cbd8788 100644 --- a/pkg/interface/src/logic/api/gcp.ts +++ b/pkg/interface/src/logic/api/gcp.ts @@ -4,20 +4,12 @@ import {GcpToken} from '../types/gcp-state'; export default class GcpApi extends BaseApi { - // Return value resolves to the token's expiry time if successful. refreshToken() { return this.spider('noun', 'gcp-token', 'get-gcp-token', {}) .then((token) => { this.store.handleEvent({ data: token }); - - if (typeof(token) === 'object' && - typeof(token['gcp-token']) === 'object' && - typeof(token['gcp-token']['expiresIn']) === 'number') { - return Promise.resolve(token['gcp-token']['expiresIn']); - } - return Promise.reject(new Error("invalid token")); }); } }; diff --git a/pkg/interface/src/logic/lib/gcpManager.ts b/pkg/interface/src/logic/lib/gcpManager.ts index 2ed03079ac..813cf24c8b 100644 --- a/pkg/interface/src/logic/lib/gcpManager.ts +++ b/pkg/interface/src/logic/lib/gcpManager.ts @@ -2,18 +2,27 @@ // // To use: // -// 1. call setApi with a GlobalApi. +// 1. call configure with a GlobalApi and GlobalStore. // 2. call start() to start the token refresh loop. // +// If the ship has S3 credentials set, we don't try to get a token, but we keep +// checking at regular intervals to see if they get unset. Otherwise, we try to +// invoke the GCP token thread on the ship until it gives us an access token. +// Once we have a token, we refresh it every hour or so, since it has an +// intrinsic expiry. +// // import GlobalApi from '../api/global'; +import GlobalStore from '../store/store'; class GcpManager { #api: GlobalApi | null = null; + #store: GlobalStore | null = null; - setApi(api: GlobalApi) { + configure(api: GlobalApi, store: GlobalStore) { this.#api = api; + this.#store = store; } #running = false; @@ -24,8 +33,8 @@ class GcpManager { console.warn('GcpManager already running'); return; } - if (!this.#api) { - console.error('GcpManager must have api set'); + if (!this.#api || !this.#store) { + console.error('GcpManager must have api and store set'); return; } this.#running = true; @@ -51,17 +60,37 @@ class GcpManager { this.start(); } + #consecutiveFailures: number = 0; + private refreshLoop() { + const s3 = this.#store.state.s3; + // XX ships currently always have S3 credentials, but the fields are all + // set to '' if they are not configured. + if (s3 && + s3.credentials && + s3.credentials.accessKeyId && + s3.credentials.secretAccessKey) { + // do nothing, and check again in 5s. + this.refreshAfter(5_000); + return; + } this.#api.gcp.refreshToken() - .then( - (expiresIn: number) => { - this.refreshAfter(this.refreshInterval(expiresIn)); - }) - .catch( - (reason) => { - console.error('GcpManager token refresh failed', reason); - this.refreshAfter(30_000); // XX backoff? - }); + .then(() => { + const token = this.#store.state.gcp?.token; + if (token) { + this.#consecutiveFailures = 0; + const interval = this.refreshInterval(token.expiresIn); + console.log('GcpManager got token; refreshing after', interval); + this.refreshAfter(interval); + } else { + throw new Error('thread succeeded, but returned no token?'); + } + }) + .catch((reason) => { + this.#consecutiveFailures++; + console.warn('GcpManager refresh failed; retrying with backoff'); + this.refreshAfter(this.backoffInterval()); + }); } private refreshAfter(durationMs) { @@ -71,7 +100,6 @@ class GcpManager { console.warn('GcpManager already has a timeout set'); return; } - console.log('GcpManager refreshing after', durationMs, 'ms'); this.#timeoutId = setTimeout(() => { this.#timeoutId = null; this.refreshLoop(); @@ -79,10 +107,18 @@ class GcpManager { } private refreshInterval(expiresIn: number) { - // Give ourselves 30 seconds for processing delays, but never refresh - // sooner than 30 minutes from now. (The expiry window should be about an - // hour.) - return Math.max(30 * 60_000, expiresIn - 30_000); + // Give ourselves a minute for processing delays, but never refresh sooner + // than 30 minutes from now. (The expiry window should be about an hour.) + return Math.max(30 * 60_000, expiresIn - 60_000); + } + + private backoffInterval() { + // exponential backoff. + const slotMs = 5_000; + const maxSlot = 60; // 5 minutes + const backoffSlots = + Math.floor(Math.random() * Math.min(maxSlot, this.#consecutiveFailures)); + return slotMs * backoffSlots; } } diff --git a/pkg/interface/src/logic/reducers/gcp-token.ts b/pkg/interface/src/logic/reducers/gcp-token.ts index 98ef0b98fa..d9ecc3969c 100644 --- a/pkg/interface/src/logic/reducers/gcp-token.ts +++ b/pkg/interface/src/logic/reducers/gcp-token.ts @@ -8,14 +8,18 @@ export default class GcpReducer{ reduce(json: Cage, state: S) { let data = json['gcp-token']; if (data) { - this.setAccessKey(data, state); + this.setToken(data, state); } } - setAccessKey(json: GcpToken, state: S) { - const data = _.get(json, 'accessKey'); - if (data) { - state.gcp.accessKey = data; + setToken(data: any, state: S) { + if (this.isToken(data)) { + state.gcp.token = data; } } + + isToken(token: any): token is GcpToken { + return (typeof(token.accessKey) === 'string' && + typeof(token.expiresIn) === 'number'); + } } diff --git a/pkg/interface/src/types/gcp-state.ts b/pkg/interface/src/types/gcp-state.ts index 976214af42..e45a37011a 100644 --- a/pkg/interface/src/types/gcp-state.ts +++ b/pkg/interface/src/types/gcp-state.ts @@ -4,5 +4,5 @@ export interface GcpToken { }; export interface GcpState { - accessKey?: string; + token?: GcpToken }; diff --git a/pkg/interface/src/views/App.js b/pkg/interface/src/views/App.js index 78a7cbea9f..4089ec1eae 100644 --- a/pkg/interface/src/views/App.js +++ b/pkg/interface/src/views/App.js @@ -79,7 +79,7 @@ class App extends React.Component { this.appChannel = new window.channel(); this.api = new GlobalApi(this.ship, this.appChannel, this.store); - gcpManager.setApi(this.api); + gcpManager.configure(this.api, this.store); this.subscription = new GlobalSubscription(this.store, this.api, this.appChannel);