diff --git a/.changelog/pr-127.txt b/.changelog/pr-127.txt new file mode 100644 index 0000000..d6b5985 --- /dev/null +++ b/.changelog/pr-127.txt @@ -0,0 +1,3 @@ +```release-note:bug +`resource/pingone_davinci_flow`: Fixed multi-outcome node routing (e.g. PingOne Forms nodes with multiple buttons/links) being dropped on export, which broke the flow when the exported HCL was re-applied. Requires `terraform-provider-pingone` with `outcomes` support on `graph_data.elements.nodes.*.data`. +``` diff --git a/definitions/pingone/davinci/flow.yaml b/definitions/pingone/davinci/flow.yaml index ab4966a..ba2a8a4 100644 --- a/definitions/pingone/davinci/flow.yaml +++ b/definitions/pingone/davinci/flow.yaml @@ -381,6 +381,43 @@ attributes: source_path: Properties transform: jsonencode_raw + # Outcomes: multi-outcome node routing (e.g. pingOneFormsConnector + # showForm "Save"/"Resend" exit paths). Only present on nodes whose + # connector/capability defines named exit paths; absent otherwise -- + # no attribute is emitted in that case. + # + # pingone-go-client v0.12.0+ has a typed Outcomes field on + # DaVinciFlowGraphDataResponseElementsNodeData, so this resolves via + # the normal named-field path in findFieldByPath. Older SDK versions + # (no typed field) fall back to AdditionalProperties["outcomes"] via + # the generic fallback in internal/core/processor.go -- see PR #127 + # for the full history. + # + # terraform-provider-pingone#1342 / PR #1345 added provider-side + # support for this attribute (merged to main, not yet in a tagged + # release as of PR #127 -- see that PR's description for the release + # status). Applying HCL with an outcomes block will fail against + # provider releases predating that fix. + - name: Outcomes + terraform_name: outcomes + type: list + source_path: Outcomes + nested_attributes: + - name: Id + terraform_name: id + type: string + source_path: Id + + - name: Result + terraform_name: result + type: string + source_path: Result + + - name: Label + terraform_name: label + type: string + source_path: Label + - name: Position terraform_name: position type: object diff --git a/go.mod b/go.mod index 29f8d50..19cadee 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/hashicorp/go-plugin v1.7.0 github.com/hashicorp/hcl/v2 v2.24.0 github.com/pingidentity/pingcli v0.8.0 - github.com/pingidentity/pingone-go-client v0.11.0 + github.com/pingidentity/pingone-go-client v0.12.0 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 github.com/zclconf/go-cty v1.18.0 @@ -16,17 +16,16 @@ require ( ) require ( - al.essio.dev/pkg/shellescape v1.5.1 // indirect github.com/agext/levenshtein v1.2.1 // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/danieljoos/wincred v1.2.2 // indirect + github.com/danieljoos/wincred v1.2.3 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/fatih/color v1.18.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect - github.com/godbus/dbus/v5 v5.1.0 // indirect + github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/hashicorp/go-hclog v1.6.3 // indirect @@ -37,12 +36,12 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/oklog/run v1.2.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/zalando/go-keyring v0.2.6 // indirect + github.com/zalando/go-keyring v0.2.8 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0 // indirect - go.opentelemetry.io/otel v1.41.0 // indirect - go.opentelemetry.io/otel/metric v1.41.0 // indirect - go.opentelemetry.io/otel/trace v1.41.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0 // indirect + go.opentelemetry.io/otel v1.44.0 // indirect + go.opentelemetry.io/otel/metric v1.44.0 // indirect + go.opentelemetry.io/otel/trace v1.44.0 // indirect golang.org/x/mod v0.35.0 // indirect golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect diff --git a/go.sum b/go.sum index 85292b3..10b09ae 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho= -al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890= github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY= @@ -8,8 +6,8 @@ github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/ github.com/bufbuild/protocompile v0.14.1/go.mod h1:ppVdAIhbr2H8asPk6k4pY7t9zB1OU5DoEw9xY/FUi1c= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0= -github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8= +github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= +github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -26,14 +24,12 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68= github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= -github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= -github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= +github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= @@ -66,8 +62,8 @@ github.com/oklog/run v1.2.0 h1:O8x3yXwah4A73hJdlrwo/2X6J62gE5qTMusH0dvz60E= github.com/oklog/run v1.2.0/go.mod h1:mgDbKRSwPhJfesJ4PntqFUbKQRZ50NgmZTSPlFA0YFk= github.com/pingidentity/pingcli v0.8.0 h1:KmUuzGliAtNlLz/okMRtT0vdRoiSAz3Kb+/FGti6rpA= github.com/pingidentity/pingcli v0.8.0/go.mod h1:3fAj8w0kZtLU6DBLKdCLQnPCN+gbVjysnwGrgsJoSYY= -github.com/pingidentity/pingone-go-client v0.11.0 h1:CtzkS2KDR9U6Z/GTeOngGSLu0hbkEeHvcl+m9mFquVI= -github.com/pingidentity/pingone-go-client v0.11.0/go.mod h1:BVVHSwjF4YHAsmvncysW56UQC8lnermeQH8WGpvNvTA= +github.com/pingidentity/pingone-go-client v0.12.0 h1:t9L18Qul3UXqqvi0578R7bzys54cEEl+oSJDT6rMdyM= +github.com/pingidentity/pingone-go-client v0.12.0/go.mod h1:ZFGQiCdcLdhDMnNZrefNY86Mb2wHH7GTWMDqv/PWKBk= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -81,26 +77,26 @@ github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s= -github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI= +github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= +github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= github.com/zclconf/go-cty v1.18.0 h1:pJ8+HNI4gFoyRNqVE37wWbJWVw43BZczFo7KUoRczaA= github.com/zclconf/go-cty v1.18.0/go.mod h1:qpnV6EDNgC1sns/AleL1fvatHw72j+S+nS+MJ+T2CSg= github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 h1:4r45xpDWB6ZMSMNJFMOjqrGHynW3DIBuR2H9j0ug+Mo= github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0 h1:7iP2uCb7sGddAr30RRS6xjKy7AZ2JtTOPA3oolgVSw8= -go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0/go.mod h1:c7hN3ddxs/z6q9xwvfLPk+UHlWRQyaeR1LdgfL/66l0= -go.opentelemetry.io/otel v1.41.0 h1:YlEwVsGAlCvczDILpUXpIpPSL/VPugt7zHThEMLce1c= -go.opentelemetry.io/otel v1.41.0/go.mod h1:Yt4UwgEKeT05QbLwbyHXEwhnjxNO6D8L5PQP51/46dE= -go.opentelemetry.io/otel/metric v1.41.0 h1:rFnDcs4gRzBcsO9tS8LCpgR0dxg4aaxWlJxCno7JlTQ= -go.opentelemetry.io/otel/metric v1.41.0/go.mod h1:xPvCwd9pU0VN8tPZYzDZV/BMj9CM9vs00GuBjeKhJps= -go.opentelemetry.io/otel/sdk v1.40.0 h1:KHW/jUzgo6wsPh9At46+h4upjtccTmuZCFAc9OJ71f8= -go.opentelemetry.io/otel/sdk v1.40.0/go.mod h1:Ph7EFdYvxq72Y8Li9q8KebuYUr2KoeyHx0DRMKrYBUE= -go.opentelemetry.io/otel/sdk/metric v1.40.0 h1:mtmdVqgQkeRxHgRv4qhyJduP3fYJRMX4AtAlbuWdCYw= -go.opentelemetry.io/otel/sdk/metric v1.40.0/go.mod h1:4Z2bGMf0KSK3uRjlczMOeMhKU2rhUqdWNoKcYrtcBPg= -go.opentelemetry.io/otel/trace v1.41.0 h1:Vbk2co6bhj8L59ZJ6/xFTskY+tGAbOnCtQGVVa9TIN0= -go.opentelemetry.io/otel/trace v1.41.0/go.mod h1:U1NU4ULCoxeDKc09yCWdWe+3QoyweJcISEVa1RBzOis= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0 h1:8tvICD4vSTOOsNrsI4Ljf6C+6UKvpTEH5XY3JMoyPoo= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0/go.mod h1:z9+yiacE0IHRqM4qFfkbt/JYlmYXgss8GY/jXoNuPJI= +go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= +go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= +go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= +go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= +go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= +go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= +go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= +go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= +go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= +go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= diff --git a/internal/core/davinci_flow_outcomes_test.go b/internal/core/davinci_flow_outcomes_test.go new file mode 100644 index 0000000..b6bcb68 --- /dev/null +++ b/internal/core/davinci_flow_outcomes_test.go @@ -0,0 +1,187 @@ +package core_test + +import ( + "strings" + "testing" + + "github.com/google/uuid" + pingone "github.com/pingidentity/pingone-go-client/pingone" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pingidentity/pingcli-plugin-terraformer/internal/core" + hclformatter "github.com/pingidentity/pingcli-plugin-terraformer/internal/formatters/hcl" + "github.com/pingidentity/pingcli-plugin-terraformer/internal/schema" +) + +// davinciFlowRegistry loads the real flow.yaml definition (not a synthetic +// mini-schema) so these tests catch drift between the AdditionalProperties +// fallback (Task 1) and the actual outcomes attribute declared in +// definitions/pingone/davinci/flow.yaml (Task 2). +func davinciFlowRegistry(t *testing.T) *schema.Registry { + t.Helper() + registry := schema.NewRegistry() + require.NoError(t, registry.LoadPlatform("../../definitions", "pingone")) + return registry +} + +// buildFlowResponse constructs a minimal but valid *pingone.DaVinciFlowResponse +// with two nodes in GraphData.Elements.Nodes: one whose Data carries a typed +// Outcomes slice (mirroring the sample flow's esqd1w6k6h node, requirements.md +// "Sample flow evidence"), and one without any outcomes (mirroring +// frbglr02tp), so a single processed resource proves both the presence and +// absence cases end-to-end through the real definition. +// +// pingone-go-client v0.12.0 added a typed Outcomes field to +// DaVinciFlowGraphDataResponseElementsNodeData (terraform-provider-pingone#1342, +// pingone-go-client#86, CDI-1370 all shipped/landed) -- outcomes is no longer +// SDK-untyped, so this constructs the field directly rather than stuffing it +// into AdditionalProperties. The AdditionalProperties fallback added in +// internal/core/processor.go for this field is now dormant (verified by the +// regression tests in processor_test.go), staying available for whatever +// SDK field next lags its JSON shape. +func buildFlowResponse() *pingone.DaVinciFlowResponse { + links := pingone.NewDaVinciFlowResponseLinks( + *pingone.NewJSONHALLink("https://example.com/environment"), + *pingone.NewJSONHALLink("https://example.com/self"), + *pingone.NewJSONHALLink("https://example.com/connectorInstances"), + *pingone.NewJSONHALLink("https://example.com/connectors"), + *pingone.NewJSONHALLink("https://example.com/flow.deploy"), + *pingone.NewJSONHALLink("https://example.com/flow.clone"), + *pingone.NewJSONHALLink("https://example.com/flow.enabled"), + *pingone.NewJSONHALLink("https://example.com/version"), + ) + environment := pingone.NewResourceRelationshipReadOnly(uuid.MustParse("00000000-0000-0000-0000-000000000001")) + + nodeWithOutcomes := pingone.NewDaVinciFlowGraphDataResponseElementsNode( + func() pingone.DaVinciFlowGraphDataResponseElementsNodeData { + d := pingone.NewDaVinciFlowGraphDataResponseElementsNodeData("esqd1w6k6h", "CONNECTION") + d.SetLabel("Save/Resend") + d.Outcomes = []pingone.DaVinciFlowGraphDataResponseElementsNodeDataOutcome{ + *pingone.NewDaVinciFlowGraphDataResponseElementsNodeDataOutcome("submit", "Save", "0qw160q8zo"), + *pingone.NewDaVinciFlowGraphDataResponseElementsNodeDataOutcome("resend", "Didn't receive an email? Resend", "k0hv0wr75q"), + } + return *d + }(), + true, "", false, true, + *pingone.NewDaVinciFlowGraphDataResponseElementsNodePosition(0, 0), + false, true, false, + ) + + nodeWithoutOutcomes := pingone.NewDaVinciFlowGraphDataResponseElementsNode( + func() pingone.DaVinciFlowGraphDataResponseElementsNodeData { + d := pingone.NewDaVinciFlowGraphDataResponseElementsNodeData("frbglr02tp", "CONNECTION") + d.SetLabel("Single Outcome Form") + return *d + }(), + true, "", false, true, + *pingone.NewDaVinciFlowGraphDataResponseElementsNodePosition(0, 0), + false, true, false, + ) + + elements := pingone.NewDaVinciFlowGraphDataResponseElements( + []pingone.DaVinciFlowGraphDataResponseElementsNode{*nodeWithOutcomes, *nodeWithoutOutcomes}, + ) + + graphData := pingone.NewDaVinciFlowGraphDataResponse( + true, + *elements, + *pingone.NewDaVinciFlowGraphDataResponsePan(0, 0), + true, true, true, 1.0, + ) + + flow := pingone.NewDaVinciFlowResponse(*links, *environment, "flow-1", "Test Flow") + flow.SetGraphData(*graphData) + return flow +} + +// TestDaVinciFlowOutcomes_ProcessorPresentAndAbsent exercises the full +// flow.yaml definition (loaded via schema.Registry, not a synthetic schema) +// through ProcessResource, proving requirements.md acceptance criteria 1 and +// 2: a node with AdditionalProperties["outcomes"] produces +// graph_data.elements.nodes[""].data.outcomes as an ordered list with +// id/result/label preserved verbatim, and a node with no outcomes key has no +// "outcomes" key in its data map at all (not an empty list, not null). +func TestDaVinciFlowOutcomes_ProcessorPresentAndAbsent(t *testing.T) { + registry := davinciFlowRegistry(t) + p := core.NewProcessor(registry) + + result, err := p.ProcessResource("pingone_davinci_flow", buildFlowResponse()) + require.NoError(t, err) + + graphData, ok := result.Attributes["graph_data"].(map[string]interface{}) + require.True(t, ok, "graph_data should be a map") + elements, ok := graphData["elements"].(map[string]interface{}) + require.True(t, ok, "graph_data.elements should be a map") + nodes, ok := elements["nodes"].(map[string]interface{}) + require.True(t, ok, "graph_data.elements.nodes should be a map") + require.Len(t, nodes, 2) + + nodeWithOutcomes, ok := nodes["esqd1w6k6h"].(map[string]interface{}) + require.True(t, ok, "node esqd1w6k6h should be present") + dataWithOutcomes, ok := nodeWithOutcomes["data"].(map[string]interface{}) + require.True(t, ok) + + outcomes, ok := dataWithOutcomes["outcomes"].([]interface{}) + require.True(t, ok, "outcomes should be a []interface{}, got %T", dataWithOutcomes["outcomes"]) + require.Len(t, outcomes, 2) + + first, ok := outcomes[0].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "0qw160q8zo", first["id"]) + assert.Equal(t, "submit", first["result"]) + assert.Equal(t, "Save", first["label"]) + + second, ok := outcomes[1].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "k0hv0wr75q", second["id"]) + assert.Equal(t, "resend", second["result"]) + assert.Equal(t, "Didn't receive an email? Resend", second["label"]) + + nodeWithoutOutcomes, ok := nodes["frbglr02tp"].(map[string]interface{}) + require.True(t, ok, "node frbglr02tp should be present") + dataWithoutOutcomes, ok := nodeWithoutOutcomes["data"].(map[string]interface{}) + require.True(t, ok) + + _, hasOutcomes := dataWithoutOutcomes["outcomes"] + assert.False(t, hasOutcomes, "node without an outcomes key must have no outcomes attribute at all -- not an empty list, not null") +} + +// TestDaVinciFlowOutcomes_HCLRendering verifies the HCL formatter, driven by +// the real flow.yaml definition, renders an outcomes = [ {...}, {...} ] block +// for the node that has outcomes, with id/result/label preserved verbatim, +// and emits no "outcomes" attribute at all for the node that doesn't -- +// covering requirements.md acceptance criterion 1 and 2 at the rendered-HCL +// layer, not just the intermediate representation. +func TestDaVinciFlowOutcomes_HCLRendering(t *testing.T) { + registry := davinciFlowRegistry(t) + p := core.NewProcessor(registry) + def, err := registry.Get("pingone_davinci_flow") + require.NoError(t, err) + + result, err := p.ProcessResource("pingone_davinci_flow", buildFlowResponse()) + require.NoError(t, err) + result.Label = "pingcli__Test-Flow" + + f := hclformatter.NewFormatter() + output, err := f.Format(result, def, hclformatter.FormatOptions{SkipDependencies: true}) + require.NoError(t, err) + + // Node with outcomes: block present with id/result/label preserved verbatim. + assert.Contains(t, output, "outcomes") + assert.Contains(t, output, `"0qw160q8zo"`) + assert.Contains(t, output, `"submit"`) + assert.Contains(t, output, `"Save"`) + assert.Contains(t, output, `"k0hv0wr75q"`) + assert.Contains(t, output, `"resend"`) + assert.Contains(t, output, `"Didn't receive an email? Resend"`) + + // Node without outcomes: no outcomes attribute rendered for that node's + // data block. Since nested_attributes rendering is keyed by presence in + // the map (nestedObjectTokens skips missing keys), the absence is already + // proven by the processor test above; here we confirm the overall output + // contains exactly the one outcomes block (not two, and not an empty one + // for frbglr02tp). + assert.Equal(t, 1, strings.Count(output, "outcomes = ["), + "exactly one outcomes block should be rendered, only for the node that has outcomes") +} diff --git a/internal/core/processor.go b/internal/core/processor.go index f69f220..4fb0331 100644 --- a/internal/core/processor.go +++ b/internal/core/processor.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strings" + "unicode" "github.com/pingidentity/pingcli-plugin-terraformer/internal/schema" ) @@ -612,6 +613,13 @@ func isEmptyValue(field reflect.Value) bool { // findFieldByPath resolves a dot-notation path (e.g. "Parent.Field") against // a reflect.Value representing a struct. Each segment is matched by exact // struct field name. Pointer fields are dereferenced automatically at each step. +// +// When a segment fails to match a named field, and the current struct has an +// exported AdditionalProperties map[string]interface{} field, resolution falls +// back to a lookup in that map (see findAdditionalPropertiesField). This +// covers SDK fields that unmarshal into the generic catch-all because a typed +// struct field doesn't exist yet — the fallback is generic, not tied to any +// specific field name. func findFieldByPath(val reflect.Value, path string) (reflect.Value, bool) { parts := strings.Split(path, ".") current := val @@ -631,7 +639,12 @@ func findFieldByPath(val reflect.Value, path string) (reflect.Value, bool) { field := findStructField(current, part) if !field.IsValid() { - return reflect.Value{}, false + fallback, ok := findAdditionalPropertiesField(current, part) + if !ok { + return reflect.Value{}, false + } + current = fallback + continue } current = field } @@ -650,6 +663,53 @@ func findStructField(val reflect.Value, name string) reflect.Value { return reflect.Value{} } +// findAdditionalPropertiesField is the fallback used by findFieldByPath when +// a named-field lookup misses. If val has an exported AdditionalProperties +// map[string]interface{} field, it looks up name as a key in that map — +// trying the exact segment string first, then a lower-first-letter camelCase +// variant (Go PascalCase source_path values commonly correspond to camelCase +// JSON keys, e.g. "Outcomes" -> "outcomes", "IdUnique" -> "idUnique"). This is +// a deliberate, narrow exception to the "source_path uses Go field names" +// rule, scoped only to this fallback path. +// +// Returns (reflect.Value{}, false) when val has no AdditionalProperties +// field, the field is nil, or neither key variant is present in the map. +func findAdditionalPropertiesField(val reflect.Value, name string) (reflect.Value, bool) { + apField := findStructField(val, "AdditionalProperties") + if !apField.IsValid() || apField.Kind() != reflect.Map || apField.IsNil() { + return reflect.Value{}, false + } + + for _, key := range []string{name, lowerFirst(name)} { + mv := apField.MapIndex(reflect.ValueOf(key)) + if !mv.IsValid() { + continue + } + // mv is a reflect.Interface (the map's value type); unwrap it to the + // concrete underlying value so downstream reflection (Kind checks, + // pointer dereferencing, slice iteration) behaves the same as it + // would for a named struct field. + elem := mv.Elem() + if !elem.IsValid() { + // Explicit nil entry in the map — treat as absent, consistent + // with nil-pointer handling elsewhere in the processor. + continue + } + return elem, true + } + return reflect.Value{}, false +} + +// lowerFirst lowercases the first rune of s, leaving the rest unchanged. +func lowerFirst(s string) string { + if s == "" { + return s + } + r := []rune(s) + r[0] = unicode.ToLower(r[0]) + return string(r) +} + // convertValue converts a reflect.Value to the appropriate Go type based on schema type func (p *Processor) convertValue(field reflect.Value, attrType string) (interface{}, error) { // Choice wrapper unwrapping: when the schema expects a primitive type but diff --git a/internal/core/processor_test.go b/internal/core/processor_test.go index 5dcced9..d38d3e3 100644 --- a/internal/core/processor_test.go +++ b/internal/core/processor_test.go @@ -1816,3 +1816,279 @@ func TestProcessorNilValueKeepEmpty_ListWithNestedAttrs_EmptySlice(t *testing.T) assert.Equal(t, []interface{}{}, jsLinks, "js_links should be []interface{}{} not nil/absent when nil_value: keep_empty on a list with empty slice") } + +// ── AdditionalProperties fallback tests ───────────────────────── + +// MockNodeDataWithAdditionalProperties mirrors the shape of SDK structs like +// DaVinciFlowGraphDataResponseElementsNodeData: a mix of named fields and a +// generic AdditionalProperties catch-all map that receives any JSON key +// without a corresponding typed field. +type MockNodeDataWithAdditionalProperties struct { + ID string + Label string + AdditionalProperties map[string]interface{} +} + +// MockOutcomeItem mirrors the flat shape of a single outcome entry: +// { result, label, id }. +type MockOutcomeItem struct { + ID string + Result string + Label string +} + +func additionalPropertiesFallbackDef(resourceType string) *schema.ResourceDefinition { + return &schema.ResourceDefinition{ + Metadata: schema.ResourceMetadata{ + Platform: "test", + ResourceType: resourceType, + APIType: "NodeData", + Name: "Node Data Test", + ShortName: "nodedata", + Version: "1.0", + }, + API: schema.APIDefinition{ + IDField: "ID", + NameField: "Label", + }, + Attributes: []schema.AttributeDefinition{ + {Name: "ID", TerraformName: "id", Type: "string", SourcePath: "ID"}, + {Name: "Label", TerraformName: "label", Type: "string", SourcePath: "Label"}, + { + Name: "Outcomes", + TerraformName: "outcomes", + Type: "list", + SourcePath: "Outcomes", + NestedAttributes: []schema.AttributeDefinition{ + {Name: "ID", TerraformName: "id", Type: "string", SourcePath: "ID"}, + {Name: "Result", TerraformName: "result", Type: "string", SourcePath: "Result"}, + {Name: "Label", TerraformName: "label", Type: "string", SourcePath: "Label"}, + }, + }, + }, + } +} + +// TestProcessorAdditionalPropertiesFallback_PresentViaLowerFirstKey verifies +// that a source_path with no matching named field (e.g. "Outcomes") falls +// back to a lookup in AdditionalProperties under the lower-first-letter +// camelCase key ("outcomes"), matching the SDK's raw-JSON-derived casing, and +// that the resolved []interface{} of map[string]interface{} elements is +// processed correctly through the existing nested_attributes pipeline. +func TestProcessorAdditionalPropertiesFallback_PresentViaLowerFirstKey(t *testing.T) { + def := additionalPropertiesFallbackDef("test_ap_fallback_present") + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + mock := &MockNodeDataWithAdditionalProperties{ + ID: "node-1", + Label: "Save/Resend", + AdditionalProperties: map[string]interface{}{ + "outcomes": []interface{}{ + map[string]interface{}{"result": "submit", "label": "Save", "id": "0qw160q8zo"}, + map[string]interface{}{"result": "resend", "label": "Didn't receive an email? Resend", "id": "k0hv0wr75q"}, + }, + }, + } + + result, err := p.ProcessResource("test_ap_fallback_present", mock) + require.NoError(t, err) + + outcomes, ok := result.Attributes["outcomes"].([]interface{}) + require.True(t, ok, "outcomes should be []interface{}, got %T", result.Attributes["outcomes"]) + require.Len(t, outcomes, 2) + + first, ok := outcomes[0].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "0qw160q8zo", first["id"]) + assert.Equal(t, "submit", first["result"]) + assert.Equal(t, "Save", first["label"]) + + second, ok := outcomes[1].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "k0hv0wr75q", second["id"]) + assert.Equal(t, "resend", second["result"]) + assert.Equal(t, "Didn't receive an email? Resend", second["label"]) +} + +// TestProcessorAdditionalPropertiesFallback_PresentViaExactKey verifies the +// fallback also matches when the map key already matches the exact segment +// string (no lower-first-letter adjustment needed). +func TestProcessorAdditionalPropertiesFallback_PresentViaExactKey(t *testing.T) { + def := additionalPropertiesFallbackDef("test_ap_fallback_exact") + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + mock := &MockNodeDataWithAdditionalProperties{ + ID: "node-2", + Label: "Exact Key", + AdditionalProperties: map[string]interface{}{ + "Outcomes": []interface{}{ + map[string]interface{}{"result": "submit", "label": "Save", "id": "abc123"}, + }, + }, + } + + result, err := p.ProcessResource("test_ap_fallback_exact", mock) + require.NoError(t, err) + + outcomes, ok := result.Attributes["outcomes"].([]interface{}) + require.True(t, ok, "outcomes should be []interface{}, got %T", result.Attributes["outcomes"]) + require.Len(t, outcomes, 1) + first, ok := outcomes[0].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "abc123", first["id"]) +} + +// TestProcessorAdditionalPropertiesFallback_KeyAbsent verifies that when the +// struct has an AdditionalProperties map but neither casing of the segment +// name is present as a key, the attribute resolves as absent — same as +// today's behavior for any unresolved field. No outcomes key should appear +// in the output at all (not an empty list, not null). +func TestProcessorAdditionalPropertiesFallback_KeyAbsent(t *testing.T) { + def := additionalPropertiesFallbackDef("test_ap_fallback_absent") + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + mock := &MockNodeDataWithAdditionalProperties{ + ID: "node-3", + Label: "No Outcomes", + AdditionalProperties: map[string]interface{}{ + "someOtherKey": "irrelevant", + }, + } + + result, err := p.ProcessResource("test_ap_fallback_absent", mock) + require.NoError(t, err) + + _, exists := result.Attributes["outcomes"] + assert.False(t, exists, "outcomes should not be present when the key is absent from AdditionalProperties") +} + +// TestProcessorAdditionalPropertiesFallback_NilAdditionalProperties verifies +// that a nil (unset) AdditionalProperties map is treated the same as a +// missing key — no panic, attribute simply absent. +func TestProcessorAdditionalPropertiesFallback_NilAdditionalProperties(t *testing.T) { + def := additionalPropertiesFallbackDef("test_ap_fallback_nil_map") + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + mock := &MockNodeDataWithAdditionalProperties{ + ID: "node-4", + Label: "Nil Map", + AdditionalProperties: nil, + } + + result, err := p.ProcessResource("test_ap_fallback_nil_map", mock) + require.NoError(t, err) + + _, exists := result.Attributes["outcomes"] + assert.False(t, exists, "outcomes should not be present when AdditionalProperties is nil") +} + +// TestProcessorAdditionalPropertiesFallback_NoAdditionalPropertiesField +// verifies that structs with no AdditionalProperties field at all are +// completely unaffected — a regression guard proving the fallback doesn't +// change behavior for the many existing structs that lack this field. +func TestProcessorAdditionalPropertiesFallback_NoAdditionalPropertiesField(t *testing.T) { + def := additionalPropertiesFallbackDef("test_ap_fallback_no_field") + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + // MockNestedResource (defined earlier in this file) has no + // AdditionalProperties field at all. + mock := &struct { + ID string + Label string + }{ID: "node-5", Label: "No AP Field"} + + result, err := p.ProcessResource("test_ap_fallback_no_field", mock) + require.NoError(t, err) + + assert.Equal(t, "node-5", result.Attributes["id"]) + assert.Equal(t, "No AP Field", result.Attributes["label"]) + _, exists := result.Attributes["outcomes"] + assert.False(t, exists, "outcomes should not be present when the struct has no AdditionalProperties field") +} + +// MockOutcomeElementWithAP is a slice element whose "Id" is not a named +// field — it only exists in the element's own AdditionalProperties map, +// mirroring how MapKeyPath resolution would need to fall back per-element. +type MockOutcomeElementWithAP struct { + Result string + AdditionalProperties map[string]interface{} +} + +// MockNodeDataWithOutcomeSlice has a normal named Outcomes slice field (so +// this test isolates convertSliceToMap's own findFieldByPath call for +// map_key_path resolution, rather than conflating it with the outer-field +// fallback already covered by the tests above). +type MockNodeDataWithOutcomeSlice struct { + ID string + Outcomes []MockOutcomeElementWithAP +} + +// TestProcessorAdditionalPropertiesFallback_MapKeyPath verifies the fallback +// also engages for convertSliceToMap's map_key_path resolution, since that +// function calls the same findFieldByPath — proving the fix is generic +// across every findFieldByPath call site, not just processOneAttribute's +// direct path, per Task 1's scope note that this should be verified rather +// than special-cased. Here, MapKeyPath "Id" has no named field on the slice +// element struct; it only resolves via that element's own AdditionalProperties +// map (key "id", lower-first-letter match). +func TestProcessorAdditionalPropertiesFallback_MapKeyPath(t *testing.T) { + def := &schema.ResourceDefinition{ + Metadata: schema.ResourceMetadata{ + Platform: "test", + ResourceType: "test_ap_fallback_map_key_path", + APIType: "NodeData", + Name: "Node Data Test", + ShortName: "nodedata", + Version: "1.0", + }, + API: schema.APIDefinition{ + IDField: "ID", + NameField: "ID", + }, + Attributes: []schema.AttributeDefinition{ + {Name: "ID", TerraformName: "id", Type: "string", SourcePath: "ID"}, + { + Name: "Outcomes", + TerraformName: "outcomes", + Type: "map", + SourcePath: "Outcomes", + MapKeyPath: "Id", + NestedAttributes: []schema.AttributeDefinition{ + {Name: "Result", TerraformName: "result", Type: "string", SourcePath: "Result"}, + }, + }, + }, + } + registry := schema.NewRegistry() + require.NoError(t, registry.Register(def)) + p := core.NewProcessor(registry) + + mock := &MockNodeDataWithOutcomeSlice{ + ID: "node-6", + Outcomes: []MockOutcomeElementWithAP{ + { + Result: "submit", + AdditionalProperties: map[string]interface{}{"id": "out-1"}, + }, + }, + } + + result, err := p.ProcessResource("test_ap_fallback_map_key_path", mock) + require.NoError(t, err) + + outcomes, ok := result.Attributes["outcomes"].(map[string]interface{}) + require.True(t, ok, "outcomes should be map[string]interface{}, got %T", result.Attributes["outcomes"]) + entry, ok := outcomes["out-1"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "submit", entry["result"]) +}