From cc31459dfffb808ef0f14c234f173fa44065bf42 Mon Sep 17 00:00:00 2001 From: Ben Durrans Date: Wed, 22 Oct 2025 12:02:03 +0100 Subject: [PATCH] fix: org resolver to not take existing org --- pkg/apiclients/ldx_sync_config/resolver.go | 68 ++-------- .../ldx_sync_config/resolver_test.go | 118 +----------------- 2 files changed, 14 insertions(+), 172 deletions(-) diff --git a/pkg/apiclients/ldx_sync_config/resolver.go b/pkg/apiclients/ldx_sync_config/resolver.go index 2c2255c06..798498080 100644 --- a/pkg/apiclients/ldx_sync_config/resolver.go +++ b/pkg/apiclients/ldx_sync_config/resolver.go @@ -5,7 +5,6 @@ import ( "fmt" url2 "net/url" - "github.com/google/uuid" "github.com/rs/zerolog" "github.com/snyk/go-application-framework/pkg/utils" @@ -26,7 +25,7 @@ type LdxSyncConfigResult struct { // Organization is the struct we return to consumers. We redefine it so that consumers don't need to be aware of the // LDX-Sync api version. -// For the initial release pf LDX-Sync they are identical so we use an alias. +// For the initial release of LDX-Sync they are identical so we use an alias. type Organization v20241015.Organization // newClient is a variable that holds the function to create a new LDX-Sync client. @@ -103,33 +102,23 @@ func getLdxSyncConfig(ldxClient v20241015.ClientWithResponsesInterface, orgId st // ResolveOrganization attempts to resolve an organization. // It follows this order: -// 1. Validates and resolves the existing organization value if provided. -// 2. Tries to find a preferred organization from LDX-Sync folder configurations. -// 3. Falls back to the user's default organization from LDX-Sync. -// 4. Falls back to the user's default organization from the Snyk API. -func ResolveOrganization(config configuration.Configuration, engine workflow.Engine, logger *zerolog.Logger, dir string, existingOrgID string) (Organization, error) { +// 1. Tries to find a preferred organization from LDX-Sync folder configurations. +// 2. Falls back to the user's default organization from LDX-Sync. +// 3. Falls back to the user's default organization from the Snyk API. +func ResolveOrganization(config configuration.Configuration, engine workflow.Engine, logger *zerolog.Logger, dir string) (Organization, error) { apiClient := newApiClient(engine, config) - // 1. Handle existing organization value - org, err := handleExistingOrganization(existingOrgID, apiClient, logger) - if err != nil { - return Organization{}, err - } - if org.Id != "" { - return org, nil - } - - // 2. Try LDX-Sync resolution + // 1. Try LDX-Sync resolution ldxClient, err := newClient(engine, config) if err != nil { logger.Debug().Err(err).Msg("Failed to create LDX-Sync client, can't proceed with LDX-Sync resolution") - return fallbackOrganization(nil, apiClient, "", logger) + return fallbackOrganization(logger, apiClient, nil, "") } cfgResult := getLdxSyncConfig(ldxClient, "", dir) if cfgResult.Error != nil { logger.Debug().Err(cfgResult.Error).Msg("LDX-Sync resolution failed, falling back to default") - return fallbackOrganization(nil, apiClient, "", logger) + return fallbackOrganization(logger, apiClient, nil, "") } configData := cfgResult.Config.Data.Attributes.ConfigData @@ -151,11 +140,11 @@ func ResolveOrganization(config configuration.Configuration, engine workflow.Eng logger.Debug().Str("remoteUrl", cfgResult.RemoteUrl).Msg("No folder configurations found in LDX-Sync config, falling back to user default organization") } - // 3 & 4. Fallback - return fallbackOrganization(&configData, apiClient, cfgResult.RemoteUrl, logger) + // 2 & 3. Fallback + return fallbackOrganization(logger, apiClient, &configData, cfgResult.RemoteUrl) } -func getDefaultOrganization(apiClient api.ApiClient, logger *zerolog.Logger) (Organization, error) { +func getDefaultOrganization(logger *zerolog.Logger, apiClient api.ApiClient) (Organization, error) { defaultOrgId, err := apiClient.GetDefaultOrgId() if err != nil { logger.Print("Failed to determine default value for \"ORGANIZATION\":", err) @@ -165,38 +154,7 @@ func getDefaultOrganization(apiClient api.ApiClient, logger *zerolog.Logger) (Or return Organization{Id: defaultOrgId, IsDefault: utils.Ptr(true)}, nil } -func handleExistingOrganization(existingOrgID string, apiClient api.ApiClient, logger *zerolog.Logger) (Organization, error) { - if len(existingOrgID) == 0 { - logger.Debug().Msg("Existing organization value provided is not a string") - return Organization{}, nil - } - - _, err := uuid.Parse(existingOrgID) - isSlugName := err != nil - - if isSlugName { - existingOrgID, err = apiClient.GetOrgIdFromSlug(existingOrgID) - if err != nil { - logger.Print("Failed to determine default value for \"ORGANIZATION\":", err) - return Organization{}, err - } - } - - defaultOrg, err := getDefaultOrganization(apiClient, logger) - if err != nil { - // If we can't get the default org, we can't compare, so return the existing org - return Organization{Id: existingOrgID, IsDefault: utils.Ptr(false)}, err - } - - // If the existing org is the default org, return an empty organization so we use the LDX-Sync resolution - if defaultOrg.Id == existingOrgID { - return Organization{}, nil - } - - return Organization{Id: existingOrgID, IsDefault: utils.Ptr(false)}, nil -} - -func fallbackOrganization(configData *v20241015.ConfigData, apiClient api.ApiClient, remoteUrl string, logger *zerolog.Logger) (Organization, error) { +func fallbackOrganization(logger *zerolog.Logger, apiClient api.ApiClient, configData *v20241015.ConfigData, remoteUrl string) (Organization, error) { // Fallback to default user organization from LDX-Sync response if configData != nil && configData.Organizations != nil { for _, org := range *configData.Organizations { @@ -209,5 +167,5 @@ func fallbackOrganization(configData *v20241015.ConfigData, apiClient api.ApiCli } // Fallback to default org resolution from API - return getDefaultOrganization(apiClient, logger) + return getDefaultOrganization(logger, apiClient) } diff --git a/pkg/apiclients/ldx_sync_config/resolver_test.go b/pkg/apiclients/ldx_sync_config/resolver_test.go index 1d0e30f8b..5368f1a3b 100644 --- a/pkg/apiclients/ldx_sync_config/resolver_test.go +++ b/pkg/apiclients/ldx_sync_config/resolver_test.go @@ -51,7 +51,6 @@ func TestResolveOrganization(t *testing.T) { expectedOrgId string expectedErr error inputDir string - existingOrgID string }{ { name: "empty input directory", @@ -59,7 +58,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: "", - existingOrgID: "", }, { name: "successful resolution with PreferredByAlgorithm", @@ -90,7 +88,6 @@ func TestResolveOrganization(t *testing.T) { }, expectedOrgId: "org-preferred", inputDir: tempDir, - existingOrgID: "", }, { name: "successful resolution using ApplicationvndApiJSON200 response", @@ -118,7 +115,6 @@ func TestResolveOrganization(t *testing.T) { }, expectedOrgId: "org-preferred-vnd", inputDir: tempDir, - existingOrgID: "", }, { name: "fallback to default org when no preferred", @@ -149,7 +145,6 @@ func TestResolveOrganization(t *testing.T) { }, expectedOrgId: "org-default", inputDir: tempDir, - existingOrgID: "", }, { name: "no preferred or default org, fallback to api", @@ -182,7 +177,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: tempDir, - existingOrgID: "", }, { name: "no organizations in folder config, fallback to default", @@ -209,7 +203,6 @@ func TestResolveOrganization(t *testing.T) { }, expectedOrgId: "org-default", inputDir: tempDir, - existingOrgID: "", }, { name: "nil folderconfig in response, fallback to default", @@ -234,7 +227,6 @@ func TestResolveOrganization(t *testing.T) { }, expectedOrgId: "org-default", inputDir: tempDir, - existingOrgID: "", }, { name: "API error, fallback to api", @@ -246,7 +238,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: tempDir, - existingOrgID: "", }, { name: "API returns 404, fallback to api", @@ -261,7 +252,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: tempDir, - existingOrgID: "", }, { name: "API returns 200 with no data, fallback to api", @@ -275,7 +265,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: tempDir, - existingOrgID: "", }, { name: "git remote detection fails, fallback to api", @@ -285,7 +274,6 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: "/tmp/non-existent-dir-for-git-fail", - existingOrgID: "", }, { name: "client creation fails, fallback to api", @@ -293,115 +281,12 @@ func TestResolveOrganization(t *testing.T) { setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) }, expectedOrgId: "default-org", inputDir: tempDir, - existingOrgID: "", setupClientCreation: func() { newClient = func(_ workflow.Engine, _ configuration.Configuration) (v20241015.ClientWithResponsesInterface, error) { return nil, errors.New("client creation failed") } }, }, - { - name: "existing valid org ID (not default)", - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetDefaultOrgId().Return("22222222-2222-2222-2222-222222222222", nil) - }, - expectedOrgId: "123e4567-e89b-12d3-a456-426614174000", - inputDir: tempDir, - existingOrgID: "123e4567-e89b-12d3-a456-426614174000", - }, - { - name: "existing org ID is default, return LDX-Sync preferred", - setupMock: func(mock *ldx_mocks.MockClientWithResponsesInterface) { - mock.EXPECT(). - GetConfigWithResponse(gomock.Any(), gomock.Any()). - Return(&v20241015.GetConfigResponse{ - JSON200: &v20241015.ConfigResponse{ - Data: v20241015.ConfigResource{ - Attributes: v20241015.ConfigAttributes{ - ConfigData: v20241015.ConfigData{ - FolderConfigs: &[]v20241015.FolderConfig{ - { - Organizations: &[]v20241015.Organization{ - {Id: "ldx-preferred-org", PreferredByAlgorithm: utils.Ptr(true)}, - }, - }, - }, - }, - }, - }, - }, - HTTPResponse: &http.Response{StatusCode: http.StatusOK}, - }, nil) - }, - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetDefaultOrgId().Return("11111111-1111-1111-1111-111111111111", nil) - }, - expectedOrgId: "ldx-preferred-org", - inputDir: tempDir, - existingOrgID: "11111111-1111-1111-1111-111111111111", - }, - { - name: "existing valid org slug (not default)", - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetOrgIdFromSlug("my-org").Return("33333333-3333-3333-3333-333333333333", nil) - mock.EXPECT().GetDefaultOrgId().Return("22222222-2222-2222-2222-222222222222", nil) - }, - expectedOrgId: "33333333-3333-3333-3333-333333333333", - inputDir: tempDir, - existingOrgID: "my-org", - }, - { - name: "existing org slug is default, return LDX-Sync preferred", - setupMock: func(mock *ldx_mocks.MockClientWithResponsesInterface) { - mock.EXPECT(). - GetConfigWithResponse(gomock.Any(), gomock.Any()). - Return(&v20241015.GetConfigResponse{ - JSON200: &v20241015.ConfigResponse{ - Data: v20241015.ConfigResource{ - Attributes: v20241015.ConfigAttributes{ - ConfigData: v20241015.ConfigData{ - FolderConfigs: &[]v20241015.FolderConfig{ - { - Organizations: &[]v20241015.Organization{ - {Id: "ldx-preferred-from-slug", PreferredByAlgorithm: utils.Ptr(true)}, - }, - }, - }, - }, - }, - }, - }, - HTTPResponse: &http.Response{StatusCode: http.StatusOK}, - }, nil) - }, - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetOrgIdFromSlug("default-org-slug").Return("11111111-1111-1111-1111-111111111111", nil) - mock.EXPECT().GetDefaultOrgId().Return("11111111-1111-1111-1111-111111111111", nil) - }, - expectedOrgId: "ldx-preferred-from-slug", - inputDir: tempDir, - existingOrgID: "default-org-slug", - }, - { - name: "existing invalid org slug", - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetOrgIdFromSlug("invalid-org").Return("", errors.New("not found")) - }, - expectedOrgId: "", - expectedErr: errors.New("not found"), - inputDir: tempDir, - existingOrgID: "invalid-org", - }, - { - name: "existing org ID, GetDefaultOrgId fails, returns error", - setupApiMock: func(mock *api_mocks.MockApiClient) { - mock.EXPECT().GetDefaultOrgId().Return("", errors.New("api error")) - }, - expectedOrgId: "", - expectedErr: errors.New("api error"), - inputDir: tempDir, - existingOrgID: "123e4567-e89b-12d3-a456-426614174000", - }, { name: "LDX fails, fallback to API default org fails", setupMock: func(mock *ldx_mocks.MockClientWithResponsesInterface) { @@ -415,7 +300,6 @@ func TestResolveOrganization(t *testing.T) { expectedOrgId: "", expectedErr: errors.New("api is down"), inputDir: tempDir, - existingOrgID: "", }, } @@ -446,7 +330,7 @@ func TestResolveOrganization(t *testing.T) { config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) - result, err := ResolveOrganization(config, mockEngine, &logger, tt.inputDir, tt.existingOrgID) + result, err := ResolveOrganization(config, mockEngine, &logger, tt.inputDir) if tt.expectedErr != nil { assert.Error(t, err) assert.Equal(t, tt.expectedErr, err)