Skip to content

Commit

Permalink
feat(feature-manager): remove Sentry from feature manager (#2130)
Browse files Browse the repository at this point in the history
* feat(feature-manager): remove Sentry from feature manager

* build(dev): add dev defaults for Sentry config
  • Loading branch information
mantariksh committed Jun 11, 2021
1 parent e62494e commit d087588
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 46 deletions.
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ services:
- MYINFO_CLIENT_SECRET=mockClientSecret
- WEBHOOK_SQS_URL=http://localhost:4566/000000000000/local-webhooks-sqs-main
- INTRANET_IP_LIST_PATH
- SENTRY_CONFIG_URL=https://random@sentry.io/123456
- CSP_REPORT_URI=https://random@sentry.io/123456
- GA_TRACKING_ID
- SENTRY_CONFIG_URL
- TWILIO_ACCOUNT_SID
- TWILIO_API_KEY
- TWILIO_API_SECRET
Expand Down
2 changes: 0 additions & 2 deletions src/app/config/feature-manager/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import FeatureManager from './util/FeatureManager.class'
import captcha from './captcha.config'
import googleAnalytics from './google-analytics.config'
import sentry from './sentry.config'
import sms from './sms.config'
import spcpMyInfo from './spcp-myinfo.config'
import verifiedFields from './verified-fields.config'
Expand All @@ -13,7 +12,6 @@ const featureManager = new FeatureManager()

// Register features and associated middleware/fallbacks
featureManager.register(captcha)
featureManager.register(sentry)
featureManager.register(googleAnalytics)
featureManager.register(spcpMyInfo)
featureManager.register(webhookVerifiedContent)
Expand Down
41 changes: 24 additions & 17 deletions src/app/config/feature-manager/sentry.config.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { FeatureNames, RegisterableFeature } from './types'
import convict, { Schema } from 'convict'
import { url } from 'convict-format-with-validator'

const sentryFeature: RegisterableFeature<FeatureNames.Sentry> = {
name: FeatureNames.Sentry,
schema: {
sentryConfigUrl: {
doc: 'Sentry.io URL for configuring the Sentry SDK',
format: 'url',
default: null,
env: 'SENTRY_CONFIG_URL',
},
cspReportUri: {
doc: 'Endpoint for content security policy reporting',
format: 'url',
default: null,
env: 'CSP_REPORT_URI',
},
export interface ISentry {
sentryConfigUrl: string
cspReportUri: string
}

convict.addFormat(url)

const sentryFeature: Schema<ISentry> = {
sentryConfigUrl: {
doc: 'Sentry.io URL for configuring the Sentry SDK',
format: 'url',
default: null,
env: 'SENTRY_CONFIG_URL',
},
cspReportUri: {
doc: 'Endpoint for content security policy reporting',
format: 'url',
default: null,
env: 'CSP_REPORT_URI',
},
}

export default sentryFeature
export const sentryConfig = convict(sentryFeature)
.validate({ allowed: 'strict' })
.getProperties()
7 changes: 0 additions & 7 deletions src/app/config/feature-manager/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Schema } from 'convict'
export enum FeatureNames {
Captcha = 'captcha',
GoogleAnalytics = 'google-analytics',
Sentry = 'sentry',
Sms = 'sms',
SpcpMyInfo = 'spcp-myinfo',
VerifiedFields = 'verified-fields',
Expand All @@ -20,11 +19,6 @@ export interface IGoogleAnalytics {
GATrackingID: string
}

export interface ISentry {
sentryConfigUrl: string
cspReportUri: string
}

export interface ISms {
twilioAccountSid: string
twilioApiKey: string
Expand Down Expand Up @@ -79,7 +73,6 @@ export interface IWebhookVerifiedContent {
export interface IFeatureManager {
[FeatureNames.Captcha]: ICaptcha
[FeatureNames.GoogleAnalytics]: IGoogleAnalytics
[FeatureNames.Sentry]: ISentry
[FeatureNames.Sms]: ISms
[FeatureNames.SpcpMyInfo]: ISpcpMyInfo
[FeatureNames.VerifiedFields]: IVerifiedFields
Expand Down
12 changes: 5 additions & 7 deletions src/app/loaders/express/__tests__/helmet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import helmet from 'helmet'
import { mocked } from 'ts-jest/utils'

import config from 'src/app/config/config'
import featureManager from 'src/app/config/feature-manager'
import { sentryConfig } from 'src/app/config/feature-manager/sentry.config'

import expressHandler from 'tests/unit/backend/helpers/jest-express'

Expand All @@ -11,10 +11,10 @@ import helmetMiddlewares from '../helmet'
describe('helmetMiddlewares', () => {
jest.mock('helmet')
const mockHelmet = mocked(helmet, true)
jest.mock('src/app/config/feature-manager')
const mockFeatureManager = mocked(featureManager, true)
jest.mock('src/app/config/config')
const mockConfig = mocked(config, true)
jest.mock('src/app/config/feature-manager/sentry.config')
const mockSentryConfig = mocked(sentryConfig, true)

const cspCoreDirectives = {
defaultSrc: ["'self'"],
Expand Down Expand Up @@ -136,9 +136,7 @@ describe('helmetMiddlewares', () => {
})

it('should call helmet.contentSecurityPolicy() with the correct directives if cspReportUri and !isDev', () => {
mockFeatureManager.props = jest
.fn()
.mockReturnValue({ cspReportUri: 'value' })
mockSentryConfig.cspReportUri = 'value'
mockConfig.isDev = false
helmetMiddlewares()
expect(mockHelmet.contentSecurityPolicy).toHaveBeenCalledWith({
Expand All @@ -151,7 +149,7 @@ describe('helmetMiddlewares', () => {
})

it('should call helmet.contentSecurityPolicy() with the correct directives if !cspReportUri and isDev', () => {
mockFeatureManager.props = jest.fn()
mockSentryConfig.cspReportUri = ''
mockConfig.isDev = true
helmetMiddlewares()
expect(mockHelmet.contentSecurityPolicy).toHaveBeenCalledWith({
Expand Down
9 changes: 2 additions & 7 deletions src/app/loaders/express/helmet.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { RequestHandler } from 'express'
import helmet from 'helmet'
import { ContentSecurityPolicyOptions } from 'helmet/dist/middlewares/content-security-policy'
import { get } from 'lodash'

import config from '../../config/config'
import featureManager, { FeatureNames } from '../../config/feature-manager'
import { sentryConfig } from '../../config/feature-manager/sentry.config'

const helmetMiddlewares = () => {
// Only add the "Strict-Transport-Security" header if request is https.
Expand Down Expand Up @@ -79,11 +78,7 @@ const helmetMiddlewares = () => {
formAction: ["'self'"],
}

const reportUri = get(
featureManager.props(FeatureNames.Sentry),
'cspReportUri',
undefined,
)
const reportUri = sentryConfig.cspReportUri

const cspOptionalDirectives: ContentSecurityPolicyOptions['directives'] = {}

Expand Down
7 changes: 2 additions & 5 deletions src/app/loaders/express/locals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { get } from 'lodash'

import config from '../../config/config'
import featureManager, { FeatureNames } from '../../config/feature-manager'
import { sentryConfig } from '../../config/feature-manager/sentry.config'

// Construct js with environment variables needed by frontend
const frontendVars = {
Expand All @@ -17,11 +18,7 @@ const frontendVars = {
'captchaPublicKey',
null,
), // Recaptcha
sentryConfigUrl: get(
featureManager.props(FeatureNames.Sentry),
'sentryConfigUrl',
null,
), // Sentry.IO
sentryConfigUrl: sentryConfig.sentryConfigUrl, // Sentry.IO
isSPMaintenance: get(
featureManager.props(FeatureNames.SpcpMyInfo),
'isSPMaintenance',
Expand Down

0 comments on commit d087588

Please sign in to comment.