Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion domain/ide/command/ldx_sync_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ func Test_RefreshConfigFromLdxSync_FolderSettingsWithURLNormalization(t *testing
Origin: v20241015.SettingMetadataOriginOrg,
Locked: util.Ptr(false),
},
"reference_folder": {
types.GetLDXSyncKey(types.SettingReferenceFolder): {
Value: "/src/main",
Origin: v20241015.SettingMetadataOriginOrg,
Locked: util.Ptr(true),
Expand Down
69 changes: 55 additions & 14 deletions internal/types/ldx_sync_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
package types

import (
"runtime"

"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"
Expand All @@ -25,7 +28,49 @@ import (
"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"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be Darwin

Copy link
Copy Markdown
Contributor Author

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.

}
log.Trace().Str("goos", goos).Str("suffix", suffix).Msg("goosToSuffix - resolved OS suffix for per-OS settings")
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{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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",
}

// ldxSyncReverseMap is the reverse of ldxSyncSettingKeyMap + perOSSettings (with OS suffix applied).
// It maps LDX-Sync API field names to internal setting names.
var ldxSyncReverseMap = func() map[string]string {
m := make(map[string]string, len(ldxSyncSettingKeyMap)+len(perOSSettings))
for internal, ldx := range ldxSyncSettingKeyMap {
m[ldx] = internal
}
for internal, baseName := range perOSSettings {
m[baseName+"_"+osSuffix] = internal
}
return m
}()

// 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",
Expand All @@ -37,8 +82,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",
Expand All @@ -50,10 +93,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.
Expand Down Expand Up @@ -196,18 +236,19 @@ 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;
// keys for other OSes are simply absent from the reverse map.
func getInternalSettingName(ldxSyncKey string) string {
Comment thread
andrewrobinsonhodges-snyk marked this conversation as resolved.
for internal, ldx := range ldxSyncSettingKeyMap {
if ldx == ldxSyncKey {
return internal
}
}
return ""
return ldxSyncReverseMap[ldxSyncKey]
}

// 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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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]
}

Expand Down
194 changes: 192 additions & 2 deletions internal/types/ldx_sync_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func TestExtractFolderSettings(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.FolderSettings = &map[string]map[string]v20241015.SettingMetadata{
normalizedURL: {
"reference_folder": {
"reference_folder_" + osSuffix: {
Value: "/src",
Origin: v20241015.SettingMetadataOriginOrg,
},
Expand All @@ -232,7 +232,7 @@ func TestExtractMachineSettings(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.Settings = &map[string]v20241015.SettingMetadata{
// Machine-scope setting
"cli_path": {
"cli_path_" + osSuffix: {
Value: "/usr/local/bin/snyk",
Origin: v20241015.SettingMetadataOriginOrg,
Locked: &locked,
Expand Down Expand Up @@ -342,6 +342,196 @@ func TestGetLDXSyncKey(t *testing.T) {
})
}

func TestExtractMachineSettings_PerOS(t *testing.T) {
fm := adapterTestFm(t)
otherOS := "windows"
if osSuffix == "windows" {
otherOS = "macos"
}

t.Run("stores current-OS cli_path and ignores other-OS", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.Settings = &map[string]v20241015.SettingMetadata{
"cli_path_" + osSuffix: {
Value: "/current/os/snyk",
Origin: v20241015.SettingMetadataOriginOrg,
Locked: util.Ptr(true),
},
"cli_path_" + otherOS: {
Value: "/other/os/snyk",
Origin: v20241015.SettingMetadataOriginOrg,
Locked: util.Ptr(true),
},
}

result := ExtractMachineSettings(response, fm)
require.NotNil(t, result)
cliField := result[SettingCliPath]
require.NotNil(t, cliField, "cli_path should be extracted for current OS")
assert.Equal(t, "/current/os/snyk", cliField.Value)
assert.True(t, cliField.IsLocked)
})

t.Run("stores current-OS binary_base_url with all three OS variants present", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.Settings = &map[string]v20241015.SettingMetadata{
"binary_base_url_macos": {
Value: "https://macos.example.com",
Origin: v20241015.SettingMetadataOriginOrg,
},
"binary_base_url_windows": {
Value: "https://windows.example.com",
Origin: v20241015.SettingMetadataOriginOrg,
},
"binary_base_url_linux": {
Value: "https://linux.example.com",
Origin: v20241015.SettingMetadataOriginOrg,
},
}

result := ExtractMachineSettings(response, fm)
require.NotNil(t, result)
field := result[SettingBinaryBaseUrl]
require.NotNil(t, field, "binary_base_url should be extracted for current OS")
assert.Equal(t, "https://"+osSuffix+".example.com", field.Value,
"should store only the value for the current OS suffix %q", osSuffix)
})

t.Run("ignores per-OS field when only other-OS variant present", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.Settings = &map[string]v20241015.SettingMetadata{
"cli_path_" + otherOS: {
Value: "/other/os/snyk",
Origin: v20241015.SettingMetadataOriginOrg,
},
}

result := ExtractMachineSettings(response, fm)
assert.Nil(t, result, "should return nil when only other-OS variant present")
})
}

func TestExtractFolderSettings_PerOS(t *testing.T) {
otherOS := "windows"
if osSuffix == "windows" {
otherOS = "macos"
}
remoteURL := "https://github.com/snyk/test-repo"

t.Run("stores current-OS reference_folder and ignores other-OS", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.FolderSettings = &map[string]map[string]v20241015.SettingMetadata{
remoteURL: {
"reference_folder_" + osSuffix: {
Value: "/current/os/folder",
Origin: v20241015.SettingMetadataOriginOrg,
Locked: util.Ptr(true),
},
"reference_folder_" + otherOS: {
Value: "/other/os/folder",
Origin: v20241015.SettingMetadataOriginOrg,
},
},
}

result := ExtractFolderSettings(response, remoteURL)
require.NotNil(t, result)
field := result[SettingReferenceFolder]
require.NotNil(t, field, "reference_folder should be extracted for current OS")
assert.Equal(t, "/current/os/folder", field.Value)
assert.True(t, field.IsLocked)
})

t.Run("stores current-OS additional_parameters with all three OS variants", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.FolderSettings = &map[string]map[string]v20241015.SettingMetadata{
remoteURL: {
"additional_parameters_macos": {
Value: "--mac-flag",
Origin: v20241015.SettingMetadataOriginOrg,
},
"additional_parameters_windows": {
Value: "--win-flag",
Origin: v20241015.SettingMetadataOriginOrg,
},
"additional_parameters_linux": {
Value: "--linux-flag",
Origin: v20241015.SettingMetadataOriginOrg,
},
},
}

result := ExtractFolderSettings(response, remoteURL)
require.NotNil(t, result)
field := result[SettingAdditionalParameters]
require.NotNil(t, field)
expected := map[string]string{"macos": "--mac-flag", "windows": "--win-flag", "linux": "--linux-flag"}
assert.Equal(t, expected[osSuffix], field.Value,
"should store only the value for the current OS suffix %q", osSuffix)
})

t.Run("ignores per-OS folder field when only other-OS variant present", func(t *testing.T) {
response := &v20241015.UserConfigResponse{}
response.Data.Attributes.FolderSettings = &map[string]map[string]v20241015.SettingMetadata{
remoteURL: {
"additional_environment_" + otherOS: {
Value: "OTHER_VAR=1",
Origin: v20241015.SettingMetadataOriginOrg,
},
},
}

result := ExtractFolderSettings(response, remoteURL)
assert.Nil(t, result, "should return nil when only other-OS variant present")
})
}

func TestGoosToSuffix(t *testing.T) {
assert.Equal(t, "macos", goosToSuffix("darwin"))
assert.Equal(t, "windows", goosToSuffix("windows"))
assert.Equal(t, "linux", goosToSuffix("linux"))
assert.Equal(t, "macos", goosToSuffix("freebsd"))
assert.Equal(t, "macos", goosToSuffix(""))
}

func TestPerOSSettingMapping(t *testing.T) {
t.Run("getInternalSettingName accepts current OS variant", func(t *testing.T) {
assert.Equal(t, SettingCliPath, getInternalSettingName("cli_path_"+osSuffix))
assert.Equal(t, SettingBinaryBaseUrl, getInternalSettingName("binary_base_url_"+osSuffix))
assert.Equal(t, SettingReferenceFolder, getInternalSettingName("reference_folder_"+osSuffix))
assert.Equal(t, SettingAdditionalParameters, getInternalSettingName("additional_parameters_"+osSuffix))
assert.Equal(t, SettingAdditionalEnvironment, getInternalSettingName("additional_environment_"+osSuffix))
})

t.Run("getInternalSettingName rejects other OS variants", func(t *testing.T) {
otherOS := "windows"
if osSuffix == "windows" {
otherOS = "macos"
}
assert.Empty(t, getInternalSettingName("cli_path_"+otherOS))
assert.Empty(t, getInternalSettingName("reference_folder_"+otherOS))
})

t.Run("getInternalSettingName rejects base name without OS suffix", func(t *testing.T) {
assert.Empty(t, getInternalSettingName("cli_path"))
assert.Empty(t, getInternalSettingName("binary_base_url"))
assert.Empty(t, getInternalSettingName("reference_folder"))
})

t.Run("GetLDXSyncKey appends OS suffix for per-OS settings", func(t *testing.T) {
assert.Equal(t, "cli_path_"+osSuffix, GetLDXSyncKey(SettingCliPath))
assert.Equal(t, "binary_base_url_"+osSuffix, GetLDXSyncKey(SettingBinaryBaseUrl))
assert.Equal(t, "reference_folder_"+osSuffix, GetLDXSyncKey(SettingReferenceFolder))
assert.Equal(t, "additional_parameters_"+osSuffix, GetLDXSyncKey(SettingAdditionalParameters))
assert.Equal(t, "additional_environment_"+osSuffix, GetLDXSyncKey(SettingAdditionalEnvironment))
})

t.Run("GetLDXSyncKey returns standard key for non-per-OS settings", func(t *testing.T) {
assert.Equal(t, "automatic", GetLDXSyncKey(SettingScanAutomatic))
assert.Equal(t, "reference_branch", GetLDXSyncKey(SettingReferenceBranch))
})
}

func TestPtrToBool(t *testing.T) {
t.Run("returns false for nil", func(t *testing.T) {
assert.False(t, util.PtrToBool(nil))
Expand Down
Loading