-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix: env0_gcp_credentials - apply after import will delete+create #380
Conversation
@@ -23,7 +26,7 @@ func TestUnitGcpCredentialsResource(t *testing.T) { | |||
} | |||
|
|||
updateGcpCredentialResource := map[string]interface{}{ | |||
"name": "testUpdate", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug in test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏼
if err := writeResourceData(&credentials, d); err != nil { | ||
return diag.Errorf("schema resource data serialization failed: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just asking, what changed that we now need it than before?
Is import
flow will not work without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The writeResourceData (and readReasourceData) is a utility function that reads all the fields in the interface and updates the schema via set
calls.
This is future proof: in case you add more fields to the schema/api - no additional coding is needed (other than updating the schema).
There is a potential bug here that drifting is not detected (E.g., if someone changes the name in the UX). Once I added this code, it caught a bug in the tests as well (so I fixed the tests as well).
Import flow will work without it, and I just fixed bugs I encountered on the way.
Issue & Steps to Reproduce / Feature Request
fixes #99
The last part of the effort to add import to all credential types (Azure and AWS is complete).
Solution