Mildwonkey/nested type format #27793
Conversation
Codecov Report
|
Eventually, the diff formatter will need to be updated to properly handle NestedTypes, but for now we can let the existing function deal with them as regular cty.Object-type attributes. To avoid printing sensitive nested attributes, we will treat any attribute with at least one sensitive nested attribute as an entirely sensitive attribute.
ImpliedType() was returning too early when the given object had optional attributes, therefore skipping the incredibly important step of accounting for the nesting mode when returning said type.
types This commit is a small bit of setup for upcoming work on the format printer for attributes with NestedTypes. I added an attribute with NestedType + NestingMode NestingList to the TestResourceChange_nestedList suite of tests to confirm that the output looks good enough for now, and added some comments about what still needs to be done and what tests _should_ fail once the diff printing is improved. This is admittedly not a very high-value commit, but I'd rather get the tests committed now than risk losing them; these are especially tedious to hand-craft.
4afd3c8 to
4145741
Compare
| @@ -4311,3 +4342,34 @@ func outputChange(name string, before, after cty.Value, sensitive bool) *plans.O | |||
|
|
|||
| return changeSrc | |||
| } | |||
|
|
|||
| // A basic test schema using NestingList for one attribute and one block | |||
| var testSchemaNestingList = &configschema.Block{ | |||
There was a problem hiding this comment.
Thanks for spotting & extracting this!
| @@ -361,6 +374,10 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At | |||
| return false | |||
| } | |||
|
|
|||
| func (p *blockBodyDiffPrinter) writeNestedAttrDiff(name string, attrS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path) bool { | |||
| panic("not implemented") | |||
| } | |||
There was a problem hiding this comment.
This uncalled method confused me a bit, and I couldn't quite see where you'd go next with it. Would it be possible to add a short FIXME comment explaining where this would be called from and some of the next steps?
There was a problem hiding this comment.
updated! the new comment should make it easier to link the TBD new function, where it's going to be called inside writeAttrDiff, and the tests with FIXME comments that show the current, less desirable, output.
0f4186e to
3c6860c
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This PR builds on #27699 . It introduces a small change to the diff formatter, so we don't inadvertently print
NestedTypeattributes with nested sensitive attributes. This is a temporary workaround until a bespokeNestedTypediff format implementation is created.I added an attribute with a
NestedTypeto theTestResourceChange_nestedListtest suite, and one test for sensitivity. This pattern needs to be extended to the other tests, and some updated when the bespoke formatter is implemented, but I think it's a good start that's worth getting into the code base now rather than waiting for the end product.This work very helpfully uncovered a bug in the
ImpliedType()function inconfigschema, which is fixed here. The last commit modifies a test to prove this fix - the issue was specifically with returning anObjectWithOptionalAttrstoo early.