Skip to content

chore: iaw uses org setting insted of feature flag #396

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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 go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ require (
github.com/oapi-codegen/oapi-codegen/v2 v2.4.1
github.com/oapi-codegen/runtime v1.1.1
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/snyk/error-catalog-golang-public v0.0.0-20250625135845-2d6f9a31f318
github.com/snyk/error-catalog-golang-public v0.0.0-20250812140843-a01d75260003
github.com/subosito/gotenv v1.6.0
golang.org/x/net v0.38.0
golang.org/x/sync v0.13.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ github.com/skeema/knownhosts v1.3.1 h1:X2osQ+RAjK76shCbvhHHHVl3ZlgDm8apHEHFqRjnB
github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQNxPwTcfiY=
github.com/snyk/code-client-go v1.21.3 h1:2+HPXCA9FGn3gaI1Jw1C4Ifn/NRAbSnmohFUvz4GC4I=
github.com/snyk/code-client-go v1.21.3/go.mod h1:WH6lNkJc785hfXmwhixxWHix3O6z+1zwz40oK8vl/zg=
github.com/snyk/error-catalog-golang-public v0.0.0-20250625135845-2d6f9a31f318 h1:2bNOlUstBBWHa3doBvdOBlMSu8AC01IHyNexT9MoKiM=
github.com/snyk/error-catalog-golang-public v0.0.0-20250625135845-2d6f9a31f318/go.mod h1:Ytttq7Pw4vOCu9NtRQaOeDU2dhBYUyNBe6kX4+nIIQ4=
github.com/snyk/error-catalog-golang-public v0.0.0-20250812140843-a01d75260003 h1:qeXih9sVe/WvhccE3MfEgglnSVKN1xTQBcsA/N96Kzo=
github.com/snyk/error-catalog-golang-public v0.0.0-20250812140843-a01d75260003/go.mod h1:Ytttq7Pw4vOCu9NtRQaOeDU2dhBYUyNBe6kX4+nIIQ4=
github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530 h1:s9PHNkL6ueYRiAKNfd8OVxlUOqU3qY0VDbgCD1f6WQY=
github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530/go.mod h1:88KbbvGYlmLgee4OcQ19yr0bNpXpOr2kciOthaSzCAg=
github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
Expand Down
24 changes: 24 additions & 0 deletions internal/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type ApiClient interface {
GetUserMe() (string, error)
GetSelf() (contract.SelfResponse, error)
GetSastSettings(orgId string) (*sast_contract.SastResponse, error)
GetOrgSettings(orgId string) (*contract.OrgSettingsResponse, error)
}

var _ ApiClient = (*snykApiClient)(nil)
Expand Down Expand Up @@ -247,6 +248,29 @@ func (a *snykApiClient) GetSastSettings(orgId string) (*sast_contract.SastRespon
return &response, err
}

func (a *snykApiClient) GetOrgSettings(orgId string) (*contract.OrgSettingsResponse, error) {
endpoint := fmt.Sprintf("%s/v1/org/%s/settings", a.url, url.QueryEscape(orgId))

res, err := a.client.Get(endpoint)
if err != nil {
return nil, fmt.Errorf("unable to retrieve org settings: %w", err)
}
//goland:noinspection GoUnhandledErrorResult
defer res.Body.Close()

body, err := io.ReadAll(res.Body)
if err != nil {
return nil, fmt.Errorf("unable to retrieve org settings: %w", err)
}

var response contract.OrgSettingsResponse
if err = json.Unmarshal(body, &response); err != nil {
return nil, fmt.Errorf("unable to retrieve org settings (status: %d): %w", res.StatusCode, err)
}

return &response, err
}

// clientGet performs an HTTP GET request to the Snyk API, handling query parameters,
// API versioning, and basic error checking.
//
Expand Down
18 changes: 18 additions & 0 deletions internal/api/contract/OrgSettingsResponse.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package contract

// API reference: https://docs.snyk.io/snyk-api/reference/organizations-v1#get-org-orgid-settings

type OrgIgnoreSettings struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: please link to the API spec this relates to

ReasonRequired bool `json:"reasonRequired,omitempty"`
AutoApproveIgnores bool `json:"autoApproveIgnores,omitempty"`
ApprovalWorkflowEnabled bool `json:"approvalWorkflowEnabled,omitempty"`
}

type OrgRequestAccessSettings struct {
Enabled bool `json:"enabled,omitempty"`
}

type OrgSettingsResponse struct {
Ignores *OrgIgnoreSettings `json:"ignores"`
RequestAccess *OrgRequestAccessSettings `json:"requestAccess"`
}
15 changes: 15 additions & 0 deletions internal/mocks/api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions pkg/local_workflows/ignore_workflow/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/uuid"

"github.com/snyk/error-catalog-golang-public/cli"
"github.com/snyk/go-application-framework/internal/api"
policyApi "github.com/snyk/go-application-framework/internal/api/policy/2024-10-15"
"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/local_workflows/local_models"
Expand Down Expand Up @@ -43,6 +44,32 @@ func addCreateIgnoreDefaultConfigurationValues(invocationCtx workflow.Invocation
})
}

func getOrgIgnoreApprovalEnabled(engine workflow.Engine) configuration.DefaultValueFunction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: please add a test

return func(_ configuration.Configuration, existingValue interface{}) (interface{}, error) {
if existingValue != nil {
return existingValue, nil
}

config := engine.GetConfiguration()
org := config.GetString(configuration.ORGANIZATION)
client := engine.GetNetworkAccess().GetHttpClient()
url := config.GetString(configuration.API_URL)
apiClient := api.NewApi(url, client)

settings, err := apiClient.GetOrgSettings(org)
if err != nil {
engine.GetLogger().Err(err).Msg("Failed to access settings.")
return nil, err
}

if settings.Ignores != nil && settings.Ignores.ApprovalWorkflowEnabled {
return true, nil
}

return false, nil
}
}
Copy link

Choose a reason for hiding this comment

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

Bug

The getOrgIgnoreApprovalEnabled function incorrectly determines if the ignore approval workflow is enabled, leading to incorrect feature gating or workflow blocking, due to three issues:

  1. Incorrect Configuration Scope: It uses engine.GetConfiguration() and engine.GetNetworkAccess() instead of the invocation-specific configuration instance passed to its DefaultValueFunction. This can lead to querying settings for the wrong organization or using the wrong API endpoint/client when invocation overrides (e.g., --org) are present.
  2. Premature Defaulting for Booleans: It returns early if existingValue is not nil. For boolean configuration keys, an unset key often defaults to false (a non-nil zero value), which bypasses the actual settings fetch and forces the feature to be false by default, incorrectly disabling it.
  3. Swallowed Errors: If fetching settings fails, it returns (nil, err). Since config.GetBool() (the likely caller) swallows this error, the value effectively defaults to false, leading to the workflow being blocked as if the feature were disabled, even due to transient network or backend issues.
Fix in Cursor Fix in Web


func remoteRepoUrlDefaultFunc(existingValue interface{}, config configuration.Configuration) (interface{}, error) {
if existingValue != nil && existingValue != "" {
return existingValue, nil
Expand Down
12 changes: 7 additions & 5 deletions pkg/local_workflows/ignore_workflow/ignore_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import (

policyApi "github.com/snyk/go-application-framework/internal/api/policy/2024-10-15"
"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/local_workflows"
"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
"github.com/snyk/go-application-framework/pkg/local_workflows/local_models"
"github.com/snyk/go-application-framework/pkg/utils/git"
"github.com/snyk/go-application-framework/pkg/workflow"
Expand Down Expand Up @@ -60,6 +59,8 @@ const (

interactiveEnsureVersionControlMessage = "👉🏼 Ensure the code containing the issue is committed and pushed to remote origin, so approvers can review it."
interactiveIgnoreRequestSubmissionMessage = "✅ Your ignore request has been submitted."

ConfigIgnoreApprovalEnabled = "internal_iaw_enabled"
)

var reasonPromptHelpMap = map[string]string{
Expand Down Expand Up @@ -88,7 +89,7 @@ func InitIgnoreWorkflows(engine workflow.Engine) error {
return err
}

config_utils.AddFeatureFlagToConfig(engine, configuration.FF_IAW_ENABLED, "ignoreApprovalWorkflow")
engine.GetConfiguration().AddDefaultValue(ConfigIgnoreApprovalEnabled, getOrgIgnoreApprovalEnabled(engine))

return nil
}
Expand All @@ -100,8 +101,9 @@ func ignoreCreateWorkflowEntryPoint(invocationCtx workflow.InvocationContext, _
config := invocationCtx.GetConfiguration()
id := invocationCtx.GetWorkflowIdentifier()

if !config.GetBool(configuration.FF_IAW_ENABLED) {
return nil, cli.NewFeatureUnderDevelopmentError("")
if !config.GetBool(ConfigIgnoreApprovalEnabled) {
orgName := config.GetString(configuration.ORGANIZATION_SLUG)
return nil, cli.NewFeatureNotEnabledError(fmt.Sprintf("The Ignore Approval Workflow feature must be enabled for the %s organization. Enable it in your organization settings: Settings > General > Ignore approval workflow for Snyk Code.", orgName))
}

interactive := config.GetBool(InteractiveKey)
Expand Down
118 changes: 115 additions & 3 deletions pkg/local_workflows/ignore_workflow/ignore_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/snyk/error-catalog-golang-public/snyk_errors"
"github.com/stretchr/testify/assert"

"github.com/snyk/go-application-framework/internal/api/contract"
policyApi "github.com/snyk/go-application-framework/internal/api/policy/2024-10-15"
"github.com/snyk/go-application-framework/pkg/configuration"
localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
Expand Down Expand Up @@ -51,7 +52,7 @@ func setupMockIgnoreContext(t *testing.T, payload string, statusCode int) *mocks
config := configuration.New()
config.Set(configuration.API_URL, "https://api.snyk.io")
config.Set(configuration.ORGANIZATION, uuid.New().String())
config.Set(configuration.FF_IAW_ENABLED, true)
config.Set(ConfigIgnoreApprovalEnabled, true)
// setup mocks
ctrl := gomock.NewController(t)
networkAccessMock := mocks.NewMockNetworkAccess(ctrl)
Expand Down Expand Up @@ -306,7 +307,7 @@ func Test_ignoreCreateWorkflowEntryPoint(t *testing.T) {

invocationContext := setupMockIgnoreContext(t, "{}", http.StatusCreated)
config := invocationContext.GetConfiguration()
config.Set(configuration.FF_IAW_ENABLED, false)
config.Set(ConfigIgnoreApprovalEnabled, false)

config.Set(InteractiveKey, false)

Expand All @@ -330,7 +331,7 @@ func setupInteractiveMockContext(t *testing.T, mockApiResponse string, mockApiSt
config := configuration.New()
config.Set(configuration.API_URL, "https://api.snyk.io")
config.Set(configuration.ORGANIZATION, uuid.New().String())
config.Set(configuration.FF_IAW_ENABLED, true)
config.Set(ConfigIgnoreApprovalEnabled, true)
config.Set(InteractiveKey, true) // Always interactive
config.Set(EnrichResponseKey, true)
config.Set(configuration.ORGANIZATION_SLUG, "some-org")
Expand Down Expand Up @@ -705,3 +706,114 @@ func Test_getExpireValue(t *testing.T) {
assert.Nil(t, result)
})
}

func Test_getOrgIgnoreApprovalEnabled(t *testing.T) {
t.Run("returns existing value when not nil", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockEngine := mocks.NewMockEngine(ctrl)
defaultValueFunc := getOrgIgnoreApprovalEnabled(mockEngine)

result, err := defaultValueFunc(nil, true)
assert.NoError(t, err)
assert.Equal(t, true, result)

result, err = defaultValueFunc(nil, false)
assert.NoError(t, err)
assert.Equal(t, false, result)
})

t.Run("approval workflow enabled", func(t *testing.T) {
result, err := setupMockEngineForOrgSettings(t, &contract.OrgSettingsResponse{
Ignores: &contract.OrgIgnoreSettings{ApprovalWorkflowEnabled: true},
})

assert.NoError(t, err)
assert.Equal(t, true, result)
})

t.Run("approval workflow disabled", func(t *testing.T) {
result, err := setupMockEngineForOrgSettings(t, &contract.OrgSettingsResponse{
Ignores: &contract.OrgIgnoreSettings{ApprovalWorkflowEnabled: false},
})

assert.NoError(t, err)
assert.Equal(t, false, result)
})

t.Run("ignores field is nil", func(t *testing.T) {
result, err := setupMockEngineForOrgSettings(t, &contract.OrgSettingsResponse{
Ignores: nil,
})

assert.NoError(t, err)
assert.Equal(t, false, result)
})

t.Run("API call fails", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

logger := zerolog.Logger{}
orgId := uuid.New().String()
apiUrl := "https://api.snyk.io"

mockEngine := mocks.NewMockEngine(ctrl)
mockConfig := mocks.NewMockConfiguration(ctrl)
mockNetworkAccess := mocks.NewMockNetworkAccess(ctrl)

httpClient := localworkflows.NewTestClient(func(req *http.Request) *http.Response {
return &http.Response{
StatusCode: http.StatusInternalServerError,
Body: io.NopCloser(bytes.NewBufferString("Internal Server Error")),
}
})

mockEngine.EXPECT().GetConfiguration().Return(mockConfig)
mockEngine.EXPECT().GetNetworkAccess().Return(mockNetworkAccess)
mockEngine.EXPECT().GetLogger().Return(&logger)
mockConfig.EXPECT().GetString(configuration.ORGANIZATION).Return(orgId)
mockConfig.EXPECT().GetString(configuration.API_URL).Return(apiUrl)
mockNetworkAccess.EXPECT().GetHttpClient().Return(httpClient)

defaultValueFunc := getOrgIgnoreApprovalEnabled(mockEngine)
result, err := defaultValueFunc(nil, nil)

assert.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "unable to retrieve org settings")
})
}

func setupMockEngineForOrgSettings(t *testing.T, response *contract.OrgSettingsResponse) (interface{}, error) {
t.Helper()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

orgId := uuid.New().String()
apiUrl := "https://api.snyk.io"

responseJSON, err := json.Marshal(response)
assert.NoError(t, err)

mockEngine := mocks.NewMockEngine(ctrl)
mockConfig := mocks.NewMockConfiguration(ctrl)
mockNetworkAccess := mocks.NewMockNetworkAccess(ctrl)

httpClient := localworkflows.NewTestClient(func(req *http.Request) *http.Response {
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewBuffer(responseJSON)),
}
})

mockEngine.EXPECT().GetConfiguration().Return(mockConfig)
mockEngine.EXPECT().GetNetworkAccess().Return(mockNetworkAccess)
mockConfig.EXPECT().GetString(configuration.ORGANIZATION).Return(orgId)
mockConfig.EXPECT().GetString(configuration.API_URL).Return(apiUrl)
mockNetworkAccess.EXPECT().GetHttpClient().Return(httpClient)

defaultValueFunc := getOrgIgnoreApprovalEnabled(mockEngine)
return defaultValueFunc(nil, nil)
}