Merge pull request #2519 from keboola/hosan/SUPPORT-15208

Fix conditional requirements in JSON Schema validation

Author: hosekpeter

internal/pkg/encoding/json/schema/schema.go

@@ -207,18 +207,32 @@ func NormalizeSchema(schema []byte) ([]byte, error) {
 	})
 
 	// Process conditional requirements: remove from required arrays and add if/then/else constructs
+	// IMPORTANT: Only add if/then for fields that were ORIGINALLY in the required array.
+	// options.dependencies is for UI visibility, not for making fields required.
+	// If a field was never required, it should stay optional even when visible.
 	for parentPathStr, reqs := range conditionalReqs {
 		parentObj := getObjectAtPath(m, parentPathStr)
 		if parentObj == nil {
 			continue
 		}
 
-		// Remove conditionally required fields from the required array
-		removeConditionalFieldsFromRequired(parentObj, reqs)
+		// Get the set of originally required fields
+		originallyRequired := getRequiredFields(parentObj)
 
-		// Generate if/then/else constructs for each conditional requirement
-		allOfItems := make([]any, 0, len(reqs))
+		// Filter to only fields that were originally required
+		var requiredReqs []conditionalRequirement
 		for _, req := range reqs {
+			if originallyRequired[req.fieldName] {
+				requiredReqs = append(requiredReqs, req)
+			}
+		}
+
+		// Remove conditionally required fields from the required array
+		removeConditionalFieldsFromRequired(parentObj, requiredReqs)
+
+		// Generate if/then/else constructs only for fields that were originally required
+		allOfItems := make([]any, 0, len(requiredReqs))
+		for _, req := range requiredReqs {
 			ifThenElse := buildIfThenElse(req)
 			if ifThenElse != nil {
 				allOfItems = append(allOfItems, ifThenElse)
@@ -246,6 +260,25 @@ func NormalizeSchema(schema []byte) ([]byte, error) {
 	return normalized, nil
 }
 
+// getRequiredFields returns a set of field names that are in the required array.
+func getRequiredFields(parentObj *orderedmap.OrderedMap) map[string]bool {
+	result := make(map[string]bool)
+	requiredVal, found := parentObj.Get("required")
+	if !found {
+		return result
+	}
+	requiredArr, ok := requiredVal.([]any)
+	if !ok {
+		return result
+	}
+	for _, field := range requiredArr {
+		if fieldStr, ok := field.(string); ok {
+			result[fieldStr] = true
+		}
+	}
+	return result
+}
+
 // removeConditionalFieldsFromRequired removes conditionally required fields from the required array.
 func removeConditionalFieldsFromRequired(parentObj *orderedmap.OrderedMap, reqs []conditionalRequirement) {
 	requiredVal, found := parentObj.Get("required")
@@ -310,8 +343,9 @@ func buildIfThenElse(req conditionalRequirement) *orderedmap.OrderedMap {
 		return nil
 	}
 
-	// Build the "if" condition properties
+	// Build the "if" condition properties and collect dependency field names
 	ifProperties := orderedmap.New()
+	requiredFields := make([]any, 0, len(req.dependencies))
 	for depField, depValue := range req.dependencies {
 		condition := orderedmap.New()
 		// Handle array values (e.g., "protocol": ["FTP", "FTPS"]) using enum
@@ -322,10 +356,19 @@ func buildIfThenElse(req conditionalRequirement) *orderedmap.OrderedMap {
 			condition.Set("const", depValue)
 		}
 		ifProperties.Set(depField, condition)
+		requiredFields = append(requiredFields, depField)
 	}
 
 	// Build the "if" clause
+	// IMPORTANT: In JSON Schema, "if: { properties: { field: {const: value} } }" without "required"
+	// evaluates to TRUE when field is MISSING (because missing properties match any schema).
+	// We must add "required" to ensure "if" only applies when the dependency field exists.
+	// This way:
+	// - If dependency field is MISSING → "if" is FALSE → "then" doesn't apply → field is optional ✓
+	// - If dependency field EXISTS but has wrong value → "if" is FALSE → "then" doesn't apply → field is optional ✓
+	// - If dependency field EXISTS and has correct value → "if" is TRUE → "then" applies → field is required ✓
 	ifClause := orderedmap.New()
+	ifClause.Set("required", requiredFields)
 	ifClause.Set("properties", ifProperties)
 
 	// Build the "then" clause with required field

internal/pkg/encoding/json/schema/schema_test.go

@@ -729,8 +729,9 @@ func TestNormalizeSchema_OptionsDependencies_NestedObject(t *testing.T) {
 func TestNormalizeSchema_OptionsDependencies_NotInRequired(t *testing.T) {
 	t.Parallel()
 
-	// Schema with options.dependencies but the field is not in the required array
-	// The transformation should still add the if/then construct but not modify required
+	// Schema with options.dependencies but the field is NOT in the required array.
+	// options.dependencies is for UI visibility, NOT for making fields required.
+	// Therefore, if the field was not originally required, we should NOT add if/then for it.
 	schema := []byte(`
 {
   "type": "object",
@@ -765,10 +766,10 @@ func TestNormalizeSchema_OptionsDependencies_NotInRequired(t *testing.T) {
 	require.True(t, ok)
 	assert.Equal(t, []any{"mode"}, required)
 
-	// Check that allOf with if/then is still added
-	allOf, ok := result["allOf"].([]any)
-	require.True(t, ok)
-	require.Len(t, allOf, 1)
+	// Check that NO allOf was added - optional_field was not in required,
+	// so we don't need conditional requirement logic
+	_, hasAllOf := result["allOf"]
+	assert.False(t, hasAllOf, "allOf should NOT be added for fields that were not originally required")
 }
 
 func TestNormalizeSchema_OptionsDependencies_Validation(t *testing.T) {
@@ -962,6 +963,156 @@ func TestNormalizeSchema_ExistingAllOfIfThen(t *testing.T) {
 	require.NoError(t, err, "Should pass validation when append_date=1 and append_date_format is provided")
 }
 
+func TestNormalizeSchema_OptionsDependencies_MissingDependencyField(t *testing.T) {
+	t.Parallel()
+
+	// This test uses the actual schema from kds-team.app-orchestration-trigger-queue-v2 component.
+	// triggerActionOnFailure is NOT in the required array.
+	// triggerActionOnFailure has options.dependencies: { waitUntilFinish: true }
+	// actionOnFailureSettings has options.dependencies: { triggerActionOnFailure: true }
+	schema := []byte(`
+{
+  "type": "object",
+  "required": ["waitUntilFinish", "#kbcToken", "kbcUrl", "orchestrationId"],
+  "properties": {
+    "waitUntilFinish": {
+      "type": "boolean"
+    },
+    "triggerActionOnFailure": {
+      "type": "boolean",
+      "options": {
+        "dependencies": {
+          "waitUntilFinish": true
+        }
+      }
+    },
+    "actionOnFailureSettings": {
+      "type": "object",
+      "required": ["failureConfigurationId"],
+      "properties": {
+        "failureConfigurationId": {
+          "type": "string"
+        }
+      },
+      "options": {
+        "dependencies": {
+          "triggerActionOnFailure": true
+        }
+      }
+    },
+    "#kbcToken": {
+      "type": "string"
+    },
+    "kbcUrl": {
+      "type": "string"
+    },
+    "orchestrationId": {
+      "type": "string"
+    }
+  }
+}
+`)
+
+	// Test 1: triggerActionOnFailure field is missing entirely (fire-and-forget config)
+	// This should PASS - actionOnFailureSettings should NOT be required
+	content1 := orderedmap.FromPairs([]orderedmap.Pair{
+		{
+			Key: "parameters",
+			Value: orderedmap.FromPairs([]orderedmap.Pair{
+				{Key: "waitUntilFinish", Value: false},
+				{Key: "#kbcToken", Value: "xxx"},
+				{Key: "kbcUrl", Value: "-"},
+				{Key: "orchestrationId", Value: "123"},
+				// triggerActionOnFailure is NOT present at all
+			}),
+		},
+	})
+	err := ValidateContent(schema, content1)
+	require.NoError(t, err, "Should pass when triggerActionOnFailure field is missing (undefined)")
+
+	// Test 2: triggerActionOnFailure is explicitly false
+	// This should PASS - actionOnFailureSettings should NOT be required
+	content2 := orderedmap.FromPairs([]orderedmap.Pair{
+		{
+			Key: "parameters",
+			Value: orderedmap.FromPairs([]orderedmap.Pair{
+				{Key: "waitUntilFinish", Value: true},
+				{Key: "#kbcToken", Value: "xxx"},
+				{Key: "kbcUrl", Value: "-"},
+				{Key: "triggerActionOnFailure", Value: false},
+				{Key: "orchestrationId", Value: "123"},
+			}),
+		},
+	})
+	err = ValidateContent(schema, content2)
+	require.NoError(t, err, "Should pass when triggerActionOnFailure is explicitly false")
+
+	// Test 3: triggerActionOnFailure is true but actionOnFailureSettings is missing
+	// This should PASS - actionOnFailureSettings was NOT in the required array,
+	// options.dependencies is only for UI visibility, not for making fields required.
+	content3 := orderedmap.FromPairs([]orderedmap.Pair{
+		{
+			Key: "parameters",
+			Value: orderedmap.FromPairs([]orderedmap.Pair{
+				{Key: "waitUntilFinish", Value: true},
+				{Key: "#kbcToken", Value: "xxx"},
+				{Key: "kbcUrl", Value: "-"},
+				{Key: "triggerActionOnFailure", Value: true},
+				{Key: "orchestrationId", Value: "123"},
+			}),
+		},
+	})
+	err = ValidateContent(schema, content3)
+	require.NoError(t, err, "Should pass - actionOnFailureSettings was NOT in required, options.dependencies is for visibility only")
+
+	// Test 4: triggerActionOnFailure is true and actionOnFailureSettings is provided but missing internal required field
+	// This should FAIL - failureConfigurationId is required WITHIN actionOnFailureSettings
+	content4 := orderedmap.FromPairs([]orderedmap.Pair{
+		{
+			Key: "parameters",
+			Value: orderedmap.FromPairs([]orderedmap.Pair{
+				{Key: "waitUntilFinish", Value: true},
+				{Key: "#kbcToken", Value: "xxx"},
+				{Key: "kbcUrl", Value: "-"},
+				{Key: "triggerActionOnFailure", Value: true},
+				{Key: "orchestrationId", Value: "123"},
+				{
+					Key:   "actionOnFailureSettings",
+					Value: orderedmap.FromPairs([]orderedmap.Pair{
+						// missing failureConfigurationId
+					}),
+				},
+			}),
+		},
+	})
+	err = ValidateContent(schema, content4)
+	require.Error(t, err, "Should fail - failureConfigurationId is required within actionOnFailureSettings")
+	assert.Contains(t, err.Error(), "failureConfigurationId")
+
+	// Test 5: triggerActionOnFailure is true and actionOnFailureSettings is provided with required field
+	// This should PASS
+	content5 := orderedmap.FromPairs([]orderedmap.Pair{
+		{
+			Key: "parameters",
+			Value: orderedmap.FromPairs([]orderedmap.Pair{
+				{Key: "waitUntilFinish", Value: true},
+				{Key: "#kbcToken", Value: "xxx"},
+				{Key: "kbcUrl", Value: "-"},
+				{Key: "triggerActionOnFailure", Value: true},
+				{Key: "orchestrationId", Value: "123"},
+				{
+					Key: "actionOnFailureSettings",
+					Value: orderedmap.FromPairs([]orderedmap.Pair{
+						{Key: "failureConfigurationId", Value: "456"},
+					}),
+				},
+			}),
+		},
+	})
+	err = ValidateContent(schema, content5)
+	require.NoError(t, err, "Should pass when actionOnFailureSettings is provided with required fields")
+}
+
 func getTestSchema() []byte {
 	return []byte(`
 {

internal/pkg/fixtures/configs/trigger-fire-and-forget.json

@@ -0,0 +1,13 @@
+{
+  "componentId": "kds-team.app-orchestration-trigger-queue-v2",
+  "name": "trigger-fire-and-forget",
+  "configuration": {
+    "parameters": {
+      "waitUntilFinish": false,
+      "#kbcToken": "KBC::ProjectSecure::dummytoken",
+      "kbcUrl": "-",
+      "orchestrationId": "123456"
+    }
+  },
+  "rows": null
+}

test/cli/pull/orchestration-trigger-conditional-requirement/args

@@ -0,0 +1 @@
+pull --storage-api-token %%TEST_KBC_STORAGE_API_TOKEN%%

test/cli/pull/orchestration-trigger-conditional-requirement/expected-code

@@ -0,0 +1 @@
+0

test/cli/pull/orchestration-trigger-conditional-requirement/expected-stderr

@@ -0,0 +1,4 @@
+Plan for "pull" operation:
+  + B main
+  + C main/application/kds-team.app-orchestration-trigger-queue-v2/trigger-fire-and-forget
+Pull done.

test/cli/pull/orchestration-trigger-conditional-requirement/expected-stdout

@@ -0,0 +1,26 @@
+{
+  "version": 2,
+  "project": {
+    "id": %%TEST_KBC_PROJECT_ID%%,
+    "apiHost": "%%TEST_KBC_STORAGE_API_HOST%%"
+  },
+  "allowTargetEnv": false,
+  "sortBy": "id",
+  "naming": {
+    "branch": "{branch_name}",
+    "config": "{component_type}/{component_id}/{config_name}",
+    "configRow": "rows/{config_row_name}",
+    "schedulerConfig": "schedules/{config_name}",
+    "sharedCodeConfig": "_shared/{target_component_id}",
+    "sharedCodeConfigRow": "codes/{config_row_name}",
+    "variablesConfig": "variables",
+    "variablesValuesRow": "values/{config_row_name}",
+    "dataAppConfig": "app/{component_id}/{config_name}"
+  },
+  "allowedBranches": [
+    "*"
+  ],
+  "ignoredComponents": [],
+  "branches": [],
+  "configurations": []
+}

test/cli/pull/orchestration-trigger-conditional-requirement/in/.keboola/manifest.json

@@ -0,0 +1,13 @@
+{
+  "allBranchesConfigs": [
+    "trigger-fire-and-forget"
+  ],
+  "branches": [
+    {
+      "branch": {
+        "name": "Main",
+        "isDefault": true
+      }
+    }
+  ]
+}