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.

Sign up for GitHub

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

Jump to bottom

fix(cli): refactor config manager with each function having a single responsibility to avoid confusion #750

Merged
merged 4 commits into from
Feb 12, 2024
Merged

fix(cli): refactor config manager with each function having a single responsibility to avoid confusion #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
248a723
fix: config reset using argv during load
leninmehedy Feb 9, 2024
21518be
fix: refactor config manager so that each function has a single respo…
leninmehedy Feb 12, 2024
1248e20
Merge branch 'main' into fix-config-reset-using-argv
leninmehedy Feb 12, 2024
0b7f24f
Merge branch 'main' into fix-config-reset-using-argv
jeromy-cannon Feb 12, 2024
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
2 changes: 2 additions & 0 deletions solo/src/commands/flags.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,5 @@ export const nodeConfigFileFlags = new Map([
settingTxt,
log4j2Xml
].map(f => [f.name, f]))

export const integerFlags = new Map([replicaCount].map(f => [f.name, f]))
227 changes: 116 additions & 111 deletions solo/src/core/config_manager.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,144 +22,157 @@ import * as flags from '../commands/flags.mjs'
import * as paths from 'path'
import * as helpers from './helpers.mjs'

/**
* ConfigManager cache command flag values so that user doesn't need to enter the same values repeatedly.
*
* For example, 'namespace' is usually remains the same across commands once it is entered, and therefore user
* doesn't need to enter it repeatedly. However, user should still be able to specify the flag explicitly for any command.
*/
export class ConfigManager {
constructor (logger, fstConfigFile = constants.SOLO_CONFIG_FILE, persistMode = true) {
constructor (logger, cachedConfigFile = constants.SOLO_CONFIG_FILE) {
if (!logger || !(logger instanceof Logger)) throw new MissingArgumentError('An instance of core/Logger is required')

if (fstConfigFile === constants.SOLO_CONFIG_FILE) {
this.fstConfigFile = fstConfigFile
} else {
if (this.verifyConfigFile(fstConfigFile)) {
this.fstConfigFile = fstConfigFile
} else {
throw new FullstackTestingError(`Invalid config file: ${fstConfigFile}`)
}
}

this.persistMode = persistMode === true
if (!cachedConfigFile) throw new MissingArgumentError('cached config file path is required')

this.logger = logger
this.config = {
flags: {},
version: '',
updatedAt: ''
}
this.cachedConfigFile = cachedConfigFile
this.reset()
}

verifyConfigFile (fstConfigFile) {
/**
* Load the cached config
*/
load () {
try {
if (fs.existsSync(fstConfigFile)) {
const configJSON = fs.readFileSync(fstConfigFile)
JSON.parse(configJSON.toString())
if (fs.existsSync(this.cachedConfigFile)) {
const configJSON = fs.readFileSync(this.cachedConfigFile)
this.config = JSON.parse(configJSON.toString())
}
return true
} catch (e) {
return false
throw new FullstackTestingError(`failed to initialize config manager: ${e.message}`, e)
}
}

persist () {
this.config.updatedAt = new Date().toISOString()
if (this.persistMode) {
let configJSON = JSON.stringify(this.config)
fs.writeFileSync(`${this.fstConfigFile}`, configJSON)
configJSON = fs.readFileSync(this.fstConfigFile)
this.config = JSON.parse(configJSON.toString())
/**
* Reset config
*/
reset () {
this.config = {
flags: {},
version: helpers.packageVersion(),
updatedAt: new Date().toISOString()
}
}

/**
* Load and cache config on disk
* Apply the command flags precedence
*
* It overwrites previous config values using argv and store in the cached config file if any value has been changed.
* It uses the below precedence for command flag values:
* 1. User input of the command flag
* 2. Cached config value of the command flag.
* 3. Default value of the command flag if the command is not 'init'.
*
* @param argv object containing various config related fields (e.g. argv)
* @param reset if we should reset old values
* @param flagList list of flags to be processed
* @returns {*} config object
* @param argv yargs.argv
* @param aliases yargv.parsed.aliases
* @return {*} updated argv
*/
load (argv = {}, reset = false, flagList = flags.allFlags) {
try {
let config = {}
let writeConfig = false
const packageJSON = helpers.loadPackageJSON()

this.logger.debug('Start: load config', { argv, cachedConfig: config })

// if config exist, then load it first
if (!reset && fs.existsSync(this.fstConfigFile)) {
const configJSON = fs.readFileSync(this.fstConfigFile)
config = JSON.parse(configJSON.toString())
this.logger.debug(`Loaded cached config from ${this.fstConfigFile}`, { cachedConfig: config })
}

if (!config.flags) {
config.flags = {}
applyPrecedence (argv, aliases) {
for (const key of Object.keys(aliases)) {
const flag = flags.allFlagsMap.get(key)
if (flag) {
if (argv[key] !== undefined) {
// argv takes precedence, nothing to do
} else if (this.hasFlag(flag)) {
argv[key] = this.getFlag(flag)
} else if (argv._[0] !== 'init') {
argv[key] = flag.definition.defaultValue
}
}
}

// we always use packageJSON version as the version, so overwrite.
config.version = packageJSON.version
return argv
}

// extract flags from argv
if (argv && Object.keys(argv).length > 0) {
for (const flag of flagList) {
if (flag.name === flags.force.name) {
continue // we don't want to cache force flag
}
/**
* Update the config using the argv
*
* @param argv list of yargs argv
* @param persist
*/
update (argv = {}, persist = false) {
if (argv && Object.keys(argv).length > 0) {
for (const flag of flags.allFlags) {
if (flag.name === flags.force.name) {
continue // we don't want to cache force flag
}

if (argv[flag.name] === '' &&
[flags.namespace.name, flags.clusterName.name, flags.chartDirectory.name].includes(flag.name)) {
continue // don't cache empty namespace, clusterName, or chartDirectory
}
if (argv[flag.name] === '' &&
[flags.namespace.name, flags.clusterName.name, flags.chartDirectory.name].includes(flag.name)) {
continue // don't cache empty namespace, clusterName, or chartDirectory
}

if (argv[flag.name] !== undefined) {
let val = argv[flag.name]
switch (flag.definition.type) {
case 'string':
if (val) {
if (flag.name === flags.chartDirectory.name || flag.name === flags.cacheDir.name) {
this.logger.debug(`Resolving directory path for '${flag.name}': ${val}`)
val = paths.resolve(val)
}
this.logger.debug(`Setting flag '${flag.name}' of type '${flag.definition.type}': ${val}`)
config.flags[flag.name] = val
writeConfig = true
if (argv[flag.name] !== undefined) {
let val = argv[flag.name]
switch (flag.definition.type) {
case 'string':
if (flag.name === flags.chartDirectory.name || flag.name === flags.cacheDir.name) {
this.logger.debug(`Resolving directory path for '${flag.name}': ${val}`)
val = paths.resolve(val)
}
this.logger.debug(`Setting flag '${flag.name}' of type '${flag.definition.type}': ${val}`)
this.config.flags[flag.name] = `${val}` // force convert to string
break

case 'number':
this.logger.debug(`Setting flag '${flag.name}' of type '${flag.definition.type}': ${val}`)
try {
if (flags.integerFlags.has(flag.name)) {
this.config.flags[flag.name] = Number.parseInt(val)
} else {
this.config.flags[flag.name] = Number.parseFloat(val)
}
break

case 'number':
case 'boolean':
this.logger.debug(`Setting flag '${flag.name}' of type '${flag.definition.type}': ${val}`)
config.flags[flag.name] = val
writeConfig = true
break

default:
throw new FullstackTestingError(`Unsupported field type for flag '${flag.name}': ${flag.definition.type}`)
}
} catch (e) {
throw new FullstackTestingError(`invalid number value '${val}': ${e.message}`, e)
}
break

case 'boolean':
this.logger.debug(`Setting flag '${flag.name}' of type '${flag.definition.type}': ${val}`)
this.config.flags[flag.name] = (val === true) || (val === 'true') // use comparison to enforce boolean value
break

default:
throw new FullstackTestingError(`Unsupported field type for flag '${flag.name}': ${flag.definition.type}`)
}
}
}

// store last command that was run
if (argv._) {
config.lastCommand = argv._
}
// store last command that was run
if (argv._) {
this.config.lastCommand = argv._
}

// store CLI config
this.config = config
if (reset || writeConfig) {
this.config.updatedAt = new Date().toISOString()

if (persist) {
this.persist()
}
}
}

this.logger.debug('Finish: load config', { argv, cachedConfig: config })

// set dev mode for logger if necessary
this.logger.setDevMode(this.getFlag(flags.devMode))
/**
* Persist the config in the cached config file
*/
persist () {
try {
this.config.updatedAt = new Date().toISOString()
let configJSON = JSON.stringify(this.config)
fs.writeFileSync(`${this.cachedConfigFile}`, configJSON)

return this.config
// refresh config with the file contents
configJSON = fs.readFileSync(this.cachedConfigFile)
this.config = JSON.parse(configJSON.toString())
} catch (e) {
throw new FullstackTestingError(`failed to load config: ${e.message}`, e)
throw new FullstackTestingError(`failed to persis config: ${e.message}`, e)
}
}

Expand Down Expand Up @@ -212,12 +225,4 @@ export class ConfigManager {
getUpdatedAt () {
return this.config.updatedAt
}

/**
* Get last command
* @return {*}
*/
getLastCommand () {
return this.config.lastCommand
}
}
30 changes: 8 additions & 22 deletions solo/src/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,37 +65,23 @@ export function main (argv) {

const processArguments = (argv, yargs) => {
if (argv._[0] === 'init') {
configManager.load({}, true) // reset cached config
configManager.reset()
} else {
configManager.load()
}

// load cluster name and namespace from kubernetes context
// Set default cluster name and namespace from kubernetes context
// these will be overwritten if user has entered the flag values explicitly
configManager.setFlag(flags.clusterName, cluster.name)
if (context.namespace) {
// this will be overwritten if user has passed --namespace flag
configManager.setFlag(flags.namespace, context.namespace)
}

for (const key of Object.keys(yargs.parsed.aliases)) {
const flag = flags.allFlagsMap.get(key)
if (flag) {
if (argv[key] !== undefined) {
// argv takes precedence, nothing to do
} else if (configManager.hasFlag(flag)) {
argv[key] = configManager.getFlag(flag)
} else if (argv._[0] !== 'init') {
argv[key] = flag.definition.defaultValue
}
}
}
// apply precedence for flags
argv = configManager.applyPrecedence(argv, yargs.parsed.aliases)

// Update config manager and persist the config.
// Note: Because of this centralized loading, we really don't need to load argv in configManager later in
// the command execution handlers. However, we are loading argv again in the command handlers to facilitate testing
// with argv injection into the command handlers.
configManager.load(argv)
configManager.persist()
// update and persist config
configManager.update(argv, true)

logger.showUser(chalk.cyan('\n******************************* Solo *********************************************'))
logger.showUser(chalk.cyan('Version\t\t\t:'), chalk.yellow(configManager.getVersion()))
Expand All @@ -116,7 +102,7 @@ export function main (argv) {
.option(flags.devMode.name, flags.devMode.definition)
.wrap(120)
.demand(1, 'Select a command')
.middleware(processArguments, true)
.middleware(processArguments, false) // applyBeforeValidate = false as otherwise middleware is called twice
.parse()
} catch (e) {
logger.showUserError(e)
Expand Down
Loading
Loading