-
Notifications
You must be signed in to change notification settings - Fork 15
feat: support per os settings from ldx-sync [IDE-1756] #1180
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
base: refactor/IDE-1786_folder-config-refactoring
Are you sure you want to change the base?
Changes from 2 commits
6e711c4
785c563
53c6184
1f23ea9
6818d0f
5caa3a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,14 +17,47 @@ | |
| package types | ||
|
|
||
| import ( | ||
| "runtime" | ||
| "strings" | ||
|
|
||
| "github.com/rs/zerolog/log" | ||
| v20241015 "github.com/snyk/go-application-framework/pkg/apiclients/ldx_sync_config/ldx_sync/2024-10-15" | ||
| "github.com/snyk/go-application-framework/pkg/configuration" | ||
| "github.com/snyk/go-application-framework/pkg/configuration/configresolver" | ||
|
|
||
| "github.com/snyk/snyk-ls/internal/util" | ||
| ) | ||
|
|
||
| // LDXSyncSettingKey maps our internal setting names to LDX-Sync API field names | ||
| // osSuffix is the OS-specific suffix for per-OS API fields. | ||
| var osSuffix = goosToSuffix(runtime.GOOS) | ||
|
|
||
| func goosToSuffix(goos string) string { | ||
| var suffix string | ||
| switch goos { | ||
| case "windows": | ||
| suffix = "windows" | ||
| case "linux": | ||
| suffix = "linux" | ||
| default: | ||
| suffix = "macos" | ||
| } | ||
| log.Debug().Str("goos", goos).Str("suffix", suffix).Msg("goosToSuffix - resolved OS suffix for per-OS settings") | ||
andrewrobinsonhodges-snyk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return suffix | ||
| } | ||
|
|
||
| // perOSSettings maps internal setting names to their base API field name. | ||
| // The API sends these as <base>_<os>, e.g. "cli_path_macos". | ||
| // Only the variant matching the current OS is accepted. | ||
| var perOSSettings = map[string]string{ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why make this a special case? We can just allow everything to be arch and os-specific.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would we add unnecessary complexity to the API and UIs?? I don't think end users will want different scan behavior on different OSs. |
||
| SettingCliPath: "cli_path", | ||
| SettingBinaryBaseUrl: "binary_base_url", | ||
| SettingReferenceFolder: "reference_folder", | ||
| SettingAdditionalParameters: "additional_parameters", | ||
| SettingAdditionalEnvironment: "additional_environment", | ||
| } | ||
|
|
||
| // ldxSyncSettingKeyMap maps internal setting names to LDX-Sync API field names. | ||
| // Per-OS settings are NOT in this map — they are handled via perOSSettings. | ||
| var ldxSyncSettingKeyMap = map[string]string{ | ||
| SettingApiEndpoint: "api_endpoint", | ||
| SettingCodeEndpoint: "code_endpoint", | ||
|
|
@@ -36,8 +69,6 @@ var ldxSyncSettingKeyMap = map[string]string{ | |
| SettingAutoConfigureMcpServer: "auto_configure_mcp_server", | ||
| SettingPublishSecurityAtInceptionRules: "publish_security_at_inception_rules", | ||
| SettingTrustEnabled: "trust_enabled", | ||
| SettingBinaryBaseUrl: "binary_base_url", | ||
| SettingCliPath: "cli_path", | ||
| SettingAutomaticDownload: "automatic_download", | ||
| SettingCliReleaseChannel: "cli_release_channel", | ||
| SettingEnabledSeverities: "severities", | ||
|
|
@@ -49,10 +80,7 @@ var ldxSyncSettingKeyMap = map[string]string{ | |
| SettingScanNetNew: "net_new", | ||
| SettingIssueViewOpenIssues: "open_issues", | ||
| SettingIssueViewIgnoredIssues: "ignored_issues", | ||
| SettingReferenceFolder: "reference_folder", | ||
| SettingReferenceBranch: "reference_branch", | ||
| SettingAdditionalParameters: "additional_parameters", | ||
| SettingAdditionalEnvironment: "additional_environment", | ||
| } | ||
|
|
||
| // ConvertLDXSyncResponseToOrgConfig converts a UserConfigResponse to our LDXSyncOrgConfig format | ||
|
|
@@ -195,18 +223,34 @@ func ExtractFolderSettings(response *v20241015.UserConfigResponse, remoteUrl str | |
| return result | ||
| } | ||
|
|
||
| // getInternalSettingName maps an LDX-Sync API field name to our internal setting name | ||
| // getInternalSettingName maps an LDX-Sync API field name to our internal setting name. | ||
| // For per-OS settings, only the variant matching the current OS is accepted. | ||
| func getInternalSettingName(ldxSyncKey string) string { | ||
andrewrobinsonhodges-snyk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for internal, ldx := range ldxSyncSettingKeyMap { | ||
| if ldx == ldxSyncKey { | ||
| return internal | ||
| } | ||
| } | ||
| for internal, baseName := range perOSSettings { | ||
| prefix := baseName + "_" | ||
| if ldxSyncKey == GetLDXSyncKey(internal) { | ||
andrewrobinsonhodges-snyk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| log.Debug().Str("settingName", ldxSyncKey).Str("internalName", internal).Str("osSuffix", osSuffix).Msg("getInternalSettingName - matched per-OS setting for current OS") | ||
andrewrobinsonhodges-snyk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return internal | ||
| } | ||
| if strings.HasPrefix(ldxSyncKey, prefix) && ldxSyncKey != prefix { | ||
| log.Debug().Str("settingName", ldxSyncKey).Str("osSuffix", osSuffix).Msg("getInternalSettingName - skipped per-OS setting for different OS") | ||
| return "" | ||
| } | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // GetLDXSyncKey returns the LDX-Sync API field name for an internal setting name | ||
| // GetLDXSyncKey returns the LDX-Sync API field name for an internal setting name. | ||
| // For per-OS settings, the current OS suffix is appended. | ||
| func GetLDXSyncKey(internalName string) string { | ||
| if baseName, ok := perOSSettings[internalName]; ok { | ||
| return baseName + "_" + osSuffix | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think, we're missing arch in general, which is important for the CLI path. |
||
| } | ||
| return ldxSyncSettingKeyMap[internalName] | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Darwin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that help? As long as it matches the LDX-Sync value it shouldn't matter.