Skip to content

fix: org resolver to not take existing org [IDE-1498] #451

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −172
Open

fix: org resolver to not take existing org [IDE-1498] #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 13 additions & 55 deletions pkg/apiclients/ldx_sync_config/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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)
}
118 changes: 1 addition & 117 deletions pkg/apiclients/ldx_sync_config/resolver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,13 @@ func TestResolveOrganization(t *testing.T) {
expectedOrgId string
expectedErr error
inputDir string
existingOrgID string
}{
{
name: "empty input directory",
setupMock: func(mock *ldx_mocks.MockClientWithResponsesInterface) {},
setupApiMock: func(mock *api_mocks.MockApiClient) { mock.EXPECT().GetDefaultOrgId().Return("default-org", nil) },
expectedOrgId: "default-org",
inputDir: "",
existingOrgID: "",
},
{
name: "successful resolution with PreferredByAlgorithm",
Expand Down Expand Up @@ -90,7 +88,6 @@ func TestResolveOrganization(t *testing.T) {
},
expectedOrgId: "org-preferred",
inputDir: tempDir,
existingOrgID: "",
},
{
name: "successful resolution using ApplicationvndApiJSON200 response",
Expand Down Expand Up @@ -118,7 +115,6 @@ func TestResolveOrganization(t *testing.T) {
},
expectedOrgId: "org-preferred-vnd",
inputDir: tempDir,
existingOrgID: "",
},
{
name: "fallback to default org when no preferred",
Expand Down Expand Up @@ -149,7 +145,6 @@ func TestResolveOrganization(t *testing.T) {
},
expectedOrgId: "org-default",
inputDir: tempDir,
existingOrgID: "",
},
{
name: "no preferred or default org, fallback to api",
Expand Down Expand Up @@ -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",
Expand All @@ -209,7 +203,6 @@ func TestResolveOrganization(t *testing.T) {
},
expectedOrgId: "org-default",
inputDir: tempDir,
existingOrgID: "",
},
{
name: "nil folderconfig in response, fallback to default",
Expand All @@ -234,7 +227,6 @@ func TestResolveOrganization(t *testing.T) {
},
expectedOrgId: "org-default",
inputDir: tempDir,
existingOrgID: "",
},
{
name: "API error, fallback to api",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -285,123 +274,19 @@ 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",
setupMock: func(mock *ldx_mocks.MockClientWithResponsesInterface) {},
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) {
Expand All @@ -415,7 +300,6 @@ func TestResolveOrganization(t *testing.T) {
expectedOrgId: "",
expectedErr: errors.New("api is down"),
inputDir: tempDir,
existingOrgID: "",
},
}

Expand Down Expand Up @@ -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)
Expand Down