resource/github_repository_webhook: Avoid spurious diff #133
Conversation
radeksimko
force-pushed
the
b-repo-webhook
branch
from
b5e0077 to
5197e0f
Compare
| } | ||
|
|
||
| // Write new keys | ||
| for k, v := range oldKeys { |
There was a problem hiding this comment.
any chance this could be done all in the previous loop?
There was a problem hiding this comment.
That was actually the very first version of this PR 😃until I saw the test intermittently failing as the for loop kept ranging over some new elements which were just added.
Why? https://golang.org/ref/spec#For_range
The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next. If a map entry that has not yet been reached is removed during iteration, the corresponding iteration value will not be produced. If a map entry is created during iteration, that entry may be produced during the iteration or may be skipped. The choice may vary for each entry created and from one iteration to the next.
Short answer: No.
Feel free to have a play yourself: https://play.golang.org/p/Tm1FPyjwd0m 😉
| Optional: true, | ||
| Sensitive: true, | ||
| DiffSuppressFunc: func(k, oldV, newV string, d *schema.ResourceData) bool { | ||
| maskedSecret := "********" |
There was a problem hiding this comment.
I haven't really worked with sensitive attributes, but it feels strange to me for a maskedSecret to ALWAYS be 8 asterisks, is this code robust?
There was a problem hiding this comment.
GitHub unfortunately doesn't document this, but I assumed it's a static placeholder they just return. Why would/should the API ever return different number of asterisks?
btw. this is unrelated to how we treat sensitive data in the schema via Sensitive, it's more related to how the backend API (GitHub in this case) implements it.
There was a problem hiding this comment.
okay, I think a code comment might be appropriate in this case mentioning it appears to be an undocumented GH api response.
…ebhook resource/github_repository_webhook: Avoid spurious diff
Fixes #81