Db/fix predicate validation bug (#547)

* preserve predicate value unknown state in PredicateObjectToModel * add changie log * config validation throws warnings, not errors * replace more validation errors with warnings
OpsLevel · Dec 13, 2024 · 68cbbb0 · 68cbbb0
1 parent decd87c
commit 68cbbb0
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 18 deletions.
diff --git a/.changes/unreleased/Bugfix-20241212-155300.yaml b/.changes/unreleased/Bugfix-20241212-155300.yaml
@@ -0,0 +1,3 @@
+kind: Bugfix
+body: fix incorrect validation error on predicate values set by for_each values
+time: 2024-12-12T15:53:00.747508-06:00
diff --git a/Taskfile.yml b/Taskfile.yml
@@ -114,6 +114,12 @@ tasks:
     cmds:
       - task: debug:do-debug-plan
 
+  debug-validate:
+    desc: After debug-start and debug-attach, run "terraform validate" with 'TF_REATTACH_PROVIDERS'
+    aliases: ["dbg-validate"]
+    cmds:
+      - task: debug:do-debug-validate
+
   debug-test:
     desc: After debug-start and debug-attach, run "terraform test" with 'TF_REATTACH_PROVIDERS'
     aliases: ["dbg-test"]

diff --git a/opslevel/resource_opslevel_check_alert_source_usage.go b/opslevel/resource_opslevel_check_alert_source_usage.go
@@ -166,7 +166,7 @@ func (r *CheckAlertSourceUsageResource) ValidateConfig(ctx context.Context, req
 	predicateModel, diags := PredicateObjectToModel(ctx, alertNamePredicate)
 	resp.Diagnostics.Append(diags...)
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("alert_name_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("alert_name_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_package_version.go b/opslevel/resource_opslevel_check_package_version.go
@@ -156,28 +156,48 @@ func (r *CheckPackageVersionResource) ValidateConfig(ctx context.Context, req re
 
 	if configModel.PackageConstraint.ValueString() == string(opslevel.PackageConstraintEnumMatchesVersion) {
 		if configModel.MissingPackageResult.IsNull() && !configModel.MissingPackageResult.IsUnknown() {
-			resp.Diagnostics.AddError("missing_package_result", "missing_package_result is required when package_constraint is 'matches_version'")
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("missing_package_result"),
+				"Invalid Configuration",
+				"missing_package_result is required when package_constraint is 'matches_version'",
+			)
 		}
 		if configModel.VersionConstraintPredicate.IsNull() && !configModel.VersionConstraintPredicate.IsUnknown() {
-			resp.Diagnostics.AddError("version_constraint_predicate", "version_constraint_predicate is required when package_constraint is 'matches_version'")
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("version_constraint_predicate"),
+				"Invalid Configuration",
+				"version_constraint_predicate is required when package_constraint is 'matches_version'",
+			)
 		}
 		if !configModel.VersionConstraintPredicate.IsNull() {
 			predicateModel, diags := PredicateObjectToModel(ctx, configModel.VersionConstraintPredicate)
 			resp.Diagnostics.Append(diags...)
 			if !slices.Contains(packageVersionPossiblePredicateTypes, opslevel.PredicateTypeEnum(predicateModel.Type.ValueString())) {
-				resp.Diagnostics.AddError("version_constraint_predicate", fmt.Sprintf("version_constraint_predicate must be one of %v", packageVersionPossiblePredicateTypes))
+				resp.Diagnostics.AddAttributeWarning(
+					path.Root("version_constraint_predicate"),
+					"Invalid Configuration",
+					fmt.Sprintf("version_constraint_predicate must be one of %v", packageVersionPossiblePredicateTypes),
+				)
 			} else {
 				if err := predicateModel.Validate(); err != nil {
-					resp.Diagnostics.AddAttributeError(path.Root("version_constraint_predicate"), "Invalid Configuration", err.Error())
+					resp.Diagnostics.AddAttributeWarning(path.Root("version_constraint_predicate"), "Invalid Configuration", err.Error())
 				}
 			}
 		}
 	} else {
 		if !configModel.MissingPackageResult.IsUnknown() && !configModel.MissingPackageResult.IsNull() {
-			resp.Diagnostics.AddError("missing_package_result", "missing_package_result is only valid when package_constraint is 'matches_version'")
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("missing_package_result"),
+				"Invalid Configuration",
+				"missing_package_result is only valid when package_constraint is 'matches_version'",
+			)
 		}
 		if !configModel.VersionConstraintPredicate.IsUnknown() && !configModel.VersionConstraintPredicate.IsNull() {
-			resp.Diagnostics.AddError("version_constraint_predicate", "version_constraint_predicate is only valid when package_constraint is 'matches_version'")
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("version_constraint_predicate"),
+				"Invalid Configuration",
+				"version_constraint_predicate is only valid when package_constraint is 'matches_version'",
+			)
 		}
 	}
 }

diff --git a/opslevel/resource_opslevel_check_repository_file.go b/opslevel/resource_opslevel_check_repository_file.go
@@ -193,7 +193,7 @@ func (r *CheckRepositoryFileResource) ValidateConfig(ctx context.Context, req re
 	predicateModel, diags := PredicateObjectToModel(ctx, fileContentsPredicate)
 	resp.Diagnostics.Append(diags...)
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_repository_grep.go b/opslevel/resource_opslevel_check_repository_grep.go
@@ -184,10 +184,14 @@ func (r *CheckRepositoryGrepResource) ValidateConfig(ctx context.Context, req re
 	resp.Diagnostics.Append(diags...)
 
 	if configModel.DirectorySearch.ValueBool() && !slices.Contains([]string{"exists", "does_not_exist"}, predicateModel.Type.ValueString()) {
-		resp.Diagnostics.AddError("Config Error", "When 'directory_search' is true, file_contents_predicate type must be 'exists' or 'does_not_exist'")
+		resp.Diagnostics.AddAttributeWarning(
+			path.Root("directory_search"),
+			"Invalid Attribute Configuration",
+			"When 'directory_search' is true, 'file_contents_predicate' type must be 'exists' or 'does_not_exist'",
+		)
 	}
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_repository_search.go b/opslevel/resource_opslevel_check_repository_search.go
@@ -185,10 +185,10 @@ func (r *CheckRepositorySearchResource) ValidateConfig(ctx context.Context, req
 	predicateModel, diags := PredicateObjectToModel(ctx, fileContentsPredicate)
 	resp.Diagnostics.Append(diags...)
 	if predicateModel.Type.ValueString() == "exists" || predicateModel.Type.ValueString() == "does_not_exist" {
-		resp.Diagnostics.AddError("Config Error", "file_contents_predicate type must not be 'exists' or 'does_not_exist'")
+		resp.Diagnostics.AddAttributeWarning(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", "type must not be 'exists' or 'does_not_exist'")
 	}
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("file_contents_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_service_ownership.go b/opslevel/resource_opslevel_check_service_ownership.go
@@ -215,7 +215,7 @@ func (r *CheckServiceOwnershipResource) ValidateConfig(ctx context.Context, req
 	predicateModel, diags := PredicateObjectToModel(ctx, tagPredicate)
 	resp.Diagnostics.Append(diags...)
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("tag_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("tag_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_service_property.go b/opslevel/resource_opslevel_check_service_property.go
@@ -187,7 +187,7 @@ func (r *CheckServicePropertyResource) ValidateConfig(ctx context.Context, req r
 	predicateModel, diags := PredicateObjectToModel(ctx, predicate)
 	resp.Diagnostics.Append(diags...)
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_tag_defined.go b/opslevel/resource_opslevel_check_tag_defined.go
@@ -169,7 +169,7 @@ func (r *CheckTagDefinedResource) ValidateConfig(ctx context.Context, req resour
 	predicateModel, diags := PredicateObjectToModel(ctx, tagPredicate)
 	resp.Diagnostics.Append(diags...)
 	if err := predicateModel.Validate(); err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("tag_predicate"), "Invalid Attribute Configuration", err.Error())
+		resp.Diagnostics.AddAttributeWarning(path.Root("tag_predicate"), "Invalid Attribute Configuration", err.Error())
 	}
 }
 

diff --git a/opslevel/resource_opslevel_check_tool_usage.go b/opslevel/resource_opslevel_check_tool_usage.go
@@ -235,7 +235,7 @@ func (r *CheckToolUsageResource) ValidateConfig(ctx context.Context, req resourc
 		predicateModel, diags := PredicateObjectToModel(ctx, predicate)
 		resp.Diagnostics.Append(diags...)
 		if err := predicateModel.Validate(); err != nil {
-			resp.Diagnostics.AddAttributeError(path.Root(predicateSchemaName), "Invalid Attribute Configuration", err.Error())
+			resp.Diagnostics.AddAttributeWarning(path.Root(predicateSchemaName), "Invalid Attribute Configuration", err.Error())
 		}
 	}
 }

diff --git a/opslevel/resource_opslevel_filter.go b/opslevel/resource_opslevel_filter.go
@@ -247,7 +247,7 @@ func (r *FilterResource) ValidateConfig(ctx context.Context, req resource.Valida
 
 	for _, filterPredicate := range predicateModels {
 		if err := filterPredicate.Validate(); err != nil {
-			resp.Diagnostics.AddAttributeError(path.Root("predicate"), "Invalid Attribute Configuration", err.Error())
+			resp.Diagnostics.AddAttributeWarning(path.Root("predicate"), "Invalid Attribute Configuration", err.Error())
 		}
 	}
 }

diff --git a/opslevel/terraform_type_conversions.go b/opslevel/terraform_type_conversions.go
@@ -155,7 +155,7 @@ func PredicateObjectToModel(ctx context.Context, predicateObj basetypes.ObjectVa
 
 	objOptions := basetypes.ObjectAsOptions{UnhandledNullAsEmpty: true, UnhandledUnknownAsEmpty: true}
 	diags := predicateObj.As(ctx, &predicateModel, objOptions)
-	if predicateModel.Value.ValueString() == "" {
+	if !predicateModel.Value.IsUnknown() && predicateModel.Value.ValueString() == "" {
 		predicateModel.Value = types.StringNull()
 	}
 	return predicateModel, diags