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.

Sign up for GitHub

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

OnCall plugin use service accounts instead of api keys #2385

Merged
merged 3 commits into from
Jun 29, 2023
Merged

OnCall plugin use service accounts instead of api keys #2385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ba8dc0
Use api tokens belonging to service accounts instead of api keys from…
mderynck Jun 28, 2023
f6d4134
Reorder, fix/expand createGrafanaToken test, CHANGELOG
mderynck Jun 28, 2023
21dfc7d
Merge branch 'dev' into mderynck/oncall-plugin-use-service-accounts
joeyorlando Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## v1.3.4 (2023-06-29)

This version contains just some small cleanup and CI changes 🙂
### Changed

- Change OnCall plugin to use service accounts and api tokens for communicating with backend, by @mderynck ([#2385](https://github.com/grafana/oncall/pull/2385))

## v1.3.3 (2023-06-28)

Expand Down
61 changes: 57 additions & 4 deletions grafana-plugin/src/state/plugin/index.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

nit PluginState.getGrafanaToken is called in the PluginState.syncDataWithOnCall method, there's a small comment there mentioning "API keys", we might to remove/update that comment

Original file line number Diff line number Diff line change
Expand Up @@ -136,22 +136,75 @@ class PluginState {
this.grafanaBackend.post(this.GRAFANA_PLUGIN_SETTINGS_URL, { ...data, enabled, pinned: true });

static readonly KEYS_BASE_URL = '/api/auth/keys';
static readonly ONCALL_KEY_NAME = 'OnCall';
static readonly SERVICE_ACCOUNTS_BASE_URL = '/api/serviceaccounts';
static readonly ONCALL_SERVICE_ACCOUNT_NAME = 'sa-autogen-OnCall';
static readonly SERVICE_ACCOUNTS_SEARCH_URL = `${PluginState.SERVICE_ACCOUNTS_BASE_URL}/search?query=${PluginState.ONCALL_SERVICE_ACCOUNT_NAME}`;

static getServiceAccount = async () => {
const serviceAccounts = await this.grafanaBackend.get(this.SERVICE_ACCOUNTS_SEARCH_URL);
return serviceAccounts.serviceAccounts.length > 0 ? serviceAccounts.serviceAccounts[0] : null;
};

static getOrCreateServiceAccount = async () => {
const serviceAccount = await this.getServiceAccount();
if (serviceAccount) {
return serviceAccount;
}

return await this.grafanaBackend.post(this.SERVICE_ACCOUNTS_BASE_URL, {
name: this.ONCALL_SERVICE_ACCOUNT_NAME,
role: 'Admin',
isDisabled: false,
});
};

static getTokenFromServiceAccount = async (serviceAccount) => {
const tokens = await this.grafanaBackend.get(`${this.SERVICE_ACCOUNTS_BASE_URL}/${serviceAccount.id}/tokens`);
return tokens.find((key: { id: number; name: string; role: string }) => key.name === PluginState.ONCALL_KEY_NAME);
};

/**
* This will satisfy a check for an existing key regardless of if the key is an older api key or under a
* service account.
*/
static getGrafanaToken = async () => {
const serviceAccount = await this.getServiceAccount();
if (serviceAccount) {
return await this.getTokenFromServiceAccount(serviceAccount);
}

const keys = await this.grafanaBackend.get(this.KEYS_BASE_URL);
return keys.find((key: { id: number; name: string; role: string }) => key.name === 'OnCall');
const oncallApiKeys = keys.find(
(key: { id: number; name: string; role: string }) => key.name === PluginState.ONCALL_KEY_NAME
);
if (oncallApiKeys) {
return oncallApiKeys;
}

return null;
};

/**
* Create service account and api token belonging to it instead of using api keys
*/
static createGrafanaToken = async () => {
const serviceAccount = await this.getOrCreateServiceAccount();
const existingToken = await this.getTokenFromServiceAccount(serviceAccount);
if (existingToken) {
await this.grafanaBackend.delete(
`${this.SERVICE_ACCOUNTS_BASE_URL}/${serviceAccount.id}/tokens/${existingToken.id}`
);
}

const existingKey = await this.getGrafanaToken();
if (existingKey) {
await this.grafanaBackend.delete(`${this.KEYS_BASE_URL}/${existingKey.id}`);
}

return await this.grafanaBackend.post(this.KEYS_BASE_URL, {
name: 'OnCall',
return await this.grafanaBackend.post(`${this.SERVICE_ACCOUNTS_BASE_URL}/${serviceAccount.id}/tokens`, {
name: PluginState.ONCALL_KEY_NAME,
role: 'Admin',
secondsToLive: null,
});
};

Expand Down
81 changes: 51 additions & 30 deletions grafana-plugin/src/state/plugin/plugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,38 +179,59 @@ describe('PluginState.updateGrafanaPluginSettings', () => {
});

describe('PluginState.createGrafanaToken', () => {
test.each([true, false])('it calls the proper methods - existing key: %s', async (onCallKeyExists) => {
const baseUrl = '/api/auth/keys';
const onCallKeyId = 12345;
const onCallKeyName = 'OnCall';
const onCallKey = { name: onCallKeyName, id: onCallKeyId };
const existingKeys = [{ name: 'foo', id: 9595 }];

PluginState.grafanaBackend.get = jest
.fn()
.mockResolvedValueOnce(onCallKeyExists ? [...existingKeys, onCallKey] : existingKeys);
PluginState.grafanaBackend.delete = jest.fn();
PluginState.grafanaBackend.post = jest.fn();

await PluginState.createGrafanaToken();

expect(PluginState.grafanaBackend.get).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.get).toHaveBeenCalledWith(baseUrl);
const cases = [
[true, true, false],
[true, false, false],
[false, true, true],
[false, true, false],
[false, false, false],
];

test.each(cases)(
'it calls the proper methods - existing key: %s, existing sa: %s, existing token: %s',
async (apiKeyExists, saExists, apiTokenExists) => {
const baseUrl = PluginState.KEYS_BASE_URL;
const serviceAccountBaseUrl = PluginState.SERVICE_ACCOUNTS_BASE_URL;
const apiKeyId = 12345;
const apiKeyName = PluginState.ONCALL_KEY_NAME;
const apiKey = { name: apiKeyName, id: apiKeyId };
const saId = 33333;
const serviceAccount = { id: saId };

PluginState.getGrafanaToken = jest.fn().mockReturnValueOnce(apiKeyExists ? apiKey : null);
PluginState.grafanaBackend.delete = jest.fn();
PluginState.grafanaBackend.post = jest.fn();

PluginState.getServiceAccount = jest.fn().mockReturnValueOnce(saExists ? serviceAccount : null);
PluginState.getOrCreateServiceAccount = jest.fn().mockReturnValueOnce(serviceAccount);
PluginState.getTokenFromServiceAccount = jest.fn().mockReturnValueOnce(apiTokenExists ? apiKey : null);

await PluginState.createGrafanaToken();

expect(PluginState.getGrafanaToken).toHaveBeenCalledTimes(1);

if (apiKeyExists) {
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledWith(`${baseUrl}/${apiKey.id}`);
} else if (apiTokenExists) {
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledWith(
`${serviceAccountBaseUrl}/${serviceAccount.id}/tokens/${apiKey.id}`
);
} else {
expect(PluginState.grafanaBackend.delete).not.toHaveBeenCalled();
}

if (onCallKeyExists) {
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.delete).toHaveBeenCalledWith(`${baseUrl}/${onCallKeyId}`);
} else {
expect(PluginState.grafanaBackend.delete).not.toHaveBeenCalled();
expect(PluginState.grafanaBackend.post).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.post).toHaveBeenCalledWith(
`${serviceAccountBaseUrl}/${serviceAccount.id}/tokens`,
{
name: apiKeyName,
role: 'Admin',
}
);
}

expect(PluginState.grafanaBackend.post).toHaveBeenCalledTimes(1);
expect(PluginState.grafanaBackend.post).toHaveBeenCalledWith(baseUrl, {
name: onCallKeyName,
role: 'Admin',
secondsToLive: null,
});
});
);
});

describe('PluginState.getPluginSyncStatus', () => {
Expand Down