Skip to content

Commit

Permalink
fix(amazonq): update security scan languages (aws#5523)
Browse files Browse the repository at this point in the history
Sometimes security issues have a Diagnostic but not SecurityIssueHover
issue. This happens when the document's languageId is not enabled when
registering the hover/code actions provider.

Update the enabled languageIds for security scans feature.

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
  • Loading branch information
ctlai95 authored and ivikash committed Sep 4, 2024
1 parent 2c8d666 commit 6f3edaf
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Add getFeature and isEnabled utility methods to FeatureConfigProvider"
}
59 changes: 44 additions & 15 deletions packages/core/src/shared/featureConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,21 @@ export class FeatureContext {
) {}
}

const testFeatureName = 'testFeature'
const customizationArnOverrideName = 'customizationArnOverride'
const featureConfigPollIntervalInMs = 30 * 60 * 1000 // 30 mins
const dataCollectionFeatureName = 'IDEProjectContextDataCollection'

// TODO: add real feature later
export const featureDefinitions = new Map([
[testFeatureName, new FeatureContext(testFeatureName, 'CONTROL', { stringValue: 'testValue' })],
export const Features = {
customizationArnOverride: 'customizationArnOverride',
dataCollectionFeature: 'IDEProjectContextDataCollection',
test: 'testFeature',
} as const

export type FeatureName = (typeof Features)[keyof typeof Features]

export const featureDefinitions = new Map<FeatureName, FeatureContext>([
[Features.test, new FeatureContext(Features.test, 'CONTROL', { stringValue: 'testValue' })],
[
customizationArnOverrideName,
new FeatureContext(customizationArnOverrideName, 'customizationARN', { stringValue: '' }),
Features.customizationArnOverride,
new FeatureContext(Features.customizationArnOverride, 'customizationARN', { stringValue: '' }),
],
])

Expand Down Expand Up @@ -103,11 +107,12 @@ export class FeatureConfigProvider {
})
getLogger().info('AB Testing Cohort Assignments %s', JSON.stringify(response.featureEvaluations))

const customizationArnOverride = this.featureConfigs.get(customizationArnOverrideName)?.value?.stringValue
const customizationArnOverride = this.featureConfigs.get(Features.customizationArnOverride)?.value
?.stringValue
if (customizationArnOverride !== undefined) {
// Double check if server-side wrongly returns a customizationArn to BID users
if (isBuilderIdConnection(AuthUtil.instance.conn)) {
this.featureConfigs.delete(customizationArnOverrideName)
this.featureConfigs.delete(Features.customizationArnOverride)
} else if (isIdcSsoConnection(AuthUtil.instance.conn)) {
let availableCustomizations = undefined
try {
Expand All @@ -131,12 +136,12 @@ export class FeatureConfigProvider {
getLogger().debug(
`Customization arn ${customizationArnOverride} not available in listAvailableCustomizations, not using`
)
this.featureConfigs.delete(customizationArnOverrideName)
this.featureConfigs.delete(Features.customizationArnOverride)
}
}
}

const dataCollectionValue = this.featureConfigs.get(dataCollectionFeatureName)?.value.stringValue
const dataCollectionValue = this.featureConfigs.get(Features.dataCollectionFeature)?.value.stringValue
if (dataCollectionValue === 'data-collection') {
this._isDataCollectionGroup = true
// Enable local workspace index by default, for Amzn users.
Expand Down Expand Up @@ -172,16 +177,16 @@ export class FeatureConfigProvider {
// 6) Add a test case for this feature.
// 7) In case `getXXX()` returns undefined, it should be treated as a default/control group.
getTestFeature(): string | undefined {
return this.getFeatureValueForKey(testFeatureName).stringValue
return this.getFeatureValueForKey(Features.test).stringValue
}

getCustomizationArnOverride(): string | undefined {
return this.getFeatureValueForKey(customizationArnOverrideName).stringValue
return this.getFeatureValueForKey(Features.customizationArnOverride).stringValue
}

// Get the feature value for the given key.
// In case of a misconfiguration, it will return a default feature value of Boolean true.
private getFeatureValueForKey(name: string): FeatureValue {
private getFeatureValueForKey(name: FeatureName): FeatureValue {
return this.featureConfigs.get(name)?.value ?? featureDefinitions.get(name)?.value ?? { boolValue: true }
}

Expand All @@ -193,4 +198,28 @@ export class FeatureConfigProvider {
public static getFeatureConfigs(): Map<string, FeatureContext> {
return FeatureConfigProvider.instance.featureConfigs
}

/**
* Retrieves the FeatureContext object for a given feature name.
*
* @param {string} featureName - The name of the feature.
* @returns {FeatureContext | undefined} The FeatureContext object for the specified feature, or undefined if the feature doesn't exist.
*/
public static getFeature(featureName: FeatureName): FeatureContext | undefined {
return FeatureConfigProvider.instance.featureConfigs.get(featureName)
}

/**
* Checks if a feature is active or not.
*
* @param {string} featureName - The name of the feature to check.
* @returns {boolean} False if the variation is not CONTROL, otherwise True
*/
public static isEnabled(featureName: FeatureName): boolean {
const featureContext = FeatureConfigProvider.getFeature(featureName)
if (featureContext && featureContext.variation.toLocaleLowerCase() !== 'control') {
return true
}
return false
}
}
10 changes: 10 additions & 0 deletions packages/core/src/test/fake/mockFeatureConfigData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@ export const mockFeatureConfigsData: FeatureEvaluation[] = [
variation: 'TREATMENT',
value: { stringValue: 'testValue' },
},
{
feature: 'featureA',
variation: 'CONTROL',
value: { stringValue: 'testValue' },
},
{
feature: 'featureB',
variation: 'TREATMENT',
value: { stringValue: 'testValue' },
},
]
74 changes: 61 additions & 13 deletions packages/core/src/test/shared/featureConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,33 @@
import assert from 'assert'
import sinon from 'sinon'
import { AWSError, Request } from 'aws-sdk'
import { FeatureConfigProvider, featureDefinitions } from '../../shared/featureConfig'
import { Features, FeatureConfigProvider, featureDefinitions, FeatureName } from '../../shared/featureConfig'
import { ListFeatureEvaluationsResponse } from '../../codewhisperer'
import { createSpyClient } from '../codewhisperer/testUtil'
import { mockFeatureConfigsData } from '../fake/mockFeatureConfigData'

describe('FeatureConfigProvider', () => {
beforeEach(async () => {
const clientSpy = await createSpyClient()
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
promise: () =>
Promise.resolve({
$response: {
requestId: '',
},
featureEvaluations: mockFeatureConfigsData,
}),
} as Request<ListFeatureEvaluationsResponse, AWSError>)
await FeatureConfigProvider.instance.fetchFeatureConfigs()
})

afterEach(function () {
sinon.restore()
})

it('featureDefinitions map is not empty', () => {
assert.notStrictEqual(featureDefinitions.size, 0)
assert.ok(featureDefinitions.has('testFeature'))
assert.ok(featureDefinitions.has(Features.test))
})

it('provider has getters for all the features', () => {
Expand All @@ -32,18 +46,52 @@ describe('FeatureConfigProvider', () => {
})

it('test getFeatureConfigsTelemetry will return expected string', async () => {
const clientSpy = await createSpyClient()
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
promise: () =>
Promise.resolve({
$response: {
requestId: '',
assert.strictEqual(
FeatureConfigProvider.instance.getFeatureConfigsTelemetry(),
`{testFeature: TREATMENT, featureA: CONTROL, featureB: TREATMENT}`
)
})

it('should should return all feature flags', async () => {
it('should should return all feature flags', async () => {
const featureConfigs = FeatureConfigProvider.getFeatureConfigs()
const expectedFeatureConfigs = {
featureA: {
name: 'featureA',
value: {
stringValue: 'testValue',
},
featureEvaluations: mockFeatureConfigsData,
}),
} as Request<ListFeatureEvaluationsResponse, AWSError>)
variation: 'CONTROL',
},
featureB: {
name: 'featureB',
value: {
stringValue: 'testValue',
},
variation: 'TREATMENT',
},
testFeature: {
name: 'testFeature',
value: {
stringValue: 'testValue',
},
variation: 'TREATMENT',
},
}

await FeatureConfigProvider.instance.fetchFeatureConfigs()
assert.strictEqual(FeatureConfigProvider.instance.getFeatureConfigsTelemetry(), `{testFeature: TREATMENT}`)
assert.deepStrictEqual(Object.fromEntries(featureConfigs), expectedFeatureConfigs)
})
})

it('should test featureA as disabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('featureA' as FeatureName), false)
})

it('should test featureB as enabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('featureB' as FeatureName), true)
})

it('should test feature-does-not-exist as disabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('feature-does-not-exist' as FeatureName), false)
})
})

0 comments on commit 6f3edaf

Please sign in to comment.