adding CSP changes to grafana integration#14
adding CSP changes to grafana integration#14dkuldeep22 wants to merge 5 commits intowavefrontHQ:masterfrom
Conversation
|
@dkuldeep22 Please add the Datasource CLI commands for CSP OAuth and API token in the README.md. |
This reverts commit 0f4df13.
Margarita-Staneva
left a comment
Margarita-Staneva
left a comment
There was a problem hiding this comment.
I have one comment. I think CSP API token should be replaced with VMware Cloud services API token.
| <a class="btn btn-secondary gf-form-btn" style="flex-grow:1" type="submit" ng-click="ctrl.resetCspApiToken()" ng-show="ctrl.cspApiTokenExists">Reset</a> | ||
| <input type="text" class="gf-form-input max-width-24" ng-hide="ctrl.cspApiTokenExists" ng-model="ctrl.current.jsonData.cspAPIToken"/> | ||
| <info-popover mode="right-absolute" ng-hide="ctrl.cspApiTokenExists"> | ||
| Paste your CSP API token here. You can find and manage your tokens on your profile page in the Wavefront app |
There was a problem hiding this comment.
Please update this throughout:
Paste your VMware Cloud services API token here. You can find and manage your API tokens on your profile page in the Wavefront app.
|
Please remove console.log statements from all files. These were added for debugging and should not be checked-in |
| const CSP_API_TOKEN_URL = "https://console.cloud.vmware.com/csp/gateway/am/api/auth/api-tokens/authorize"; | ||
| const CSP_OAUTH_TOKEN_URL = "https://console.cloud.vmware.com/csp/gateway/am/api/auth/authorize"; | ||
|
|
||
| const appSecret = "" |
There was a problem hiding this comment.
This is unused variable. Please remove.
| const appId = instanceSettings.jsonData.cspOAuthClientId | ||
| const appSecret = instanceSettings.jsonData.cspOAuthClientSecret | ||
| const credentials = `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`; | ||
|
|
There was a problem hiding this comment.
What are we trying to do here?
Buffer does not exist and credentials is not getting used anywhere.
There was a problem hiding this comment.
Use btoa for base64 encoding.
Try this: const credentials = Basic ${btoa(${appId}:${appSecret})};``
| const startSecs = dateToEpochSeconds(options.range.from); | ||
|
|
||
| const endSecs = dateToEpochSeconds(options.range.to); | ||
| console.log("********endSecs", endSecs); |
There was a problem hiding this comment.
Let's remove all console.log statements here.
| grant_type: "client_credentials", | ||
| }), | ||
| headers: { | ||
| "Content-type": "application/x-www-form-urlencoded; charset=UTF-8" |
There was a problem hiding this comment.
We should add "Authorization" in headers.
| const appId = instanceSettings.jsonData.cspOAuthClientId | ||
| const appSecret = instanceSettings.jsonData.cspOAuthClientSecret | ||
| const credentials = `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`; | ||
|
|
There was a problem hiding this comment.
Use btoa for base64 encoding.
Try this: const credentials = Basic ${btoa(${appId}:${appSecret})};``
| } catch(e) { | ||
| console.error(e); | ||
| } | ||
| this.requestConfigProto.headers["Authorization"] = "Bearer " + instanceSettings.jsonData.cspAPIToken; |
There was a problem hiding this comment.
I think we should set "Authorization" header to access token which we get after making call to CSP_OAUTH_TOKEN_URL. Not directly to csp api token. Please confirm this point from Gangadhar or Sadanand.
If they confirm, then change line#45 from :
.then((json) => console.log(json));
to:
then((accessToken) => this.requestConfigProto.headers["Authorization"] = accessToken)
|
|
||
|
|
||
| function refreshToken() { | ||
| if (instanceSettings.jsonData.cspAPIToken) { |
There was a problem hiding this comment.
This code is same as what we are doing at line#35 and 51.
Can we create two functions like: fetchAccessTokenUsingCSPAPIToken and fetchAccessTokenUsingCSPOAuth and use these functions during initialisation and interval update
| } | ||
|
|
||
| // Execute the function initially | ||
| refreshToken(); |
There was a problem hiding this comment.
Since, we are already calling it during initialisation at line#33, we can skip this initial call of refreshToken
| refreshToken(); | ||
|
|
||
| // Execute the function every 25 minutes (25 * 60 * 1000 milliseconds) | ||
| const interval = setInterval(refreshToken, 25 * 60 * 1000); |
There was a problem hiding this comment.
We are not stopping the interval at any time. No need to store the result in interval const. Calling the function should be sufficient.
setInterval(refreshToken, 25 * 60 * 1000);
4 participants
No description provided.