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.

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

DXCDT-384: Keyword preservation in auxiliary files #758

Merged
merged 37 commits into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2eb7e09
Renaming load to loadAssetsFromLocal
willvedd Feb 21, 2023
74d55d1
Renaming load to loadAssetsFromAuth0
willvedd Feb 21, 2023
93aae5a
Merge branch 'master' into DXCDT-375-rename-load-to-loadAssetsFromLocal
willvedd Feb 21, 2023
95b4bed
Merge branch 'master' into DXCDT-375-rename-load-to-loadAssetsFromLocal
willvedd Feb 21, 2023
3c82769
Merge branch 'DXCDT-375-rename-load-to-loadAssetsFromLocal' into DXCD…
willvedd Feb 21, 2023
90f8758
integrating into export process
willvedd Feb 22, 2023
4bea2cd
Adding config interpreter
willvedd Feb 22, 2023
8fc8498
Disabling keyword replacement in certain cases
willvedd Feb 22, 2023
e8c7b98
Resolving conflicts with master
willvedd Feb 22, 2023
df66ab0
Fixing directory load
willvedd Feb 22, 2023
4db97a1
More forgiving lookup logic if properties and/or addresses do not exist
willvedd Feb 22, 2023
f1f6a40
Fixing directory load again
willvedd Feb 22, 2023
6149064
Merge branch 'DXCDT-383-integrate-preserve-keywords-function-into-exp…
willvedd Feb 22, 2023
f5c7596
Adding case that would have previously tripped up process
willvedd Feb 22, 2023
a6153f0
Adding e2e tests for keyword preservation
willvedd Feb 22, 2023
ce3eba0
Removing old tests, updating recordings, removing console. logs
willvedd Feb 22, 2023
39ab7a7
Commenting-out test to fix later on
willvedd Feb 22, 2023
1b9a065
Fixing workdirectory for e2e tests
willvedd Feb 22, 2023
2212901
Adding eventual cases for auxillary files
willvedd Feb 22, 2023
32a7da8
Adding TODO
willvedd Feb 22, 2023
7387b88
Adding preqrequisite checks
willvedd Feb 23, 2023
a88ca39
Merging with master
willvedd Feb 23, 2023
8e7e7cd
Fixing test
willvedd Feb 23, 2023
771dd53
Standardizing definitions of resource identifiers
willvedd Feb 28, 2023
608698c
Removing incompatible id from email templates
willvedd Feb 28, 2023
517d471
Readding identifiers field for resource server
willvedd Feb 28, 2023
8b4368b
Fixing email templates identifier field
willvedd Feb 28, 2023
e83257a
Merge branch 'master' of https://github.com/auth0/auth0-deploy-cli in…
willvedd Feb 28, 2023
ce74a1b
Reformulating arguments into object
willvedd Feb 28, 2023
2a4ee58
Adding e2e case
willvedd Feb 28, 2023
0759f6e
Init
willvedd Feb 28, 2023
62e2267
Removing console log
willvedd Feb 28, 2023
fd7f896
Adding more robust e2e test
willvedd Feb 28, 2023
20e26a2
Updating recordings
willvedd Feb 28, 2023
72c4b7d
Fixing database directory handler, test
willvedd Mar 1, 2023
cb68fe5
Adding unit test cases for not replacing keywords
willvedd Mar 1, 2023
c397bbe
Merging with master
willvedd Mar 1, 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
5 changes: 4 additions & 1 deletion src/context/directory/handlers/branding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ function parse(context: DirectoryContext): ParsedBranding {
});
definition.body = loadFileAndReplaceKeywords(
path.join(brandingTemplatesFolder, definition.body),
context.mappings
{
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}
Comment on lines +43 to +46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the diffs in this PR are due to the changing function signature of loadFileAndReplaceKeywords which adds an option to disable keyword replacement.

);
return definition;
}, {});
Expand Down
5 changes: 4 additions & 1 deletion src/context/directory/handlers/clients.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ function parse(context: DirectoryContext): ParsedClients {
const htmlFileName = path.join(clientsFolder, client.custom_login_page);

if (isFile(htmlFileName)) {
client.custom_login_page = loadFileAndReplaceKeywords(htmlFileName, context.mappings);
client.custom_login_page = loadFileAndReplaceKeywords(htmlFileName, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
});
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/context/directory/handlers/connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ function parse(context: DirectoryContext): ParsedConnections {
`Passwordless email template purportedly located at ${htmlFileName} does not exist for connection. Ensure the existence of this file to proceed with deployment.`
);
}
connection.options.email.body = loadFileAndReplaceKeywords(htmlFileName, context.mappings);
connection.options.email.body = loadFileAndReplaceKeywords(htmlFileName, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
});
}

return connection;
Expand Down
21 changes: 14 additions & 7 deletions src/context/directory/handlers/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {

import { DirectoryHandler } from '.';
import DirectoryContext from '..';
import { Asset, ParsedAsset } from '../../../types';
import { Asset, KeywordMappings, ParsedAsset } from '../../../types';

type ParsedDatabases = ParsedAsset<'databases', Asset[]>;

Expand All @@ -31,12 +31,15 @@ type DatabaseMetadata = {
};
};

function getDatabase(folder: string, mappings): {} {
function getDatabase(
folder: string,
mappingOpts: { mappings: KeywordMappings; disableKeywordReplacement: boolean }
): {} {
const metaFile = path.join(folder, 'database.json');

const metaData: DatabaseMetadata | {} = (() => {
try {
return loadJSON(metaFile, mappings);
return loadJSON(metaFile, mappingOpts);
} catch (err) {
log.warn(`Skipping database folder ${folder} as cannot find or read ${metaFile}`);
return {};
Expand All @@ -60,15 +63,14 @@ function getDatabase(folder: string, mappings): {} {

// If any customScripts configured then load content of files
if (database.options.customScripts) {
Object.entries(database.options.customScripts).forEach(([name, script]) => {
Object.entries(database.options.customScripts).forEach(([name, script]: [string, string]) => {
if (!constants.DATABASE_SCRIPTS.includes(name)) {
// skip invalid keys in customScripts object
log.warn('Skipping invalid database configuration: ' + name);
} else {
database.options.customScripts[name] = loadFileAndReplaceKeywords(
//@ts-ignore
path.join(folder, script),
mappings
mappingOpts
);
}
});
Expand All @@ -87,7 +89,12 @@ function parse(context: DirectoryContext): ParsedDatabases {
.filter((f) => isDirectory(f));

const databases = folders
.map((f) => getDatabase(f, context.mappings))
.map((f) =>
getDatabase(f, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
})
)
.filter((p) => Object.keys(p).length > 1);

return {
Expand Down
5 changes: 4 additions & 1 deletion src/context/directory/handlers/emailTemplates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ function parse(context: DirectoryContext): ParsedEmailTemplates {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}),
body: loadFileAndReplaceKeywords(html, context.mappings),
body: loadFileAndReplaceKeywords(html, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}),
};
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/context/directory/handlers/pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ function parse(context: DirectoryContext): ParsedPages {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}),
html: loadFileAndReplaceKeywords(html, context.mappings),
html: loadFileAndReplaceKeywords(html, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}),
};
});

Expand Down
5 changes: 4 additions & 1 deletion src/context/directory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export default class DirectoryContext {
// try load not relative to yaml file
toLoad = f;
}
return loadFileAndReplaceKeywords(toLoad, this.mappings);
return loadFileAndReplaceKeywords(toLoad, {
mappings: this.mappings,
disableKeywordReplacement: this.disableKeywordReplacement,
});
}

async loadAssetsFromLocal(opts = { disableKeywordReplacement: false }): Promise<void> {
Expand Down
5 changes: 4 additions & 1 deletion src/context/yaml/handlers/branding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ async function parse(context: YAMLContext): Promise<ParsedBranding> {
const markupFile = path.join(context.basePath, templateDefinition.body);
return {
template: templateDefinition.template,
body: loadFileAndReplaceKeywords(markupFile, context.mappings),
body: loadFileAndReplaceKeywords(markupFile, {
mappings: context.mappings,
disableKeywordReplacement: context.disableKeywordReplacement,
}),
};
}
);
Expand Down
8 changes: 7 additions & 1 deletion src/context/yaml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ export default class YAMLContext {
mappings: KeywordMappings;
mgmtClient: Auth0APIClient;
assets: Assets;
disableKeywordReplacement: boolean;

constructor(config: Config, mgmtClient) {
this.configFile = config.AUTH0_INPUT_FILE;
this.config = config;
this.mappings = config.AUTH0_KEYWORD_REPLACE_MAPPINGS || {};
this.mgmtClient = mgmtClient;
this.disableKeywordReplacement = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default, keyword replacement is enabled (not disabled). This configuration value is only enabled during the keyword preservation stage.


//@ts-ignore because the assets property gets filled out throughout
this.assets = {};
Expand All @@ -50,11 +52,15 @@ export default class YAMLContext {
// try load not relative to yaml file
toLoad = f;
}
return loadFileAndReplaceKeywords(path.resolve(toLoad), this.mappings);
return loadFileAndReplaceKeywords(path.resolve(toLoad), {
mappings: this.mappings,
disableKeywordReplacement: this.disableKeywordReplacement,
});
}

async loadAssetsFromLocal(opts = { disableKeywordReplacement: false }) {
// Allow to send object/json directly
this.disableKeywordReplacement = opts.disableKeywordReplacement;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only location where disableKeywordReplacement is set to true.

if (typeof this.configFile === 'object') {
this.assets = this.configFile;
} else {
Expand Down
10 changes: 8 additions & 2 deletions src/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,18 @@ export function convertClientNamesToIds(names: string[], clients: Asset[]): stri
return [...unresolved, ...result];
}

export function loadFileAndReplaceKeywords(file: string, mappings: KeywordMappings): string {
export function loadFileAndReplaceKeywords(
file: string,
{
mappings,
disableKeywordReplacement = false,
}: { mappings: KeywordMappings; disableKeywordReplacement: boolean }
): string {
// Load file and replace keyword mappings
const f = path.resolve(file);
try {
fs.accessSync(f, fsConstants.F_OK);
if (mappings) {
if (mappings && !disableKeywordReplacement) {
return keywordReplace(fs.readFileSync(f, 'utf8'), mappings);
}
return fs.readFileSync(f, 'utf8');
Expand Down
8 changes: 4 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export function loadJSON(
}
): any {
try {
const content = loadFileAndReplaceKeywords(
file,
opts.disableKeywordReplacement ? {} : opts.mappings
);
const content = loadFileAndReplaceKeywords(file, {
mappings: opts.mappings,
disableKeywordReplacement: opts.disableKeywordReplacement,
});
return JSON.parse(content);
} catch (e) {
throw new Error(`Error parsing JSON from metadata file: ${file}, because: ${e.message}`);
Expand Down
73 changes: 64 additions & 9 deletions test/e2e/e2e.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from 'chai';
import path from 'path';
import fs from 'fs';
import { copySync } from 'fs-extra';
import { copySync, emptyDirSync } from 'fs-extra';
import { getFiles, existsMustBeDir } from '../../src/utils';
import { load as yamlLoad } from 'js-yaml';
import { setupRecording, testNameToWorkingDirectory } from './e2e-utils';
Expand Down Expand Up @@ -384,8 +384,32 @@ describe('keyword preservation', () => {
config,
});

//Dumping without keyword preservation so we can assert that remote values will overwrite local values
await dump({
output_folder: workDirectory,
format: 'yaml',
config: {
...config,
AUTH0_PRESERVE_KEYWORDS: false,
},
});
const yamlWithoutPreservation = yamlLoad(
fs.readFileSync(path.join(workDirectory, 'tenant.yaml'))
);
expect(yamlWithoutPreservation.tenant.friendly_name).to.equal(
'This tenant name should be preserved'
);
expect(yamlWithoutPreservation.tenant.support_email).to.equal('support@travel0.com');
expect(yamlWithoutPreservation.tenant.support_url).to.equal('https://travel0.com/support');
expect(
yamlWithoutPreservation.emailTemplates.find(({ template }) => template === 'welcome_email')
.resultUrl
).to.equal('https://travel0.com/welcome');

emptyDirSync(workDirectory);
copySync(`${__dirname}/testdata/should-preserve-keywords/yaml`, workDirectory); //It is necessary to copy directory contents to work directory to prevent overwriting of Git-committed files

//This dump will attempt to preserve keywords
await dump({
output_folder: workDirectory,
format: 'yaml',
Expand All @@ -402,10 +426,10 @@ describe('keyword preservation', () => {

// expect(yaml.tenant.enabled_locales).to.equal('@@LANGUAGES@@'); TODO: enable @@ARRAY@@ keyword preservation in yaml formats

// const emailTemplateHTML = fs
// .readFileSync(path.join(workDirectory, 'emailTemplates', 'welcome_email.html'))
// .toString();
// expect(emailTemplateHTML).to.contain('##TENANT##'); TODO: enable keyword preservation in auxillary template files
const emailTemplateHTML = fs
.readFileSync(path.join(workDirectory, 'emailTemplates', 'welcome_email.html'))
.toString();
expect(emailTemplateHTML).to.contain('##TENANT_NAME##');

recordingDone();
});
Expand All @@ -420,8 +444,39 @@ describe('keyword preservation', () => {
config,
});

//Dumping without keyword preservation so we can assert that remote values will overwrite local values
await dump({
output_folder: workDirectory,
format: 'directory',
config: {
...config,
AUTH0_PRESERVE_KEYWORDS: false,
},
});

const jsonWithoutPreservation = JSON.parse(
fs.readFileSync(path.join(workDirectory, 'tenant.json')).toString()
);

expect(jsonWithoutPreservation.friendly_name).to.equal('This tenant name should be preserved');
expect(jsonWithoutPreservation.enabled_locales).to.deep.equal(['en', 'es']);
expect(jsonWithoutPreservation.support_email).to.equal('support@travel0.com');
expect(jsonWithoutPreservation.support_url).to.equal('https://travel0.com/support');

const emailTemplateJsonWithoutPreservation = JSON.parse(
fs.readFileSync(path.join(workDirectory, 'emails', 'welcome_email.json')).toString()
);

expect(emailTemplateJsonWithoutPreservation.resultUrl).to.equal('https://travel0.com/welcome');

expect(
fs.readFileSync(path.join(workDirectory, 'emails', 'welcome_email.html')).toString()
).to.contain('This tenant name should be preserved');

emptyDirSync(workDirectory);
Comment on lines +447 to +476
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These new E2E assertions are to ensure that keyword replacement worked when keyword preservation disabled. Otherwise, false positives could occur during testing.

copySync(`${__dirname}/testdata/should-preserve-keywords/directory`, workDirectory); //It is necessary to copy directory contents to work directory to prevent overwriting of Git-committed files

//This dump will attempt to preserve keywords
await dump({
output_folder: workDirectory,
format: 'directory',
Expand All @@ -442,10 +497,10 @@ describe('keyword preservation', () => {
expect(emailTemplateJson.resultUrl).to.equal('https://##DOMAIN##/welcome');
expect(emailTemplateJson.subject).to.not.equal('##THIS_SHOULD_NOT_BE_PRESERVED##');

// const emailTemplateHTML = fs
// .readFileSync(path.join(workDirectory, 'emailTemplates', 'welcome_email.html'))
// .toString();
// expect(emailTemplateHTML).to.contain('##TENANT##'); TODO: enable keyword preservation in auxillary template files
const emailTemplateHTML = fs
.readFileSync(path.join(workDirectory, 'emailTemplates', 'welcome_email.html'))
.toString();
expect(emailTemplateHTML).to.contain('##TENANT_NAME##');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This E2E assertion proves that this functionality works 🎉


recordingDone();
});
Expand Down
Loading