Validate config before patching

mindersec · Jun 4, 2024 · 7cf3925 · 7cf3925
1 parent 6da5849
commit 7cf3925
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 3 deletions.
diff --git a/internal/providers/manager/manager.go b/internal/providers/manager/manager.go
@@ -252,6 +252,16 @@ func (p *providerManager) PatchProviderConfig(
 		return fmt.Errorf("error marshalling provider config: %w", err)
 	}
 
+	manager, err := p.getClassManager(dbProvider.Class)
+	if err != nil {
+		return err
+	}
+
+	err = manager.ValidateConfig(ctx, dbProvider.Class, mergedJSON)
+	if err != nil {
+		return fmt.Errorf("error validating provider config: %w", err)
+	}
+
 	return p.store.Update(ctx, dbProvider.ID, dbProvider.ProjectID, mergedJSON)
 }
 

diff --git a/internal/providers/manager/manager_test.go b/internal/providers/manager/manager_test.go
@@ -73,6 +73,7 @@ func TestProviderManager_PatchProviderConfig(t *testing.T) {
 		Patch         map[string]any
 		MergedConfig  json.RawMessage
 		ExpectedError string
+		ValidConfig   bool
 	}{
 		{
 			Name:          "Enabling the auto_enroll field",
@@ -84,6 +85,7 @@ func TestProviderManager_PatchProviderConfig(t *testing.T) {
 				},
 			},
 			MergedConfig: json.RawMessage(`{ "auto_registration": { "enabled": ["repository"] }, "github-app": {}}`),
+			ValidConfig:  true,
 		},
 		{
 			Name:          "Disabling the auto_enroll field",
@@ -95,6 +97,20 @@ func TestProviderManager_PatchProviderConfig(t *testing.T) {
 				},
 			},
 			MergedConfig: json.RawMessage(`{ "auto_registration": { "enabled": [] }, "github-app": {}}`),
+			ValidConfig:  true,
+		},
+		{
+			Name:          "Invalid config doesn't call the store.Update method",
+			Provider:      githubAppProvider,
+			CurrentConfig: json.RawMessage(`{ "auto_registration": { "enabled": ["repository"] }, "github-app": {}}`),
+			Patch: map[string]any{
+				"auto_registration": map[string]any{
+					"enabled": []string{"my_little_pony"},
+				},
+			},
+			MergedConfig:  json.RawMessage(`{ "auto_registration": { "enabled": [] }, "github-app": {}}`),
+			ValidConfig:   false,
+			ExpectedError: "error validating provider config: invalid config",
 		},
 	}
 
@@ -119,9 +135,20 @@ func TestProviderManager_PatchProviderConfig(t *testing.T) {
 				Return(dbProvider, nil).
 				Times(1)
 
-			store.EXPECT().Update(ctx, dbProvider.ID, dbProvider.ProjectID, &configMatcher{expected: scenario.MergedConfig}).
-				Return(nil).
-				Times(1)
+			if scenario.ValidConfig {
+				classManager.EXPECT().ValidateConfig(ctx, dbProvider.Class, gomock.Any()).
+					Return(nil).
+					Times(1)
+				store.EXPECT().Update(ctx, dbProvider.ID, dbProvider.ProjectID, &configMatcher{expected: scenario.MergedConfig}).
+					Return(nil).
+					Times(1)
+			} else {
+				classManager.EXPECT().ValidateConfig(ctx, dbProvider.Class, gomock.Any()).
+					Return(errors.New("invalid config")).
+					Times(1)
+				store.EXPECT().Update(ctx, dbProvider.ID, dbProvider.ProjectID, gomock.Any()).
+					Times(0)
+			}
 
 			err = provManager.PatchProviderConfig(ctx, scenario.Provider.Name, scenario.Provider.ProjectID, scenario.Patch)
 			if scenario.ExpectedError != "" {