From 2d22a4d5fd1d01426dc0f2cf8446ae1bc8575121 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 12 Dec 2024 15:14:23 -0500 Subject: [PATCH 1/2] internal/schemavalidator: Fix bug where unknown values were returning error diagnostics too early --- internal/schemavalidator/also_requires.go | 3 +- .../schemavalidator/also_requires_test.go | 27 ++++++++++++++++++ internal/schemavalidator/at_least_one_of.go | 1 + .../schemavalidator/at_least_one_of_test.go | 28 +++++++++++++++++++ internal/schemavalidator/conflicts_with.go | 3 +- .../schemavalidator/conflicts_with_test.go | 27 ++++++++++++++++++ .../schemavalidator/exactly_one_of_test.go | 27 ++++++++++++++++++ 7 files changed, 114 insertions(+), 2 deletions(-) diff --git a/internal/schemavalidator/also_requires.go b/internal/schemavalidator/also_requires.go index 2f4662a7..f2de2828 100644 --- a/internal/schemavalidator/also_requires.go +++ b/internal/schemavalidator/also_requires.go @@ -58,7 +58,8 @@ func (av AlsoRequiresValidator) MarkdownDescription(_ context.Context) string { func (av AlsoRequiresValidator) Validate(ctx context.Context, req AlsoRequiresValidatorRequest, res *AlsoRequiresValidatorResponse) { // If attribute configuration is null, there is nothing else to validate - if req.ConfigValue.IsNull() { + // If attribute configuration is unknown, delay the validation until it is known. + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { return } diff --git a/internal/schemavalidator/also_requires_test.go b/internal/schemavalidator/also_requires_test.go index 6c7da078..3ea0d40b 100644 --- a/internal/schemavalidator/also_requires_test.go +++ b/internal/schemavalidator/also_requires_test.go @@ -80,6 +80,33 @@ func TestAlsoRequiresValidatorValidate(t *testing.T) { path.MatchRoot("foo"), }, }, + "self-is-unknown": { + req: schemavalidator.AlsoRequiresValidatorRequest{ + ConfigValue: types.StringUnknown(), + Path: path.Root("bar"), + PathExpression: path.MatchRoot("bar"), + Config: tfsdk.Config{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "foo": schema.Int64Attribute{}, + "bar": schema.StringAttribute{}, + }, + }, + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "foo": tftypes.Number, + "bar": tftypes.String, + }, + }, map[string]tftypes.Value{ + "foo": tftypes.NewValue(tftypes.Number, nil), + "bar": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + }), + }, + }, + in: path.Expressions{ + path.MatchRoot("foo"), + }, + }, "error_missing-one": { req: schemavalidator.AlsoRequiresValidatorRequest{ ConfigValue: types.StringValue("bar value"), diff --git a/internal/schemavalidator/at_least_one_of.go b/internal/schemavalidator/at_least_one_of.go index fa932ec2..c68ea841 100644 --- a/internal/schemavalidator/at_least_one_of.go +++ b/internal/schemavalidator/at_least_one_of.go @@ -58,6 +58,7 @@ func (av AtLeastOneOfValidator) MarkdownDescription(_ context.Context) string { func (av AtLeastOneOfValidator) Validate(ctx context.Context, req AtLeastOneOfValidatorRequest, res *AtLeastOneOfValidatorResponse) { // If attribute configuration is not null, validator already succeeded. + // If attribute configuration is unknown, delay the validation until it is known. if !req.ConfigValue.IsNull() { return } diff --git a/internal/schemavalidator/at_least_one_of_test.go b/internal/schemavalidator/at_least_one_of_test.go index b9a88e34..ce2122cf 100644 --- a/internal/schemavalidator/at_least_one_of_test.go +++ b/internal/schemavalidator/at_least_one_of_test.go @@ -84,6 +84,34 @@ func TestAtLeastOneOfValidatorValidate(t *testing.T) { }, expected: &schemavalidator.AtLeastOneOfValidatorResponse{}, }, + "self-is-unknown": { + req: schemavalidator.AtLeastOneOfValidatorRequest{ + ConfigValue: types.StringUnknown(), + Path: path.Root("bar"), + PathExpression: path.MatchRoot("bar"), + Config: tfsdk.Config{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "foo": schema.Int64Attribute{}, + "bar": schema.StringAttribute{}, + }, + }, + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "foo": tftypes.Number, + "bar": tftypes.String, + }, + }, map[string]tftypes.Value{ + "foo": tftypes.NewValue(tftypes.Number, 42), + "bar": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + }), + }, + }, + in: path.Expressions{ + path.MatchRoot("foo"), + }, + expected: &schemavalidator.AtLeastOneOfValidatorResponse{}, + }, "error_none-set": { req: schemavalidator.AtLeastOneOfValidatorRequest{ ConfigValue: types.StringNull(), diff --git a/internal/schemavalidator/conflicts_with.go b/internal/schemavalidator/conflicts_with.go index fd183893..9cbc096c 100644 --- a/internal/schemavalidator/conflicts_with.go +++ b/internal/schemavalidator/conflicts_with.go @@ -58,7 +58,8 @@ func (av ConflictsWithValidator) MarkdownDescription(_ context.Context) string { func (av ConflictsWithValidator) Validate(ctx context.Context, req ConflictsWithValidatorRequest, res *ConflictsWithValidatorResponse) { // If attribute configuration is null, it cannot conflict with others - if req.ConfigValue.IsNull() { + // If attribute configuration is unknown, delay the validation until it is known. + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { return } diff --git a/internal/schemavalidator/conflicts_with_test.go b/internal/schemavalidator/conflicts_with_test.go index 92b9c37b..faa45a9c 100644 --- a/internal/schemavalidator/conflicts_with_test.go +++ b/internal/schemavalidator/conflicts_with_test.go @@ -139,6 +139,33 @@ func TestConflictsWithValidatorValidate(t *testing.T) { path.MatchRoot("foo"), }, }, + "self-is-unknown": { + req: schemavalidator.ConflictsWithValidatorRequest{ + ConfigValue: types.StringUnknown(), + Path: path.Root("bar"), + PathExpression: path.MatchRoot("bar"), + Config: tfsdk.Config{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "foo": schema.Int64Attribute{}, + "bar": schema.StringAttribute{}, + }, + }, + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "foo": tftypes.Number, + "bar": tftypes.String, + }, + }, map[string]tftypes.Value{ + "foo": tftypes.NewValue(tftypes.Number, 42), + "bar": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + }), + }, + }, + in: path.Expressions{ + path.MatchRoot("foo"), + }, + }, "error_allow-duplicate-input": { req: schemavalidator.ConflictsWithValidatorRequest{ ConfigValue: types.StringValue("bar value"), diff --git a/internal/schemavalidator/exactly_one_of_test.go b/internal/schemavalidator/exactly_one_of_test.go index c0c82b6d..cb6dd1b2 100644 --- a/internal/schemavalidator/exactly_one_of_test.go +++ b/internal/schemavalidator/exactly_one_of_test.go @@ -81,6 +81,33 @@ func TestExactlyOneOfValidator(t *testing.T) { path.MatchRoot("foo"), }, }, + "self-is-unknown": { + req: schemavalidator.ExactlyOneOfValidatorRequest{ + ConfigValue: types.StringUnknown(), + Path: path.Root("bar"), + PathExpression: path.MatchRoot("bar"), + Config: tfsdk.Config{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "foo": schema.Int64Attribute{}, + "bar": schema.StringAttribute{}, + }, + }, + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "foo": tftypes.Number, + "bar": tftypes.String, + }, + }, map[string]tftypes.Value{ + "foo": tftypes.NewValue(tftypes.Number, 42), + "bar": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + }), + }, + }, + in: path.Expressions{ + path.MatchRoot("foo"), + }, + }, "error_too-many": { req: schemavalidator.ExactlyOneOfValidatorRequest{ ConfigValue: types.StringValue("bar value"), From 09325a7f41dd0a188f0535fe189f8fdcc1b2c614 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 12 Dec 2024 15:21:32 -0500 Subject: [PATCH 2/2] changelog --- .changes/unreleased/BUG FIXES-20241212-152125.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/BUG FIXES-20241212-152125.yaml diff --git a/.changes/unreleased/BUG FIXES-20241212-152125.yaml b/.changes/unreleased/BUG FIXES-20241212-152125.yaml new file mode 100644 index 00000000..f54b9f04 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20241212-152125.yaml @@ -0,0 +1,6 @@ +kind: BUG FIXES +body: Fixed bug with `ConflictsWith` and `AlsoRequires` validators where unknown values + would raise invalid diagnostics during `terraform validate`. +time: 2024-12-12T15:21:25.964001-05:00 +custom: + Issue: "251"