-
Notifications
You must be signed in to change notification settings - Fork 196
chore: Uses NewUnknownReplacements meant to replace schemafunc.CopyUnknowns
logic and schemafunc.NewAttributeChanges
#3192
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
base: master
Are you sure you want to change the base?
Changes from 29 commits
b04314c
ed85e27
9ffd580
10d8a82
19a9f90
a2bac7b
cbd65fb
7e4004b
3e8b7d0
3cc4e05
77eb77b
dda909b
b728367
d9c59d4
ce1e5c5
9d3f769
cbf31d9
4d70f84
2233dc5
ec64b97
9684396
938c6b4
7e22345
8d9093b
d710412
3bbec61
9e37725
c8ce3cd
c1ac25b
19d305b
7229da8
0fe2780
8d87c23
a714bdb
47aa17f
df15b83
13825e5
bc6aeb1
02cae8f
e958361
10ebaf4
d7deeb3
40a66a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
package conversion | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/attr" | ||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/path" | ||
"github.com/hashicorp/terraform-plugin-go/tftypes" | ||
) | ||
|
||
type TPFSchema interface { | ||
TypeAtTerraformPath(context.Context, *tftypes.AttributePath) (attr.Type, error) | ||
} | ||
|
||
type TPFSrc interface { | ||
GetAttribute(context.Context, path.Path, any) diag.Diagnostics | ||
} | ||
|
||
// AttributePathValue retrieves the value for src (state/plan/config) @ attributePath with converted path.Path, schema is needed to get the correct types.XXX (String/Object/etc.) | ||
func AttributePathValue(ctx context.Context, diags *diag.Diagnostics, attributePath *tftypes.AttributePath, src TPFSrc, schema TPFSchema) (attr.Value, path.Path) { | ||
convertedPath, localDiags := AttributePath(ctx, attributePath, schema) | ||
diags.Append(localDiags...) | ||
if diags.HasError() { | ||
return nil, convertedPath | ||
} | ||
attrType, err := schema.TypeAtTerraformPath(ctx, attributePath) | ||
if err != nil { | ||
diags.AddError("Unable to get type for attribute path", fmt.Sprintf("%s: %s", attributePath.String(), err)) | ||
return nil, convertedPath | ||
} | ||
attrValue := attrType.ValueType(ctx) | ||
if localDiags := src.GetAttribute(ctx, convertedPath, &attrValue); localDiags.HasError() { | ||
diags.Append(localDiags...) | ||
return nil, convertedPath | ||
} | ||
return attrValue, convertedPath | ||
} | ||
|
||
// AttributePath similar to the internal function in TPF, but simpler interface as argument and less logging | ||
maastha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func AttributePath(ctx context.Context, tfType *tftypes.AttributePath, schema TPFSchema) (path.Path, diag.Diagnostics) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't gone deep, but it smells a lot we're copying internal methods. Also, what is the impact on the policy? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we get any additional information here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Waiting on reply. |
||
fwPath := path.Empty() | ||
for tfTypeStepIndex, tfTypeStep := range tfType.Steps() { | ||
currentTfTypeSteps := tfType.Steps()[:tfTypeStepIndex+1] | ||
currentTfTypePath := tftypes.NewAttributePathWithSteps(currentTfTypeSteps) | ||
attrType, err := schema.TypeAtTerraformPath(ctx, currentTfTypePath) | ||
|
||
if err != nil { | ||
return path.Empty(), diag.Diagnostics{ | ||
diag.NewErrorDiagnostic( | ||
"Unable to Convert Attribute Path", | ||
"An unexpected error occurred while trying to convert an attribute path. "+ | ||
"This is an error in terraform-plugin-framework used by the provider. "+ | ||
"Please report the following to the provider developers.\n\n"+ | ||
// Since this is an error with the attribute path | ||
// conversion, we cannot return a protocol path-based | ||
// diagnostic. Returning a framework human-readable | ||
// representation seems like the next best thing to do. | ||
fmt.Sprintf("Attribute Path: %s\n", currentTfTypePath.String())+ | ||
fmt.Sprintf("Original Error: %s", err), | ||
), | ||
} | ||
} | ||
|
||
fwStep, err := AttributePathStep(ctx, tfTypeStep, attrType) | ||
|
||
if err != nil { | ||
return path.Empty(), diag.Diagnostics{ | ||
diag.NewErrorDiagnostic( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider having a helper func for the 3 errors in the func |
||
"Unable to Convert Attribute Path", | ||
"An unexpected error occurred while trying to convert an attribute path. "+ | ||
"This is either an error in terraform-plugin-framework or a custom attribute type used by the provider. "+ | ||
"Please report the following to the provider developers.\n\n"+ | ||
// Since this is an error with the attribute path | ||
// conversion, we cannot return a protocol path-based | ||
// diagnostic. Returning a framework human-readable | ||
// representation seems like the next best thing to do. | ||
fmt.Sprintf("Attribute Path: %s\n", currentTfTypePath.String())+ | ||
fmt.Sprintf("Original Error: %s", err), | ||
), | ||
} | ||
} | ||
|
||
// In lieu of creating a path.NewPathFromSteps function, this path | ||
// building logic is inlined to not expand the path package API. | ||
switch fwStep := fwStep.(type) { | ||
case path.PathStepAttributeName: | ||
fwPath = fwPath.AtName(string(fwStep)) | ||
case path.PathStepElementKeyInt: | ||
fwPath = fwPath.AtListIndex(int(fwStep)) | ||
case path.PathStepElementKeyString: | ||
fwPath = fwPath.AtMapKey(string(fwStep)) | ||
case path.PathStepElementKeyValue: | ||
fwPath = fwPath.AtSetValue(fwStep.Value) | ||
default: | ||
return fwPath, diag.Diagnostics{ | ||
diag.NewErrorDiagnostic( | ||
"Unable to Convert Attribute Path", | ||
"An unexpected error occurred while trying to convert an attribute path. "+ | ||
"This is an error in terraform-plugin-framework used by the provider. "+ | ||
"Please report the following to the provider developers.\n\n"+ | ||
// Since this is an error with the attribute path | ||
// conversion, we cannot return a protocol path-based | ||
// diagnostic. Returning a framework human-readable | ||
// representation seems like the next best thing to do. | ||
fmt.Sprintf("Attribute Path: %s\n", currentTfTypePath.String())+ | ||
fmt.Sprintf("Original Error: unknown path.PathStep type: %#v", fwStep), | ||
), | ||
} | ||
} | ||
} | ||
|
||
return fwPath, nil | ||
} | ||
|
||
func AttributePathStep(ctx context.Context, tfType tftypes.AttributePathStep, attrType attr.Type) (path.PathStep, error) { | ||
maastha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
switch tfType := tfType.(type) { | ||
case tftypes.AttributeName: | ||
return path.PathStepAttributeName(string(tfType)), nil | ||
case tftypes.ElementKeyInt: | ||
return path.PathStepElementKeyInt(int64(tfType)), nil | ||
case tftypes.ElementKeyString: | ||
return path.PathStepElementKeyString(string(tfType)), nil | ||
case tftypes.ElementKeyValue: | ||
attrValue, err := Value(ctx, tftypes.Value(tfType), attrType) | ||
|
||
if err != nil { | ||
return nil, fmt.Errorf("unable to create PathStepElementKeyValue from tftypes.Value: %w", err) | ||
} | ||
|
||
return path.PathStepElementKeyValue{Value: attrValue}, nil | ||
default: | ||
return nil, fmt.Errorf("unknown tftypes.AttributePathStep: %#v", tfType) | ||
} | ||
} | ||
|
||
func Value(ctx context.Context, tfType tftypes.Value, attrType attr.Type) (attr.Value, error) { | ||
if attrType == nil { | ||
return nil, fmt.Errorf("unable to convert tftypes.Value (%s) to attr.Value: missing attr.Type", tfType.String()) | ||
} | ||
|
||
attrValue, err := attrType.ValueFromTerraform(ctx, tfType) | ||
|
||
if err != nil { | ||
return nil, fmt.Errorf("unable to convert tftypes.Value (%s) to attr.Value: %w", tfType.String(), err) | ||
} | ||
|
||
return attrValue, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
package conversion | ||
|
||
import ( | ||
"fmt" | ||
"strings" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/path" | ||
) | ||
|
||
func IsListIndex(p path.Path) bool { | ||
lastPart := lastPart(p) | ||
if IsMapIndex(p) || IsSetIndex(p) { | ||
return false | ||
} | ||
return strings.HasSuffix(lastPart, "]") | ||
} | ||
|
||
func IsMapIndex(p path.Path) bool { | ||
lastPart := lastPart(p) | ||
return strings.HasSuffix(lastPart, "\"]") | ||
} | ||
|
||
func IsSetIndex(p path.Path) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you tell more about IsSetIndex and why is different from the rest, here we do Contains instead of HasSuffix. Can you give an example of set index to see how it looks like and how the suffix is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example: Want to separate IsListIndex from IsMapIndex and IsSetIndex. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks, so I don't still understand why we want to differentiate IsSetIndex and IsMapIndex, can we just have IsMapIndex? what is a use case where we want to treat them differently? (also note IsMapIndex is not correct bc you have to do something like in IsListIndex impl., say if IsSetIndex(p) return false) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a subtle difference:
See also test case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, I missed that different end, thanks. and when do we want to treat them differently? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So far |
||
lastPart := lastPart(p) | ||
return strings.Contains(lastPart, "[Value(") | ||
} | ||
|
||
func HasAncestor(p, ancestor path.Path) bool { | ||
prefixString := ancestor.String() | ||
pString := p.String() | ||
return strings.HasPrefix(pString, prefixString) | ||
} | ||
|
||
func AttributeName(p path.Path) string { | ||
noIndex := trimLastIndex(p) | ||
parts := strings.Split(noIndex, ".") | ||
return parts[len(parts)-1] | ||
} | ||
|
||
// AsAddedIndex returns "" if the path is not an index otherwise it adds `+` before the index | ||
func AsAddedIndex(p path.Path) string { | ||
if !isIndexValue(p) { | ||
return "" | ||
} | ||
lastPart := lastPart(p) | ||
indexWithSign := strings.Replace(lastPart, "[", "[+", 1) | ||
everythingExceptLast, _ := strings.CutSuffix(p.String(), lastPart) | ||
return everythingExceptLast + indexWithSign | ||
} | ||
|
||
// AsRemovedIndex returns "" if the path is not an index otherwise it adds `-` before the index | ||
func AsRemovedIndex(p path.Path) string { | ||
if !isIndexValue(p) { | ||
return "" | ||
} | ||
lastPart := lastPart(p) | ||
lastPartWithRemoveIndex := strings.Replace(lastPart, "[", "[-", 1) | ||
everythingExceptLast, _ := strings.CutSuffix(p.String(), lastPart) | ||
return everythingExceptLast + lastPartWithRemoveIndex | ||
} | ||
|
||
func AncestorPathWithIndex(p path.Path, attributeName string, diags *diag.Diagnostics) path.Path { | ||
for { | ||
EspenAlbert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
p = p.ParentPath() | ||
if p.Equal(path.Empty()) { | ||
diags.AddError("Parent path not found", fmt.Sprintf("Parent attribute %s not found in path %s", attributeName, p.String())) | ||
return p | ||
} | ||
if AttributeName(p) == attributeName { | ||
return p | ||
} | ||
} | ||
} | ||
|
||
func AncestorPathNoIndex(p path.Path, attributeName string, diags *diag.Diagnostics) path.Path { | ||
parent := AncestorPathWithIndex(p, attributeName, diags) | ||
if diags.HasError() { | ||
return parent | ||
} | ||
return trimLastIndexPath(parent) | ||
} | ||
|
||
func AncestorPaths(p path.Path) []path.Path { | ||
ancestors := []path.Path{} | ||
for { | ||
ancestor := p.ParentPath() | ||
if ancestor.Equal(path.Empty()) { | ||
return ancestors | ||
} | ||
ancestors = append(ancestors, ancestor) | ||
p = ancestor | ||
} | ||
} | ||
|
||
func lastPart(p path.Path) string { | ||
parts := strings.Split(p.String(), ".") | ||
return parts[len(parts)-1] | ||
} | ||
|
||
func isIndexValue(p path.Path) bool { | ||
return IsMapIndex(p) || IsListIndex(p) || IsSetIndex(p) | ||
} | ||
|
||
func trimLastIndex(p path.Path) string { | ||
return trimLastIndexPath(p).String() | ||
} | ||
|
||
func trimLastIndexPath(p path.Path) path.Path { | ||
if isIndexValue(p) { | ||
return p.ParentPath() | ||
} | ||
return p | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
package conversion_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/path" | ||
"github.com/hashicorp/terraform-plugin-framework/types" | ||
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestIsIndexTypes(t *testing.T) { | ||
listIndexPath := path.Root("replication_specs").AtListIndex(0) | ||
mapIndexPath := path.Root("replication_specs").AtMapKey("myKey") | ||
setIndexPath := path.Root("replication_specs").AtSetValue(types.StringValue("myKey")) | ||
assert.True(t, conversion.IsListIndex(listIndexPath)) | ||
assert.False(t, conversion.IsListIndex(setIndexPath)) | ||
assert.False(t, conversion.IsListIndex(mapIndexPath)) | ||
|
||
assert.True(t, conversion.IsSetIndex(setIndexPath)) | ||
assert.False(t, conversion.IsSetIndex(mapIndexPath)) | ||
assert.False(t, conversion.IsSetIndex(listIndexPath)) | ||
|
||
assert.True(t, conversion.IsMapIndex(mapIndexPath)) | ||
assert.False(t, conversion.IsMapIndex(setIndexPath)) | ||
assert.False(t, conversion.IsMapIndex(listIndexPath)) | ||
} | ||
|
||
func TestIndexMethods(t *testing.T) { | ||
assert.True(t, conversion.IsListIndex(path.Root("replication_specs").AtListIndex(0))) | ||
assert.False(t, conversion.IsListIndex(path.Root("replication_specs").AtName("region_configs"))) | ||
assert.False(t, conversion.IsListIndex(path.Root("replication_specs").AtMapKey("region_configs"))) | ||
assert.Equal(t, "replication_specs[+0]", conversion.AsAddedIndex(path.Root("replication_specs").AtListIndex(0))) | ||
assert.Equal(t, "replication_specs[0].region_configs[+1]", conversion.AsAddedIndex(path.Root("replication_specs").AtListIndex(0).AtName("region_configs").AtListIndex(1))) | ||
assert.Equal(t, "replication_specs[+\"myKey\"]", conversion.AsAddedIndex(path.Root("replication_specs").AtMapKey("myKey"))) | ||
assert.Equal(t, "replication_specs[+Value(\"myKey\")]", conversion.AsAddedIndex(path.Root("replication_specs").AtSetValue(types.StringValue("myKey")))) | ||
assert.Equal(t, "replication_specs[-1]", conversion.AsRemovedIndex(path.Root("replication_specs").AtListIndex(1))) | ||
assert.Equal(t, "replication_specs[0].region_configs[-1]", conversion.AsRemovedIndex(path.Root("replication_specs").AtListIndex(0).AtName("region_configs").AtListIndex(1))) | ||
assert.Equal(t, "replication_specs[-\"myKey\"]", conversion.AsRemovedIndex(path.Root("replication_specs").AtMapKey("myKey"))) | ||
assert.Equal(t, "replication_specs[-Value(\"myKey\")]", conversion.AsRemovedIndex(path.Root("replication_specs").AtSetValue(types.StringValue("myKey")))) | ||
setIndex := path.Root("advanced_configuration").AtName("custom_openssl_cipher_config_tls12").AtSetValue(types.StringValue("TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384")) | ||
assert.Equal(t, "advanced_configuration.custom_openssl_cipher_config_tls12[-Value(\"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384\")]", conversion.AsRemovedIndex(setIndex)) | ||
assert.Equal(t, "advanced_configuration.custom_openssl_cipher_config_tls12", conversion.AncestorPathNoIndex(setIndex, "custom_openssl_cipher_config_tls12", new(diag.Diagnostics)).String()) | ||
assert.Equal(t, "", conversion.AsRemovedIndex(path.Root("replication_specs"))) | ||
} | ||
|
||
func TestHasAncestor(t *testing.T) { | ||
prefix := path.Root("replication_specs").AtListIndex(0) | ||
assert.True(t, conversion.HasAncestor(path.Root("replication_specs").AtListIndex(0), prefix)) | ||
assert.True(t, conversion.HasAncestor(path.Root("replication_specs").AtListIndex(0).AtName("region_configs"), prefix)) | ||
assert.False(t, conversion.HasAncestor(path.Root("replication_specs").AtListIndex(1), prefix)) | ||
assert.True(t, conversion.HasAncestor(path.Root("replication_specs").AtListIndex(0).AtName("region_configs").AtListIndex(1), path.Empty())) | ||
} | ||
|
||
func TestParentPathWithIndex_Found(t *testing.T) { | ||
diags := new(diag.Diagnostics) | ||
// Build a nested path: resource -> parent -> child | ||
basePath := path.Root("resource") | ||
parentPath := basePath.AtName("parent") | ||
childPath := parentPath.AtName("child") | ||
|
||
assert.Equal(t, parentPath.String(), conversion.AncestorPathWithIndex(childPath, "parent", diags).String()) | ||
assert.Equal(t, basePath.String(), conversion.AncestorPathWithIndex(childPath, "resource", diags).String()) | ||
assert.Empty(t, diags, "Diagnostics should not have errors") | ||
} | ||
|
||
func TestParentPathWithIndex_FoundIncludesIndex(t *testing.T) { | ||
diags := new(diag.Diagnostics) | ||
// Build a nested path: resource[0] -> parent[0] -> child | ||
basePath := path.Root("resource") | ||
parentPath := basePath.AtListIndex(0).AtName("parent") | ||
childPath := parentPath.AtListIndex(0).AtName("child") | ||
assert.Equal(t, "resource[0].parent[0].child", childPath.String()) | ||
|
||
assert.Equal(t, parentPath.AtListIndex(0).String(), conversion.AncestorPathWithIndex(childPath, "parent", diags).String()) | ||
assert.Equal(t, basePath.AtListIndex(0).String(), conversion.AncestorPathWithIndex(childPath, "resource", diags).String()) | ||
assert.Empty(t, diags, "Diagnostics should not have errors") | ||
} | ||
|
||
func TestParentPathNoIndex_RemovesIndex(t *testing.T) { | ||
diags := new(diag.Diagnostics) | ||
// Build a nested path: resource[0] -> parent[0] -> child | ||
basePath := path.Root("resource") | ||
parentPath := basePath.AtListIndex(0).AtName("parent") | ||
childPath := parentPath.AtListIndex(0).AtName("child") | ||
assert.Equal(t, "resource[0].parent[0].child", childPath.String()) | ||
|
||
assert.Equal(t, parentPath.String(), conversion.AncestorPathNoIndex(childPath, "parent", diags).String()) | ||
assert.Equal(t, basePath.String(), conversion.AncestorPathNoIndex(childPath, "resource", diags).String()) | ||
assert.Empty(t, diags, "Diagnostics should not have errors") | ||
} | ||
|
||
func TestParentPathWithIndex_NotFound(t *testing.T) { | ||
diags := new(diag.Diagnostics) | ||
// Build a path: resource -> child | ||
basePath := path.Root("resource") | ||
childPath := basePath.AtName("child") | ||
|
||
result := conversion.AncestorPathWithIndex(childPath, "nonexistent", diags) | ||
// The function should traverse to path.Empty() and add an error. | ||
assert.True(t, result.Equal(path.Empty()), "Expected result to be empty if parent not found") | ||
assert.True(t, diags.HasError(), "Diagnostics should have an error when parent attribute is missing") | ||
} | ||
|
||
func TestParentPathWithIndex_EmptyPath(t *testing.T) { | ||
diags := new(diag.Diagnostics) | ||
emptyPath := path.Empty() | ||
result := conversion.AncestorPathWithIndex(emptyPath, "any", diags) | ||
// Since the path is empty, it should immediately return empty and add error. | ||
assert.True(t, result.Equal(path.Empty()), "Expected empty path as result from an empty input path") | ||
assert.True(t, diags.HasError(), "Diagnostics should have an error for empty input path") | ||
} |
Uh oh!
There was an error while loading. Please reload this page.