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

Dump configuration in the results directory #226

Closed
wants to merge 7 commits into from

Conversation

ccronca
Copy link
Collaborator

@ccronca ccronca commented Oct 16, 2024

This pull request introduces two new methods for exporting configuration data to YAML files. The configuration is dumped after the scanners are executed since some scanners modify the configuration at runtime. Additionally, if a default configuration file exists, it is also loaded to ensure we capture all relevant data. There are three different configurations files:

  1. Original configuration: The configuration originally provided after running the scanners and loading the default configuration file.
  2. Environment-replaced configuration: This includes values that are overridden by environment variables
  3. Default-based configuration: This captures configurations that use hardcoded default values when specific entries are not found in the original configuration. Keeping this separate helps identify any missing entries in the original file. This file only includes defaults that are hardcoded in RapiDAST, while user-provided default values are captured in the original configuration

scan_error_count = scan_error_count + 1
else:
logging.info(f"scanner: '{name}' completed successfully")
try:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a try/catch block to handle any unhandled exceptions, ensuring that the configuration dump is done

@sfowl
Copy link
Collaborator

sfowl commented Oct 17, 2024

I might be missing something obvious, but it's not clear to me why we would want to dump more than one config file. Wouldn't the only version we would be interested in be the version that is used at runtime? I.e. the rendered config, including any env var substitutions

@ccronca
Copy link
Collaborator Author

ccronca commented Oct 17, 2024

I might be missing something obvious, but it's not clear to me why we would want to dump more than one config file. Wouldn't the only version we would be interested in be the version that is used at runtime? I.e. the rendered config, including any env var substitutions

The main reason is to distinguish between configuration provided by the end-user, hardcoded default values, and values from environment variables. This way, if we want to test only the user-provided configuration, we can do so. For example, If environment variables are already interpolated into the config (they would be retrieved directly from the config rather than from the environment variables), we may not be testing the exact user configuration, and the same applies to default values.

That said, I don't have a strong opinion on this approach. I also see the convenience of having everything in a single file for easier troubleshooting. Perhaps we could have a configuration file with everything interpolated, while also including the environment variables and default values separately for reference. What do you think ?

@ccronca ccronca marked this pull request as draft October 17, 2024 07:32
@ccronca ccronca self-assigned this Oct 17, 2024
@ccronca
Copy link
Collaborator Author

ccronca commented Oct 18, 2024

Superseded by: #227

@ccronca ccronca closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants