Update error message for apply validation #21312
Conversation
Add a hint that the validation failure has occurred at the root of the resource schema to the error message. This is because the root resource has an empty path when being validated and the path is being relied upon to provide context into the error message.
|
|
There was a problem hiding this comment.
Hi @chrisst, thank you for submitting this !
It does not appear that your implementation is complete: what is the result if the path is not empty? It could be that I don't completely understand the situation that leads to this confusing output - is there a GitHub issue that describes the bug that lead you to open this PR? It's easier to review PRs when we have an open issue; it gives us a chance to understand the root cause and it can help to have reproduction cases so we can write tests.
If you are still interested in working on this PR (I know it's been awhile since you opened this and I am sorry for the long silence), let's continue this discussion about the problem space and design. It would help if you could point to any related GitHub issues.
| @@ -28,12 +28,17 @@ func AssertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu | |||
|
|
|||
| func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Value, path cty.Path) []error { | |||
| var errs []error | |||
| var atRoot string | |||
| if len(path) == 0 { | |||
There was a problem hiding this comment.
What happens if this is not 0? Is there a value we can use in those cases?
There was a problem hiding this comment.
On line 54 of this PR, if path is not zero it's used to print out the path of the offending field. The issue was that if path was empty, eg it was a root field, the error message that got generated from "append(errs, path.NewErrorf("inconsistent values for sensitive attribute"))" generates a message that doesn't make sense - see the message in the PR description. So IIRC the only situation that wasn't already handled well was if the path was zero.
|
This PR was meant to assist provider developers in finding a bad configuration for a resource they are developing. There isn't an issue because it's hopefully an error situation that shouldn't make it to a shipped version of a resource. I just had to dig into code a bit to figure out what the real error was and thought I'd leave better bread crumbs for the next person. |
|
Thank you very much for the explanation @chrisst , now I follow you. If you'd like (as long as I have the ability to push to your fork), I can resolve the conflict for you, otherwise you can do it yourself and I will approve this PR - I'll leave that up to you but I'm happy to take over since you've waited so long already. |
|
conflict resolved, thanks! |
Codecov Report
|
* Update error message for apply validation Add a hint that the validation failure has occurred at the root of the resource schema to the error message. This is because the root resource has an empty path when being validated and the path is being relied upon to provide context into the error message.
|
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. |
ghost
locked and limited conversation to collaborators
Add a hint that the validation failure has occurred at the root of the resource
schema to the error message. This is because the root resource has an empty
path when being validated and the path is being relied upon to provide context
into the error message.
In practice when the ID of a resource gets cleared accidentally due to a bug during Create the entire
actualschema is null. The user facing error message when this happens is:This PR would change the message to: