-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
### Description This PR moves the direct REST API calls from the frontend component to the backend. Instead of leveraging the Backstage proxy to make direct API calls to PagerDuty the calls are made now to the backend plugin. This removes the dependency on the proxy and prevents other plugins from using the PagerDuty proxy configuration to call PagerDuty APIs for other purposes which raises few security concerns. This PR also fixes an issue with the on-call users not showing up (#58). **Issue number:** #37. ### Type of change - [x] New feature (non-breaking change which adds functionality) - [x] Fix (non-breaking change which fixes an issue) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) ### Checklist - [x] I have performed a self-review of this change - [x] Changes have been tested - [x] Changes are documented - [x] Changes generate *no new warnings* - [x] PR title follows [conventional commit semantics](https://www.conventionalcommits.org/en/v1.0.0/) If this is a breaking change 👇 - [ ] I have documented the migration process - [ ] I have implemented necessary warnings (if it can live side by side) ## Acknowledgement By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. **Disclaimer:** We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.
- Loading branch information
Showing
19 changed files
with
164 additions
and
413 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,32 +15,23 @@ | |
| */ | ||
| import { | ||
| PagerDutyApi, | ||
| PagerDutyChangeEvent, | ||
| PagerDutyEntity, | ||
| PagerDutyIncident, | ||
| PagerDutyTriggerAlarmRequest, | ||
| } from '../src'; | ||
| import { PagerDutyChangeEvent, PagerDutyIncident, PagerDutyUser } from '@pagerduty/backstage-plugin-common'; | ||
| import { Entity } from '@backstage/catalog-model'; | ||
|
|
||
| export const mockPagerDutyApi: PagerDutyApi = { | ||
| async getServiceByPagerDutyEntity(pagerDutyEntity: PagerDutyEntity) { | ||
| return { | ||
| service: { | ||
| name: pagerDutyEntity.name, | ||
| integrationKey: 'key', | ||
| id: '123', | ||
| html_url: 'http://service', | ||
| id: "SERV1CE1D", | ||
| html_url: "www.example.com", | ||
| escalation_policy: { | ||
| id: '123', | ||
| html_url: 'http://escalationpolicy', | ||
| user: { | ||
| id: '123', | ||
| summary: 'summary', | ||
| email: '[email protected]', | ||
| html_url: 'http://user', | ||
| name: 'some-user', | ||
| avatar_url: 'http://avatar', | ||
| }, | ||
| id: "ESCALAT1ONP01ICY1D", | ||
| name: "ep-one", | ||
| html_url: "http://www.example.com/escalation-policy/ESCALAT1ONP01ICY1D", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
@@ -50,20 +41,12 @@ export const mockPagerDutyApi: PagerDutyApi = { | |
| return { | ||
| service: { | ||
| name: entity.metadata.name, | ||
| integrationKey: 'key', | ||
| id: '123', | ||
| html_url: 'http://service', | ||
| id: "SERV1CE1D", | ||
| html_url: "www.example.com", | ||
| escalation_policy: { | ||
| id: '123', | ||
| html_url: 'http://escalationpolicy', | ||
| user: { | ||
| id: '123', | ||
| summary: 'summary', | ||
| email: '[email protected]', | ||
| html_url: 'http://user', | ||
| name: 'some-user', | ||
| avatar_url: 'http://avatar', | ||
| }, | ||
| id: "ESCALAT1ONP01ICY1D", | ||
| name: "ep-one", | ||
| html_url: "http://www.example.com/escalation-policy/ESCALAT1ONP01ICY1D", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
@@ -85,12 +68,17 @@ export const mockPagerDutyApi: PagerDutyApi = { | |
| }, | ||
| }, | ||
| ], | ||
| serviceId: serviceId, | ||
| service: { | ||
| id: serviceId, | ||
| summary: 'service summary', | ||
| html_url: 'http://service', | ||
| }, | ||
| created_at: '2015-10-06T21:30:42Z', | ||
| } as PagerDutyIncident; | ||
| }; | ||
|
|
||
| return { | ||
|
|
||
| incidents: [ | ||
| incident('Some Alerting Incident'), | ||
| incident('Another Alerting Incident'), | ||
|
|
@@ -124,23 +112,24 @@ export const mockPagerDutyApi: PagerDutyApi = { | |
| }, | ||
|
|
||
| async getOnCallByPolicyId() { | ||
| const oncall = (id: string, name: string, escalation: number) => { | ||
| const oncall = (id: string, name: string) => { | ||
| return { | ||
| user: { | ||
| id: id, | ||
| name: name, | ||
| html_url: 'http://assignee', | ||
| summary: 'summary', | ||
| email: '[email protected]', | ||
| avatar_url: 'http://avatar', | ||
| }, | ||
| escalation_level: escalation, | ||
| }; | ||
| }; | ||
|
|
||
| return { | ||
| oncalls: [oncall('1', 'Jane Doe', 1), oncall('2', 'John Doe', 2), oncall('3', 'James Doe', 1)], | ||
| }; | ||
| const users: PagerDutyUser[] = [ | ||
| oncall('1', 'Jane Doe'), | ||
| oncall('2', 'John Doe'), | ||
| oncall('3', 'James Doe'), | ||
| ]; | ||
|
|
||
| return users; | ||
| }, | ||
|
|
||
| async triggerAlarm(request: PagerDutyTriggerAlarmRequest) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ | |
| import { MockFetchApi } from '@backstage/test-utils'; | ||
| import { DiscoveryApi } from '@backstage/core-plugin-api'; | ||
| import { PagerDutyClient, UnauthorizedError } from './client'; | ||
| import { PagerDutyService, PagerDutyUser } from '../components/types'; | ||
| import { PagerDutyService } from '@pagerduty/backstage-plugin-common'; | ||
| import { NotFoundError } from '@backstage/errors'; | ||
| import { Entity } from '@backstage/catalog-model'; | ||
|
|
||
|
|
@@ -25,7 +25,7 @@ const mockDiscoveryApi: jest.Mocked<DiscoveryApi> = { | |
| getBaseUrl: jest | ||
| .fn() | ||
| .mockName('discoveryApi') | ||
| .mockResolvedValue('http://localhost:7007/proxy'), | ||
| .mockResolvedValue('http://localhost:7007/pagerduty'), | ||
| }; | ||
| const mockFetchApi: MockFetchApi = new MockFetchApi({ | ||
| baseImplementation: mockFetch, | ||
|
|
@@ -34,25 +34,15 @@ const mockFetchApi: MockFetchApi = new MockFetchApi({ | |
| let client: PagerDutyClient; | ||
| let entity: Entity; | ||
|
|
||
| const user: PagerDutyUser = { | ||
| name: 'person1', | ||
| id: 'p1', | ||
| summary: 'person1', | ||
| email: '[email protected]', | ||
| html_url: 'http://a.com/id1', | ||
| avatar_url: 'http://a.com/id1/avatar', | ||
| }; | ||
|
|
||
| const service: PagerDutyService = { | ||
| id: 'def456', | ||
| name: 'pagerduty-name', | ||
| html_url: 'www.example.com', | ||
| id: "SERV1CE1D", | ||
| name: "service-one", | ||
| html_url: "www.example.com", | ||
| escalation_policy: { | ||
| id: 'def', | ||
| user: user, | ||
| html_url: 'http://a.com/id1', | ||
| id: "ESCALAT1ONP01ICY1D", | ||
| name: "ep-one", | ||
| html_url: "http://www.example.com/escalation-policy/ESCALAT1ONP01ICY1D", | ||
| }, | ||
| integrationKey: 'abc123', | ||
| }; | ||
|
|
||
| const requestHeaders = { | ||
|
|
@@ -93,14 +83,14 @@ describe('PagerDutyClient', () => { | |
| mockFetch.mockResolvedValueOnce({ | ||
| status: 200, | ||
| ok: true, | ||
| json: () => Promise.resolve({ services: [service] }), | ||
| json: () => Promise.resolve({ service }), | ||
| }); | ||
|
|
||
| expect(await client.getServiceByEntity(entity)).toEqual({ | ||
| service, | ||
| }); | ||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| 'http://localhost:7007/proxy/pagerduty/services?time_zone=UTC&include[]=integrations&include[]=escalation_policies&query=abc123', | ||
| 'http://localhost:7007/pagerduty/services?integration_key=abc123', | ||
| requestHeaders, | ||
| ); | ||
| }); | ||
|
|
@@ -161,7 +151,7 @@ describe('PagerDutyClient', () => { | |
| mockFetch.mockResolvedValueOnce({ | ||
| status: 200, | ||
| ok: true, | ||
| json: () => Promise.resolve({ services: [] }), | ||
| json: () => Promise.resolve({ }), | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -181,7 +171,7 @@ describe('PagerDutyClient', () => { | |
| metadata: { | ||
| name: 'pagerduty-test', | ||
| annotations: { | ||
| 'pagerduty.com/service-id': 'def456', | ||
| 'pagerduty.com/service-id': 'SERVICE1D', | ||
| }, | ||
| }, | ||
| }; | ||
|
|
@@ -198,7 +188,7 @@ describe('PagerDutyClient', () => { | |
| service, | ||
| }); | ||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| 'http://localhost:7007/proxy/pagerduty/services/def456?time_zone=UTC&include[]=integrations&include[]=escalation_policies', | ||
| 'http://localhost:7007/pagerduty/services/SERVICE1D', | ||
| requestHeaders, | ||
| ); | ||
| }); | ||
|
|
@@ -270,18 +260,18 @@ describe('PagerDutyClient', () => { | |
| }; | ||
| }); | ||
|
|
||
| it('queries proxy path by integration id', async () => { | ||
| it('queries pagerduty path by integration id', async () => { | ||
| mockFetch.mockResolvedValueOnce({ | ||
| status: 200, | ||
| ok: true, | ||
| json: () => Promise.resolve({ services: [service] }), | ||
| json: () => Promise.resolve({ service }), | ||
| }); | ||
|
|
||
| expect(await client.getServiceByEntity(entity)).toEqual({ | ||
| service, | ||
| }); | ||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| 'http://localhost:7007/proxy/pagerduty/services?time_zone=UTC&include[]=integrations&include[]=escalation_policies&query=abc123', | ||
| 'http://localhost:7007/pagerduty/services?integration_key=abc123', | ||
| requestHeaders, | ||
| ); | ||
| }); | ||
|
|
@@ -342,7 +332,7 @@ describe('PagerDutyClient', () => { | |
| mockFetch.mockResolvedValueOnce({ | ||
| status: 200, | ||
| ok: true, | ||
| json: () => Promise.resolve({ services: [] }), | ||
| json: () => Promise.resolve({ }), | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.