diff --git a/.github/workflows/deploy-azure.yml b/.github/workflows/deploy-azure.yml index 3eca9ff9..76744033 100644 --- a/.github/workflows/deploy-azure.yml +++ b/.github/workflows/deploy-azure.yml @@ -45,6 +45,7 @@ jobs: files: 'src/WebsiteHost/**/.env.deploy' variables: ${{ toJSON(vars)}} secrets: ${{ toJSON(secrets)}} + warnOnAdditionalVars: false - name: Build WebsiteHost (FrontEnd) for Deploy run: | cd src/WebsiteHost/ClientApp @@ -56,6 +57,8 @@ jobs: files: '**/appsettings.json,**/appsettings.Azure.json' variables: ${{ toJSON(vars)}} secrets: ${{ toJSON(secrets)}} + warnOnAdditionalVars: true + ignoreAdditionalVars: '^DEPLOY_' - name: Package (ApiHost1) for Deploy run: dotnet publish --configuration ${{env.DEPLOY_BUILD_CONFIGURATION}} "src/ApiHost1/ApiHost1.csproj" --output ".\publish\ApiHost1" /p:HostingPlatform=${{env.HOSTED_ON}} - name: Package (WebsiteHost) for Deploy diff --git a/src/SaaStack.sln.DotSettings b/src/SaaStack.sln.DotSettings index 90d638f9..acee66fb 100644 --- a/src/SaaStack.sln.DotSettings +++ b/src/SaaStack.sln.DotSettings @@ -1750,6 +1750,7 @@ public void When$condition$_Then$outcome$() True True True + True True True True @@ -1795,6 +1796,7 @@ public void When$condition$_Then$outcome$() True True True + True True True True diff --git a/src/Tools.GitHubActions/VariableSubstitution/README.md b/src/Tools.GitHubActions/VariableSubstitution/README.md index 50a50f69..3aa775a5 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/README.md +++ b/src/Tools.GitHubActions/VariableSubstitution/README.md @@ -142,15 +142,28 @@ In the GitHub project, you would define the following variables or secrets: **Required**. This must have this specific value in the YML file: `${{ toJSON(vars)}}`. -## Outputs -None. +### `warnOnAdditionalVars` + +**Optional**. If set to `true`, the action will create a build warning if there are additional variables or secrets in the GitHub repository that are not substituted into any `appsettings.json`/`.env` files. Default `false`. + +### `ignoreAdditionalVars` + +**Optional**. A regular expression that matches any GitHub variables/secrets that should be ignored if `warnOnAdditionalVars` is `true`. + +## Outputs All variables, in all files found by the `files` input, will be substituted with the values of the variables/secrets found in the GitHub repository, for the specified environment. +Additional build warnings will be raised in these cases: +* In `appsettings.json` files, when a declared `Deploy -> Required -> Keys` key in the `appsettings.json` file is present in specific `appsettings.json` file. +* In all files, when a variable is substituted and the variable is both defined as a secret and a variable in the GitHub repository. In this case, the secret value will be preferred. + +> These warnings cannot be suppressed. + ## Example usage -For C# host projects: +For .NET host projects: ```yaml jobs: @@ -164,6 +177,8 @@ jobs: files: '**/appsettings.json' variables: ${{ toJSON(vars)}} secrets: ${{ toJSON(secrets)}} + warnOnAdditionalVars: true + ignoreAdditionalVars: '^DEPLOY_' ``` For JavaScript/NodeJS projects: @@ -180,6 +195,8 @@ jobs: files: '**/WebsiteHost/**/.env.deploy' variables: ${{ toJSON(vars)}} secrets: ${{ toJSON(secrets)}} + warnOnAdditionalVars: true + ignoreAdditionalVars: '^DEPLOY_' ``` > Note: That you should specify the name of the deployment environment that you have set up in your GitHub project, on the job itself. When you do, all secrets and variables from that environment, plus those form the GitHub repository (plus those from your GitHub organization). \ No newline at end of file diff --git a/src/Tools.GitHubActions/VariableSubstitution/action.yml b/src/Tools.GitHubActions/VariableSubstitution/action.yml index b32db617..7cbe1201 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/action.yml +++ b/src/Tools.GitHubActions/VariableSubstitution/action.yml @@ -11,6 +11,13 @@ inputs: variables: description: 'The variables of the GitHub repository (and environment). Must be the value: \$\{\{ toJSON(vars)\}\}' required: true + warnOnAdditionalVars: + description: 'Whether to output warnings if there are additional variables or secrets in the GitHub repository that are not substituted into any files' + required: false + default: false + ignoreAdditionalVars: + description: 'A regular expression that matches any GitHub variables/secrets that should be ignored if `warnOnAdditionalVars` is enabled' + required: false runs: using: 'node20' main: './built/index.js' diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.spec.ts b/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.spec.ts index d7e0a279..441b3a27 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.spec.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.spec.ts @@ -2,6 +2,7 @@ import {ILogger} from "./logger"; import {AppSettingRequiredVariable, SettingsFileProcessorMessages} from "./settingsFileProcessor"; import {IFileReaderWriter} from "./fileReaderWriter"; import {AppSettingsJsonFileProcessor} from "./appSettingsJsonFileProcessor"; +import {WarningOptions} from "./main"; describe('getVariables', () => { const logger: jest.Mocked = { @@ -246,13 +247,13 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {}); + const result = await processor.substitute(new WarningOptions(), {}, {}); expect(result).toEqual(true); expect(readerWriter.readSettingsFile).not.toHaveBeenCalled(); expect(logger.info).not.toHaveBeenCalled(); }); - + it('should not substitute, when read file throws', async () => { const readerWriter: jest.Mocked = { @@ -261,7 +262,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"aname": "avalue"}, {}); + const result = await processor.substitute(new WarningOptions(), {"aname": "avalue"}, {}); expect(result).toEqual(false); expect(readerWriter.readSettingsFile).toHaveBeenCalledWith("apath"); @@ -278,7 +279,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"aname": "avalue"}, {}); + const result = await processor.substitute(new WarningOptions(), {"aname": "avalue"}, {}); expect(result).toEqual(false); expect(readerWriter.readSettingsFile).toHaveBeenCalledWith("apath"); @@ -295,7 +296,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"ANAME": "avalue2"}, {}); + const result = await processor.substitute(new WarningOptions(), {"ANAME": "avalue2"}, {}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", {"aname": "avalue2"}); @@ -311,7 +312,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {"ANAME": "avalue2"}); + const result = await processor.substitute(new WarningOptions(), {}, {"ANAME": "avalue2"}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", {"aname": "avalue2"}); @@ -333,7 +334,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {"ALEVEL1_ALEVEL2_ALEVEL3": "avalue2"}); + const result = await processor.substitute(new WarningOptions(), {}, {"ALEVEL1_ALEVEL2_ALEVEL3": "avalue2"}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", { @@ -371,7 +372,7 @@ describe('substitute', () => { }; const processor = new AppSettingsJsonFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {"ALEVEL1_ALEVEL2_ALEVEL3": "avalue2"}); + const result = await processor.substitute(new WarningOptions(), {}, {"ALEVEL1_ALEVEL2_ALEVEL3": "avalue2"}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", expect.objectContaining({ diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.ts b/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.ts index 25566b96..981cf6fb 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/appSettingsJsonFileProcessor.ts @@ -8,6 +8,7 @@ import { ISettingsFileProcessor, SettingsFileProcessorMessages } from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; export class AppSettingsJsonFileProcessor implements ISettingsFileProcessor { _logger: ILogger; @@ -41,7 +42,7 @@ export class AppSettingsJsonFileProcessor implements ISettingsFileProcessor { } } - private static substituteVariablesRecursively(logger: ILogger, gitHubVariables: any, gitHubSecrets: any, json: any, prefix: string = "") { + private static substituteVariablesRecursively(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any, json: any, prefix: string = "") { for (const key in json) { if (json.hasOwnProperty(key)) { const element = json[key]; @@ -51,12 +52,12 @@ export class AppSettingsJsonFileProcessor implements ISettingsFileProcessor { if (deployKey) { json[key] = SettingsFileProcessorMessages.redactedDeployMessage(); } else { - AppSettingsJsonFileProcessor.substituteVariablesRecursively(logger, gitHubVariables, gitHubSecrets, element, fullyQualifiedVariableName); + AppSettingsJsonFileProcessor.substituteVariablesRecursively(logger, warningOptions, gitHubVariables, gitHubSecrets, element, fullyQualifiedVariableName); } } else { if (typeof element === "string" || typeof element === "number" || typeof element === "boolean") { const githubVariableName = GitHubVariables.calculateVariableOrSecretName(fullyQualifiedVariableName); - const gitHubSecretOrVariableValue = GitHubVariables.getVariableOrSecretValue(gitHubVariables, gitHubSecrets, githubVariableName); + const gitHubSecretOrVariableValue = GitHubVariables.getVariableOrSecretValue(logger, warningOptions, gitHubVariables, gitHubSecrets, githubVariableName); if (gitHubSecretOrVariableValue) { logger.info(SettingsFileProcessorMessages.substitutingVariable(fullyQualifiedVariableName)); json[key] = gitHubSecretOrVariableValue; @@ -153,7 +154,7 @@ export class AppSettingsJsonFileProcessor implements ISettingsFileProcessor { return `${prefix}:${key}`; } - async substitute(gitHubVariables: any, gitHubSecrets: any): Promise { + async substitute(warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise { if (Object.keys(gitHubVariables).length === 0 && Object.keys(gitHubSecrets).length === 0) { return true; @@ -161,7 +162,7 @@ export class AppSettingsJsonFileProcessor implements ISettingsFileProcessor { try { const json = await this._readerWriter.readSettingsFile(this._path); - AppSettingsJsonFileProcessor.substituteVariablesRecursively(this._logger, gitHubVariables, gitHubSecrets, json); + AppSettingsJsonFileProcessor.substituteVariablesRecursively(this._logger, warningOptions, gitHubVariables, gitHubSecrets, json); await this._readerWriter.writeSettingsFile(this._path, json); this._logger.info(SettingsFileProcessorMessages.substitutingSucceeded(this._path)); return true; diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSet.ts b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSet.ts index a7ea0dfc..43de3a1d 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSet.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSet.ts @@ -1,6 +1,7 @@ import {ISettingsFile, SettingsFile} from "./settingsFile"; import {ILogger} from "./logger"; import {AppSettingRequiredVariable, GitHubVariables} from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; export interface IConfigurationSet { readonly hostProjectPath: string; @@ -12,7 +13,7 @@ export interface IConfigurationSet { verify(logger: ILogger, gitHubVariables: any, gitHubSecrets: any): boolean; - substitute(logger: ILogger, gitHubVariables: any, gitHubSecrets: any): Promise; + substitute(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise; } export class ConfigurationSet implements IConfigurationSet { @@ -47,11 +48,11 @@ export class ConfigurationSet implements IConfigurationSet { return this._definedVariables; } - async substitute(logger: ILogger, gitHubVariables: any, gitHubSecrets: any): Promise { + async substitute(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise { logger.info(ConfigurationSetMessages.startSubstitution(this.hostProjectPath)); let isSetSubstituted = true; for (const file of this.settingFiles) { - const isSubstituted = await file.substitute(logger, gitHubVariables, gitHubSecrets); + const isSubstituted = await file.substitute(logger, warningOptions, gitHubVariables, gitHubSecrets); if (!isSubstituted) { isSetSubstituted = false; } diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.spec.ts b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.spec.ts index 719b23b1..6b7b4abf 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.spec.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.spec.ts @@ -3,6 +3,7 @@ import {ILogger} from "./logger"; import {IGlobPatternParser} from "./globPatternParser"; import {IAppSettingsReaderWriterFactory} from "./appSettingsReaderWriterFactory"; import {AppSettingRequiredVariable, ISettingsFileProcessor} from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; describe('ConfigurationSets', () => { const logger: jest.Mocked = { @@ -26,7 +27,7 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); expect(sets.hasNone).toBe(true); expect(globParser.parseFiles).toHaveBeenCalledWith(["aglobpattern"]); @@ -47,18 +48,18 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); expect(sets.hasNone).toBe(false); expect(sets.length).toBe(1); - expect(sets.sets[0].hostProjectPath).toEqual("."); - expect(sets.sets[0].settingFiles.length).toEqual(1); - expect(sets.sets[0].settingFiles[0].path).toEqual("afile.json"); - expect(sets.sets[0].definedVariables).toEqual(["aname"]); - expect(sets.sets[0].requiredVariables).toEqual([]); + expect(sets._sets[0].hostProjectPath).toEqual("."); + expect(sets._sets[0].settingFiles.length).toEqual(1); + expect(sets._sets[0].settingFiles[0].path).toEqual("afile.json"); + expect(sets._sets[0].definedVariables).toEqual(["aname"]); + expect(sets._sets[0].requiredVariables).toEqual([]); expect(globParser.parseFiles).toHaveBeenCalledWith(["aglobpattern"]); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "afile.json"); - expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets.sets)); + expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets._sets)); }); it('should create a single set, when has one file in a directory', async () => { @@ -67,25 +68,25 @@ describe('ConfigurationSets', () => { parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile.json"])), }; const readerWriter: jest.Mocked = { - getVariables: jest.fn((_path)=> Promise.resolve({variables: ["aname"], requiredVariables: []})), + getVariables: jest.fn(_path => Promise.resolve({variables: ["aname"], requiredVariables: []})), substitute: jest.fn(), }; const readerWriterFactory: jest.Mocked = { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); expect(sets.hasNone).toBe(false); expect(sets.length).toBe(1); - expect(sets.sets[0].hostProjectPath).toEqual("apath"); - expect(sets.sets[0].settingFiles.length).toEqual(1); - expect(sets.sets[0].settingFiles[0].path).toEqual("apath/afile.json"); - expect(sets.sets[0].definedVariables).toEqual(["aname"]); - expect(sets.sets[0].requiredVariables).toEqual([]); + expect(sets._sets[0].hostProjectPath).toEqual("apath"); + expect(sets._sets[0].settingFiles.length).toEqual(1); + expect(sets._sets[0].settingFiles[0].path).toEqual("apath/afile.json"); + expect(sets._sets[0].definedVariables).toEqual(["aname"]); + expect(sets._sets[0].requiredVariables).toEqual([]); expect(globParser.parseFiles).toHaveBeenCalledWith(["aglobpattern"]); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "apath/afile.json"); - expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets.sets)); + expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets._sets)); }); it('should create a single set, when has many files in same directory', async () => { @@ -94,27 +95,30 @@ describe('ConfigurationSets', () => { parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json", "apath/afile2.json"])), }; const readerWriter: jest.Mocked = { - getVariables: jest.fn((_path)=> Promise.resolve({variables: ["aname1", "aname2"], requiredVariables: []})), + getVariables: jest.fn(_path => Promise.resolve({ + variables: ["aname1", "aname2"], + requiredVariables: [] + })), substitute: jest.fn(), }; const readerWriterFactory: jest.Mocked = { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); expect(sets.hasNone).toBe(false); expect(sets.length).toBe(1); - expect(sets.sets[0].hostProjectPath).toEqual("apath"); - expect(sets.sets[0].settingFiles.length).toEqual(2); - expect(sets.sets[0].settingFiles[0].path).toEqual("apath/afile1.json"); - expect(sets.sets[0].settingFiles[1].path).toEqual("apath/afile2.json"); - expect(sets.sets[0].definedVariables).toEqual(["aname1", "aname2"]); - expect(sets.sets[0].requiredVariables).toEqual([]); + expect(sets._sets[0].hostProjectPath).toEqual("apath"); + expect(sets._sets[0].settingFiles.length).toEqual(2); + expect(sets._sets[0].settingFiles[0].path).toEqual("apath/afile1.json"); + expect(sets._sets[0].settingFiles[1].path).toEqual("apath/afile2.json"); + expect(sets._sets[0].definedVariables).toEqual(["aname1", "aname2"]); + expect(sets._sets[0].requiredVariables).toEqual([]); expect(globParser.parseFiles).toHaveBeenCalledWith(["aglobpattern"]); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "apath/afile1.json"); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "apath/afile2.json"); - expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets.sets)); + expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets._sets)); }); it('should create a single set with combined required variables, when both files have Required settings', async () => { @@ -130,23 +134,29 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; readerWriter.getVariables - .mockResolvedValueOnce({variables: ["aname1"], requiredVariables: [new AppSettingRequiredVariable("arequired1", "AREQUIRED1"), new AppSettingRequiredVariable("arequired2", "AREQUIRED2")]}) - .mockResolvedValueOnce({variables: ["aname2"], requiredVariables: [new AppSettingRequiredVariable("arequired2", "AREQUIRED2"), new AppSettingRequiredVariable("arequired3", "AREQUIRED3")]}); + .mockResolvedValueOnce({ + variables: ["aname1"], + requiredVariables: [new AppSettingRequiredVariable("arequired1", "AREQUIRED1"), new AppSettingRequiredVariable("arequired2", "AREQUIRED2")] + }) + .mockResolvedValueOnce({ + variables: ["aname2"], + requiredVariables: [new AppSettingRequiredVariable("arequired2", "AREQUIRED2"), new AppSettingRequiredVariable("arequired3", "AREQUIRED3")] + }); - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); expect(sets.hasNone).toBe(false); expect(sets.length).toBe(1); - expect(sets.sets[0].hostProjectPath).toEqual("apath"); - expect(sets.sets[0].settingFiles.length).toEqual(2); - expect(sets.sets[0].settingFiles[0].path).toEqual("apath/afile1.json"); - expect(sets.sets[0].settingFiles[1].path).toEqual("apath/afile2.json"); - expect(sets.sets[0].definedVariables).toEqual(["aname1", "aname2"]); - expect(sets.sets[0].requiredVariables).toEqual([new AppSettingRequiredVariable("arequired1", "AREQUIRED1"), new AppSettingRequiredVariable("arequired2", "AREQUIRED2"), new AppSettingRequiredVariable("arequired3", "AREQUIRED3")]); + expect(sets._sets[0].hostProjectPath).toEqual("apath"); + expect(sets._sets[0].settingFiles.length).toEqual(2); + expect(sets._sets[0].settingFiles[0].path).toEqual("apath/afile1.json"); + expect(sets._sets[0].settingFiles[1].path).toEqual("apath/afile2.json"); + expect(sets._sets[0].definedVariables).toEqual(["aname1", "aname2"]); + expect(sets._sets[0].requiredVariables).toEqual([new AppSettingRequiredVariable("arequired1", "AREQUIRED1"), new AppSettingRequiredVariable("arequired2", "AREQUIRED2"), new AppSettingRequiredVariable("arequired3", "AREQUIRED3")]); expect(globParser.parseFiles).toHaveBeenCalledWith(["aglobpattern"]); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "apath/afile1.json"); expect(readerWriterFactory.createReadWriter).toHaveBeenCalledWith(logger, "apath/afile2.json"); - expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets.sets)); + expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.foundSettingsFiles(sets._sets)); }); }); @@ -164,11 +174,11 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); const result = sets.verifyConfiguration({}, {}); - expect(result).toBe(true) + expect(result).toBe(true); expect(logger.info).not.toHaveBeenCalledWith(ConfigurationSetsMessages.startVerification()); }); @@ -178,14 +188,14 @@ describe('ConfigurationSets', () => { parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), }; const readerWriter: jest.Mocked = { - getVariables: jest.fn((_path)=> Promise.resolve({variables: ["aname"], requiredVariables: []})), + getVariables: jest.fn(_path => Promise.resolve({variables: ["aname"], requiredVariables: []})), substitute: jest.fn(), }; const readerWriterFactory: jest.Mocked = { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); const result = sets.verifyConfiguration({}, {}); @@ -200,14 +210,17 @@ describe('ConfigurationSets', () => { parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), }; const readerWriter: jest.Mocked = { - getVariables: jest.fn((_path)=> Promise.resolve({variables: ["aname"], requiredVariables: [new AppSettingRequiredVariable("aname", "ANAME")]})), + getVariables: jest.fn(_path => Promise.resolve({ + variables: ["aname"], + requiredVariables: [new AppSettingRequiredVariable("aname", "ANAME")] + })), substitute: jest.fn(), }; const readerWriterFactory: jest.Mocked = { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); const result = sets.verifyConfiguration({}, {}); @@ -216,6 +229,137 @@ describe('ConfigurationSets', () => { }); }); + describe('verifyAdditionalVariables', () => { + it('should not warn when no GitHub variables', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({variables: ["aname"], requiredVariables: []})), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({}, {}); + + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should not warn when additional GitHub variables, but option disabled', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({variables: [], requiredVariables: []})), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(false, undefined, undefined); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({"ANAME": "avalue"}, {}); + + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should warn when additional GitHub variables, and option enabled', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({variables: [], requiredVariables: []})), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(true, undefined, undefined); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({"ANAME": "avalue"}, {}); + + expect(logger.warning).toHaveBeenCalledWith(ConfigurationSetsMessages.additionalVariablesUnused(["ANAME"])); + }); + + it('should not warn when no additional GitHub variables, and option enabled', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({variables: ["aname"], requiredVariables: []})), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(true, undefined, undefined); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({"ANAME": "avalue"}, {}); + + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should warn when additional GitHub variables, and option enabled, but not ignored', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({ + variables: ["aname1", "aname1"], + requiredVariables: [] + })), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(true, "^apattern", undefined); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({"ANAME2": "avalue"}, {}); + + expect(logger.warning).toHaveBeenCalledWith(ConfigurationSetsMessages.additionalVariablesUnused(["ANAME2"])); + }); + + it('should not warn when additional GitHub variables, and option enabled, and ignored', async () => { + + const globParser: jest.Mocked = { + parseFiles: jest.fn(_matches => Promise.resolve(["apath/afile1.json"])), + }; + const readerWriter: jest.Mocked = { + getVariables: jest.fn(_path => Promise.resolve({variables: [], requiredVariables: []})), + substitute: jest.fn(), + }; + const readerWriterFactory: jest.Mocked = { + createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) + }; + const options = new WarningOptions(true, "^ANAME", undefined); + + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', options); + + sets.verifyAdditionalVariables({"ANAME": "avalue"}, {}); + + expect(logger.warning).not.toHaveBeenCalled(); + }); + }); + describe('substituteVariables', () => { it('should return true, when there are no sets', async () => { @@ -230,11 +374,11 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); - const result = await sets.substituteVariables({"avariable":"avalue1"}, {"asecret":"avalue2"}); + const result = await sets.substituteVariables({"avariable": "avalue1"}, {"asecret": "avalue2"}); - expect(result).toBe(true) + expect(result).toBe(true); expect(readerWriter.substitute).not.toHaveBeenCalled(); expect(logger.info).not.toHaveBeenCalledWith(ConfigurationSetsMessages.startSubstitution()); }); @@ -252,12 +396,12 @@ describe('ConfigurationSets', () => { createReadWriter: jest.fn((_logger, _filePath) => Promise.resolve(readerWriter)) }; - const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern'); + const sets = await ConfigurationSets.create(logger, globParser, readerWriterFactory, 'aglobpattern', new WarningOptions()); const result = await sets.substituteVariables({}, {}); expect(result).toBe(true); - expect(readerWriter.substitute).toHaveBeenCalledWith({}, {}); + expect(readerWriter.substitute).toHaveBeenCalledWith(new WarningOptions(), {}, {}); expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.startSubstitution()); expect(logger.info).toHaveBeenCalledWith(ConfigurationSetsMessages.substitutionSucceeded()); }); diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.ts b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.ts index 9355a311..deb6e45c 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/configurationSets.ts @@ -4,31 +4,35 @@ import {IGlobPatternParser} from "./globPatternParser"; import {ISettingsFile, SettingsFile} from "./settingsFile"; import {IAppSettingsReaderWriterFactory} from "./appSettingsReaderWriterFactory"; import {ConfigurationSet, IConfigurationSet} from "./configurationSet"; +import {WarningOptions} from "./main"; +import {GitHubVariables} from "./settingsFileProcessor"; export class ConfigurationSets { - sets: IConfigurationSet[] = []; - private readonly logger: ILogger; - - private constructor(logger: ILogger, sets: IConfigurationSet[]) { - this.sets = sets; - this.logger = logger; + _sets: IConfigurationSet[] = []; + private readonly _logger: ILogger; + private readonly _warningOptions: WarningOptions; + + private constructor(logger: ILogger, sets: IConfigurationSet[], warningOptions: WarningOptions) { + this._sets = sets; + this._logger = logger; + this._warningOptions = warningOptions; } get hasNone(): boolean { - return this.sets.length === 0; + return this._sets.length === 0; } get length(): number { - return this.sets.length; + return this._sets.length; } - public static async create(logger: ILogger, globParser: IGlobPatternParser, readerWriterFactory: IAppSettingsReaderWriterFactory, globPattern: string): Promise { + public static async create(logger: ILogger, globParser: IGlobPatternParser, readerWriterFactory: IAppSettingsReaderWriterFactory, globPattern: string, warningOptions: WarningOptions): Promise { const matches = globPattern.length > 0 ? globPattern.split(',') : []; const files = await globParser.parseFiles(matches); if (files.length === 0) { logger.warning(ConfigurationSetsMessages.noSettingsFound(globPattern)); - return new ConfigurationSets(logger, []); + return new ConfigurationSets(logger, [], warningOptions); } const sets: ConfigurationSet[] = []; @@ -42,7 +46,7 @@ export class ConfigurationSets { logger.info(ConfigurationSetsMessages.foundSettingsFiles(sets)); - return new ConfigurationSets(logger, sets); + return new ConfigurationSets(logger, sets, warningOptions); } private static async accumulateFilesIntoSets(logger: ILogger, readerWriterFactory: IAppSettingsReaderWriterFactory, sets: ConfigurationSet[], file: string) { @@ -62,46 +66,71 @@ export class ConfigurationSets { } verifyConfiguration(gitHubVariables: any, gitHubSecrets: any): boolean { - if (this.sets.length === 0) { + if (this._sets.length === 0) { return true; } - this.logger.info(ConfigurationSetsMessages.startVerification()); + this._logger.info(ConfigurationSetsMessages.startVerification()); let isAllSetsVerified = true; - for (const set of this.sets) { - const isSetVerified = set.verify(this.logger, gitHubVariables, gitHubSecrets); + for (const set of this._sets) { + const isSetVerified = set.verify(this._logger, gitHubVariables, gitHubSecrets); if (!isSetVerified) { isAllSetsVerified = false; } } if (!isAllSetsVerified) { - this.logger.error(ConfigurationSetsMessages.verificationFailed()); + this._logger.error(ConfigurationSetsMessages.verificationFailed()); } else { - this.logger.info(ConfigurationSetsMessages.verificationSucceeded()); + this._logger.info(ConfigurationSetsMessages.verificationSucceeded()); } + this.verifyAdditionalVariables(gitHubVariables, gitHubSecrets); + return isAllSetsVerified; } + public verifyAdditionalVariables(gitHubVariables: any, gitHubSecrets: any) { + + if (this._warningOptions.warnOnAdditionalVariables) { + const allSeenVariables = [...new Set(this._sets.map(set => set.definedVariables.map(name => GitHubVariables.calculateVariableOrSecretName(name))).flat())]; + const allAvailableVariables = [...new Set(Object.keys(gitHubVariables).concat(Object.keys(gitHubSecrets)))]; + + let additionalAvailableVariables = allAvailableVariables + .filter(_variable => !allSeenVariables.includes('github_token')) + .filter(variable => !allSeenVariables.includes(variable)); + if (this._warningOptions.ignoreAdditionalVariableExpression.length > 0) { + additionalAvailableVariables = additionalAvailableVariables + .filter(variable => { + const match = new RegExp(this._warningOptions.ignoreAdditionalVariableExpression).exec(variable); + return match == null + }); + } + + if (additionalAvailableVariables.length > 0) { + this._logger.warning(ConfigurationSetsMessages.additionalVariablesUnused(additionalAvailableVariables)); + } + } + } + async substituteVariables(gitHubVariables: any, gitHubSecrets: any): Promise { - if (this.sets.length === 0) { + if (this._sets.length === 0) { return true; } - this.logger.info(ConfigurationSetsMessages.startSubstitution()); + this._logger.info(ConfigurationSetsMessages.startSubstitution()); let isAllSetsSubstituted = true; - for (const set of this.sets) { - const isSetSubstituted = await set.substitute(this.logger, gitHubVariables, gitHubSecrets); + for (const set of this._sets) { + const isSetSubstituted = await set.substitute(this._logger, this._warningOptions, gitHubVariables, gitHubSecrets); if (!isSetSubstituted) { isAllSetsSubstituted = false; } } if (!isAllSetsSubstituted) { - this.logger.error(ConfigurationSetsMessages.substitutionFailed()); + this._logger.error(ConfigurationSetsMessages.substitutionFailed()); } else { - this.logger.info(ConfigurationSetsMessages.substitutionSucceeded()); + this._logger.info(ConfigurationSetsMessages.substitutionSucceeded()); } return isAllSetsSubstituted; @@ -121,4 +150,9 @@ export class ConfigurationSetsMessages { public static startSubstitution = (): string => "Substituting all settings in all settings files, in all hosts"; public static substitutionFailed = (): string => "Substitution of all settings in all settings files, in all hosts: -> Failed! See errors above"; public static substitutionSucceeded = (): string => "Substitution of all settings in all settings files, in all hosts: -> Successful!"; + public static additionalVariablesUnused = (additionalVariables: string[]): string => { + const count = additionalVariables.length; + const variables = additionalVariables.join(', \n\t'); + return `The following '${count}' GitHub variables or secrets have been defined in this repository, but they will not to be substituted in any of the settings files, in any of the hosts:\n${variables}`; + }; } \ No newline at end of file diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.spec.ts b/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.spec.ts index af77b15b..543e616b 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.spec.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.spec.ts @@ -2,6 +2,7 @@ import {ILogger} from "./logger"; import {AppSettingRequiredVariable, SettingsFileProcessorMessages} from "./settingsFileProcessor"; import {IFileReaderWriter} from "./fileReaderWriter"; import {FlatFileProcessor} from "./flatFileProcessor"; +import {WarningOptions} from "./main"; describe('getVariables', () => { const logger: jest.Mocked = { @@ -107,7 +108,7 @@ describe('getVariables', () => { expect(result.variables).toEqual(["aname1", "aname2", "aname3"]); expect(result.requiredVariables).toEqual([new AppSettingRequiredVariable("aname2", "ANAME2")]); }); - + it('should return variables with required variables, when file is has many substitutions', async () => { const readerWriter: jest.Mocked = { @@ -138,7 +139,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {}); + const result = await processor.substitute(new WarningOptions(), {}, {}); expect(result).toEqual(true); expect(readerWriter.readSettingsFile).not.toHaveBeenCalled(); @@ -153,7 +154,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"aname": "avalue"}, {}); + const result = await processor.substitute(new WarningOptions(), {"aname": "avalue"}, {}); expect(result).toEqual(false); expect(readerWriter.readSettingsFile).toHaveBeenCalledWith("apath"); @@ -170,7 +171,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"aname": "avalue"}, {}); + const result = await processor.substitute(new WarningOptions(), {"aname": "avalue"}, {}); expect(result).toEqual(false); expect(readerWriter.readSettingsFile).toHaveBeenCalledWith("apath"); @@ -187,7 +188,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"ANAME": "avalue1"}, {}); + const result = await processor.substitute(new WarningOptions(), {"ANAME": "avalue1"}, {}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", "aname=avalue"); @@ -204,7 +205,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"ANAME": "avalue1"}, {}); + const result = await processor.substitute(new WarningOptions(), {"ANAME": "avalue1"}, {}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", "aname=avalue1"); @@ -220,7 +221,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({}, {"ANAME": "avalue1"}); + const result = await processor.substitute(new WarningOptions(), {}, {"ANAME": "avalue1"}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", "aname=avalue1"); @@ -236,7 +237,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"LEVEL1_LEVEL2_LEVEL3": "avalue1"}, {}); + const result = await processor.substitute(new WarningOptions(), {"LEVEL1_LEVEL2_LEVEL3": "avalue1"}, {}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", "aname=\"avalue1\""); @@ -252,7 +253,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({"ANAME": "avalue1"}, {}); + const result = await processor.substitute(new WarningOptions(), {"ANAME": "avalue1"}, {}); expect(result).toEqual(true); expect(readerWriter.writeSettingsFile).toHaveBeenCalledWith("apath", "aname=\"avalue1\""); @@ -268,7 +269,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({ + const result = await processor.substitute(new WarningOptions(), { "ANAME1": "avalue1", "ANAME2": "avalue2", "ANAME3": "avalue3", @@ -290,7 +291,7 @@ describe('substitute', () => { }; const processor = new FlatFileProcessor(logger, readerWriter, "apath"); - const result = await processor.substitute({ + const result = await processor.substitute(new WarningOptions(), { "ANAME1": "avalue1", "ANAME2": "avalue2", "ANAME3": "avalue3", diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.ts b/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.ts index 541fd332..ef57789e 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/flatFileProcessor.ts @@ -7,6 +7,7 @@ import { ISettingsFileProcessor, SettingsFileProcessorMessages } from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; export class FlatFileProcessor implements ISettingsFileProcessor { static PairMatchExpression: RegExp = /(?[\w\d_.]+)(=)(?[\w\d_\-.#{}"'()\[\]=,:;&%$@]*)/; @@ -52,7 +53,7 @@ export class FlatFileProcessor implements ISettingsFileProcessor { } } - private static substituteVariables(logger: ILogger, gitHubVariables: any, gitHubSecrets: any, text: TextValue, _prefix: string = "") { + private static substituteVariables(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any, text: TextValue, _prefix: string = "") { if (text.value.length === 0) { return; @@ -74,7 +75,7 @@ export class FlatFileProcessor implements ISettingsFileProcessor { if (valueMatch && valueMatch.groups) { const variableName = valueMatch.groups.variable; const githubVariableName = GitHubVariables.calculateVariableOrSecretName(variableName); - const gitHubSecretOrVariableValue = GitHubVariables.getVariableOrSecretValue(gitHubVariables, gitHubSecrets, githubVariableName); + const gitHubSecretOrVariableValue = GitHubVariables.getVariableOrSecretValue(logger, warningOptions, gitHubVariables, gitHubSecrets, githubVariableName); if (gitHubSecretOrVariableValue) { logger.info(SettingsFileProcessorMessages.substitutingVariable(variableName)); text.value = text.value.replace(`\#\{${variableName}\}`, gitHubSecretOrVariableValue); @@ -85,7 +86,7 @@ export class FlatFileProcessor implements ISettingsFileProcessor { } } - async substitute(gitHubVariables: any, gitHubSecrets: any): Promise { + async substitute(warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise { if (Object.keys(gitHubVariables).length === 0 && Object.keys(gitHubSecrets).length === 0) { return true; @@ -94,7 +95,7 @@ export class FlatFileProcessor implements ISettingsFileProcessor { try { const content = await this._readerWriter.readSettingsFile(this._path) as string; const textValue = new TextValue(content); //We need to alter the encapsulated value in the next function - FlatFileProcessor.substituteVariables(this._logger, gitHubVariables, gitHubSecrets, textValue); + FlatFileProcessor.substituteVariables(this._logger, warningOptions, gitHubVariables, gitHubSecrets, textValue); await this._readerWriter.writeSettingsFile(this._path, textValue.value); this._logger.info(SettingsFileProcessorMessages.substitutingSucceeded(this._path)); return true; diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/githubAction.ts b/src/Tools.GitHubActions/VariableSubstitution/src/githubAction.ts index 5d65d084..db17fa0f 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/githubAction.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/githubAction.ts @@ -5,6 +5,8 @@ export interface IGitHubAction { getRequiredWorkflowInput(paramName: string): string; + getOptionalWorkflowInput(paramName: string, defaultValue: string): string; + getRepositoryOwner(): string; getRepositoryName(): string; @@ -36,4 +38,9 @@ export class GithubAction implements IGitHubAction { getRequiredWorkflowInput(paramName: string): string { return core.getInput(paramName, {required: true}); } + + getOptionalWorkflowInput(paramName: string, defaultValue: string): string { + const value = core.getInput(paramName, {required: false}); + return value ?? defaultValue; + } } \ No newline at end of file diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/main.ts b/src/Tools.GitHubActions/VariableSubstitution/src/main.ts index 300d75b4..502942e8 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/main.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/main.ts @@ -25,9 +25,13 @@ export class Main { const projectName = this._action.getRepositoryName(); const gitHubSecrets = this._action.getGitHubSecrets(); const gitHubEnvironmentVariables = this._action.getGitHubVariables(); - this._logger.info(MainMessages.writeInputs(filesParam, authorName, projectName)); + const warnOnAdditionalVars: boolean = this._action.getOptionalWorkflowInput('warnOnAdditionalVars', "false") === "true"; + const ignoreAdditionalVars: string = this._action.getOptionalWorkflowInput('ignoreAdditionalVars', ""); + const warnOnDuplicateVars: boolean = true; + const warningOptions = new WarningOptions(warnOnAdditionalVars, ignoreAdditionalVars, warnOnDuplicateVars); + this._logger.info(MainMessages.writeInputs(filesParam, authorName, projectName, warningOptions)); - const configurationSets = await ConfigurationSets.create(this._logger, this._globParser, this._readerWriterFactory, filesParam); + const configurationSets = await ConfigurationSets.create(this._logger, this._globParser, this._readerWriterFactory, filesParam, warningOptions); if (configurationSets.hasNone) { this._logger.info(MainMessages.abortNoSettings()); return; @@ -49,8 +53,20 @@ export class Main { } } +export class WarningOptions { + public warnOnAdditionalVariables: boolean = false; + public ignoreAdditionalVariableExpression: string = ""; + public warnOnDuplicateVariables: boolean = false; + + constructor(warnOnAdditionalVariables?: boolean, ignoreAdditionalVariableExpression?: string, warnOnDuplicateVariables?: boolean) { + this.warnOnAdditionalVariables = warnOnAdditionalVariables ?? false; + this.ignoreAdditionalVariableExpression = ignoreAdditionalVariableExpression ?? ""; + this.warnOnDuplicateVariables = warnOnDuplicateVariables ?? false; + } +} + export class MainMessages { public static unknownError = () => "An unknown error occurred while processing the settings files"; - public static writeInputs = (filesParam: string, authorName: string, projectName: string) => `Scanning settings files: '${filesParam}', in GitHub project '${authorName}/${projectName}'`; + public static writeInputs = (filesParam: string, authorName: string, projectName: string, warningOptions: WarningOptions) => `Scanning settings files: '${filesParam}', in GitHub project '${authorName}/${projectName}', with options ${JSON.stringify(warningOptions)}`; public static abortNoSettings = () => 'No settings files found in this repository, skipping variable substitution altogether'; } \ No newline at end of file diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.spec.ts b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.spec.ts index a7fa55c3..2fcaca28 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.spec.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.spec.ts @@ -2,6 +2,7 @@ import {SettingsFile, SettingsFileMessages} from "./settingsFile"; import {IAppSettingsReaderWriterFactory} from "./appSettingsReaderWriterFactory"; import {ILogger} from "./logger"; import {ISettingsFileProcessor} from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; describe('create', () => { const logger: jest.Mocked = { @@ -50,10 +51,10 @@ describe('substitute', () => { const settingsFile = await SettingsFile.create(logger, readerWriterFactory, "apath"); - const result = await settingsFile.substitute(logger, {"avariable": "avalue1"}, {"asecret": "avalue2"}); + const result = await settingsFile.substitute(logger, new WarningOptions(), {"avariable": "avalue1"}, {"asecret": "avalue2"}); expect(result).toEqual(true); - expect(readerWriter.substitute).toHaveBeenCalledWith({"avariable": "avalue1"}, {"asecret": "avalue2"}); + expect(readerWriter.substitute).toHaveBeenCalledWith(new WarningOptions(), {"avariable": "avalue1"}, {"asecret": "avalue2"}); expect(logger.info).toHaveBeenCalledWith(SettingsFileMessages.substitutingStarted("apath")); }); }); diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.ts b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.ts index 31b8d557..9eb374a4 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFile.ts @@ -1,6 +1,7 @@ import {IAppSettingsReaderWriterFactory} from "./appSettingsReaderWriterFactory"; import {ILogger} from "./logger"; import {AppSettingRequiredVariable, AppSettingVariables, ISettingsFileProcessor} from "./settingsFileProcessor"; +import {WarningOptions} from "./main"; export interface ISettingsFile { readonly path: string; @@ -8,7 +9,7 @@ export interface ISettingsFile { readonly hasRequired: boolean; readonly requiredVariables: AppSettingRequiredVariable[]; - substitute(logger: ILogger, gitHubVariables: any, gitHubSecrets: any): Promise; + substitute(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise; } export class SettingsFile implements ISettingsFile { @@ -52,10 +53,10 @@ export class SettingsFile implements ISettingsFile { return new SettingsFile(readerWriter, path, variables); } - async substitute(logger: ILogger, gitHubVariables: any, gitHubSecrets: any): Promise { + async substitute(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise { logger.info(SettingsFileMessages.substitutingStarted(this.path)); - return await this._readerWriter.substitute(gitHubVariables, gitHubSecrets); + return await this._readerWriter.substitute(warningOptions, gitHubVariables, gitHubSecrets); } } diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.spec.ts b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.spec.ts new file mode 100644 index 00000000..26e01c17 --- /dev/null +++ b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.spec.ts @@ -0,0 +1,83 @@ +import {GitHubVariables, SettingsFileProcessorMessages} from "./settingsFileProcessor"; +import {ILogger} from "./logger"; +import {WarningOptions} from "./main"; + +describe('GitHubVariables', () => { + const logger: jest.Mocked = { + info: jest.fn(), + warning: jest.fn(), + error: jest.fn(), + }; + + describe('getVariableOrSecretValue', () => { + it('should return undefined if neither variable nor secret', () => { + + const result = GitHubVariables.getVariableOrSecretValue(logger, new WarningOptions(), {}, {}, 'aname'); + + expect(result).toBeUndefined(); + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should return value if variable defined', () => { + + const result = GitHubVariables.getVariableOrSecretValue(logger, new WarningOptions(), {"aname": "avalue"}, {}, 'aname'); + + expect(result).toEqual("avalue"); + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should return value if secret defined', () => { + + const result = GitHubVariables.getVariableOrSecretValue(logger, new WarningOptions(), {}, {"aname": "avalue"}, 'aname'); + + expect(result).toEqual("avalue"); + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should return secret value if both variable and secret defined, and option disabled', () => { + + const result = GitHubVariables.getVariableOrSecretValue(logger, new WarningOptions(undefined, undefined, false), {"aname": "avariablevalue"}, {"aname": "asecretvalue"}, 'aname'); + + expect(result).toEqual("asecretvalue"); + expect(logger.warning).not.toHaveBeenCalled(); + }); + + it('should return secret value if both variable and secret defined', () => { + + const result = GitHubVariables.getVariableOrSecretValue(logger, new WarningOptions(undefined, undefined, true), {"aname": "avariablevalue"}, {"aname": "asecretvalue"}, 'aname'); + + expect(result).toEqual("asecretvalue"); + expect(logger.warning).toHaveBeenCalledWith(SettingsFileProcessorMessages.ambiguousVariableAndSecret('aname')); + }); + }); + + describe('isDefined', () => { + it('should return false if neither variable nor secret', () => { + + const result = GitHubVariables.isDefined({}, {}, 'aname'); + + expect(result).toBe(false); + }); + + it('should return true if variable exists', () => { + + const result = GitHubVariables.isDefined({"aname": "avalue"}, {}, 'aname'); + + expect(result).toBe(true); + }); + + it('should return true if secret exists', () => { + + const result = GitHubVariables.isDefined({}, {"aname": "avalue"}, 'aname'); + + expect(result).toBe(true); + }); + + it('should return true if both secret and variable exists', () => { + + const result = GitHubVariables.isDefined({"aname": "avalue"}, {"aname": "avalue"}, 'aname'); + + expect(result).toBe(true); + }); + }) +}); \ No newline at end of file diff --git a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.ts b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.ts index b243f636..1030079d 100644 --- a/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.ts +++ b/src/Tools.GitHubActions/VariableSubstitution/src/settingsFileProcessor.ts @@ -1,5 +1,8 @@ +import {ILogger} from "./logger"; +import {WarningOptions} from "./main"; + export interface ISettingsFileProcessor { - substitute(gitHubVariables: any, gitHubSecrets: any): Promise; + substitute(warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any): Promise; getVariables(path: string): Promise; } @@ -26,16 +29,26 @@ export class GitHubVariables { return gitHubVariables.hasOwnProperty(gitHubVariableName) || gitHubSecrets.hasOwnProperty(gitHubVariableName); } - public static getVariableOrSecretValue(gitHubVariables: any, gitHubSecrets: any, gitHubVariableName: string): any | undefined { + public static getVariableOrSecretValue(logger: ILogger, warningOptions: WarningOptions, gitHubVariables: any, gitHubSecrets: any, gitHubVariableName: string): any | undefined { + let variable = undefined; if (gitHubVariables.hasOwnProperty(gitHubVariableName)) { - return gitHubVariables[gitHubVariableName]; + variable = gitHubVariables[gitHubVariableName]; } + + let secret = undefined; if (gitHubSecrets.hasOwnProperty(gitHubVariableName)) { - return gitHubSecrets[gitHubVariableName]; + secret = gitHubSecrets[gitHubVariableName]; + } + + if (variable && secret) { + if (warningOptions.warnOnDuplicateVariables) { + logger.warning(SettingsFileProcessorMessages.ambiguousVariableAndSecret(gitHubVariableName)); + } + return secret } - return undefined; + return variable ?? secret; } public static calculateVariableOrSecretName(fullyQualifiedVariableName: string) { @@ -55,4 +68,5 @@ export class SettingsFileProcessorMessages { const now = new Date().toISOString(); return `All keys substituted, and removed: '${now}'`; }; + public static ambiguousVariableAndSecret = (variableName: string) => `'${variableName} was found as both a variable and secret in this GitHub repository. Using the secret value, and ignoring the variable value. Delete the variable or the secret to always use the other value, in future builds`; } \ No newline at end of file