-
Notifications
You must be signed in to change notification settings - Fork 19
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
con4m v2 prep: stop using autogenerated con4m API #214
Comments
This was referenced Mar 7, 2024
ee7
added a commit
that referenced
this issue
Mar 28, 2024
To prepare for the transition to con4m v2, reduce our usage of the autogenerated getters from c4autoconf.nim. The all-encompassing object that c4autoconf.nim defines is ChalkConfig, which currently looks like: type ChalkConfig* = ref object `@@attrscope@@`*: AttrScope [...] attestationConfig*: AttestationConfig [...] extractConfig*: ExtractConfig dockerConfig*: DockerConfig execConfig*: ExecConfig loadConfig*: LoadConfig envConfig*: EnvConfig srcConfig*: SrcConfig cloudProviderConfig*: CloudProviderConfig [...] As a starting point, refactor away all accesses of ChalkConfig fields that end in Config (which are those shown above). That is, with this commit, the following command no longer returns matches: git grep 'chalkConfig\.\w*Config\.' In con4m v2, we'll be using the lookup proc: proc lookup*[T](ctx: RuntimeState, key: string): Option[T] which is easier to move to from this commit's pattern of: get[T](chalkConfig, "foo.bar") Refs: #214
ee7
added a commit
that referenced
this issue
Apr 10, 2024
Continue from 66a5bb4 ("refactor: begin to avoid autogenerated getters"), operating on the top-level fields of `ChalkConfig` that have a simple type. With this commit, running the following command in the repo root directory no longer returns any matches: git grep 'chalkConfig.get' `ChalkConfig` currently gets defined as the below, with added comments and newlines to clarify the current status. Essentially, the remaining fields are `getoptsObjs` and those of type `OrderedTableRef`. type ChalkConfig* = ref object `@@attrscope@@`*: AttrScope # The below 12 fields will be handled later getoptsObjs*: GetoptsType keyspecs*: OrderedTableRef[string, KeySpec] plugins*: OrderedTableRef[string, PluginSpec] sinks*: OrderedTableRef[string, SinkSpec] sinkConfs*: OrderedTableRef[string, SinkConfigObj] auths*: OrderedTableRef[string, AuthSpec] authConfs*: OrderedTableRef[string, AuthConfigObj] markTemplates*: OrderedTableRef[string, MarkTemplate] reportTemplates*: OrderedTableRef[string, ReportTemplate] outputConfigs*: OrderedTableRef[string, OutputConfig] reportSpecs*: OrderedTableRef[string, ReportSpec] tools*: OrderedTableRef[string, ToolInfo] # The below 8 fields were handled by 66a5bb4 attestationConfig*: AttestationConfig extractConfig*: ExtractConfig dockerConfig*: DockerConfig execConfig*: ExecConfig loadConfig*: LoadConfig envConfig*: EnvConfig srcConfig*: SrcConfig cloudProviderConfig*: CloudProviderConfig # The below 2 fields will be handled later techStackRules*: OrderedTableRef[string, TechStackRule] linguist_languages*: OrderedTableRef[string, LinguistLanguage] # The below fields are handled by this commit config_path*: seq[string] config_filename*: string valid_chalk_command_names*: seq[string] valid_chalk_commands*: seq[string] ignore_when_normalizing*: seq[string] default_command*: Option[string] selected_command*: string color*: Option[bool] log_level*: string chalk_log_level*: string virtual_chalk*: bool zip_extensions*: seq[string] pyc_extensions*: seq[string] con4m_pinpoint*: bool chalk_debug*: bool cache_fd_limit*: int publish_audit*: bool report_total_time*: bool audit_location*: string audit_file_size*: Con4mSize log_search_path*: seq[string] artifact_search_path*: seq[string] default_tmp_dir*: Option[string] always_try_to_sign*: bool inform_if_cant_sign*: bool use_transparency_log*: bool use_signing_key_backup_service*: bool signing_key_backup_service_url*: string signing_key_backup_service_auth_config_name*: string signing_key_backup_service_timeout*: Con4mDuration signing_key_location*: string ignore_patterns*: seq[string] load_external_config*: bool load_embedded_config*: bool run_sbom_tools*: bool run_sast_tools*: bool recursive*: bool docker_exe*: Option[string] chalk_contained_items*: bool show_config*: bool ktype_names*: seq[string] use_report_cache*: bool report_cache_location*: string report_cache_lock_timeout_sec*: int force_output_on_reporting_fails*: bool env_always_show*: seq[string] env_never_show*: seq[string] env_redact*: seq[string] env_default_action*: string aws_iam_role*: Option[string] skip_command_report*: bool skip_summary_report*: bool symlink_behavior*: string install_completion_script*: bool use_pager*: bool crashoverride_usage_reporting_url*: string crashoverride_workspace_id*: string use_tech_stack_detection*: bool Also move the helper procs from config.nim to run_management.nim, so the latter module can use them. The run_management module can't import the config module because config already imports run_management, and Nim doesn't support cyclic imports yet. The config module already exported run_management, so this change doesn't affect other modules. Refs: #214
This was referenced Jun 11, 2024
ee7
added a commit
that referenced
this issue
Jun 17, 2024
Continue (from past commits [1][2][3]) to prepare for the new con4m, by reducing our usage of the autogenerated types and procs. This time, remove nearly every remaining access of a chalkConfig field. The two remaining field accesses are in the `get` overloads: $ git grep --break --heading --ignore-case --context=1 'chalkConfig\.' src/run_management.nim 30:proc get*[T](chalkConfig: ChalkConfig, fqn: string): T = 31: get[T](chalkConfig.`@@attrscope@@`, fqn) 32: 33:proc getOpt*[T](chalkConfig: ChalkConfig, fqn: string): Option[T] = 34: getOpt[T](chalkConfig.`@@attrscope@@`, fqn) We'll remove those later. Refs: #214 [1] 66a5bb4, "refactor: begin to avoid autogenerated getters" [2] 4157b58, "refactor: reduce usage of autogenerated getters, part 2" [3] 855197e, "refactor(confload, subscan): remove chalkConfig field accesses"
ee7
added a commit
that referenced
this issue
Jun 18, 2024
Continue (from past commits [1][2][3][4]) to prepare for the new con4m, removing the last usage of autogenerated types: ChalkConfig itself. In particular, this commit removes from the confload module's loadLocalStructs() procedure: chalkConfig = state.attrs.loadChalkConfig() but intentionally makes minimal changes there otherwise. That is, all the addCallback(loadLocalStructs) calls still exist for now, but they now do less. After this commit it's straightforward to remove c4autoconf entirely, because there's exactly zero remaining use of autogenerated procs and types. Refs: #214 [1] 66a5bb4, "refactor: begin to avoid autogenerated getters" [2] 4157b58, "refactor: reduce usage of autogenerated getters, part 2" [3] 855197e, "refactor(confload, subscan): remove chalkConfig field accesses" [4] bad5618, "refactor: remove remaining chalkConfig field accesses"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
currently con4m autogenerates a bunch of nim code in
src/c4autoconf.nim
which we use to reference con4m config values across the codebase.we wont be autogenerating same config utils in con4m v2 https://github.com/crashappsec/con4m/tree/jtv/v2. To help to fascilitate migration to v2, we can migrate all autogenerated calls to use con4m getters/settings now which will eventual migration to v2.
for example places like
chalk/src/selfextract.nim
Line 301 in 72a92cc
will need to migrate to direct con4m getters. dont remember exact API as of now but it will need to use existing con4m getters:
https://github.com/crashappsec/con4m/blob/fad5f9719f6123d0c587c5424a8876eca410be64/files/con4m/st.nim#L207-L268
once this is refactored it should make migration to v2 a lot easier as hopefully well be able to batch replace most getters/setters to v2 API as well as change all the relevant con4m definitions in chalk
The text was updated successfully, but these errors were encountered: