Skip to content
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 login credential type identifier parsing #989

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

ksamoray
Copy link
Collaborator

@ksamoray ksamoray commented Oct 4, 2023

Values used for type identifier parsing were incorrect

@@ -449,7 +449,7 @@ func setCredentialValuesInSchema(d *schema.ResourceData, credential *data.Struct
return errors.New("no valid credential found")
}

d.Set("credential", elem)
d.Set("credential", []interface{}{elem})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this wrong before and we did not realize only because elem was an empty map?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. And the list was empty as the constants which were used in the switch case were wrong as well. So values were taken from the state.

@@ -435,7 +435,7 @@ func setCredentialValuesInSchema(d *schema.ResourceData, credential *data.Struct
elem["thumbprint"] = credEntry.Thumbprint
elem["password"] = credEntry.Password
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we see non-empty diff here since sensitive data is not sent back by NSX?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - in transport node the polymorphic objects are recreated with make(map[]interface{}), while here the same map is being populated with the GET values. So passwords are retained from the state.
I'm implementing a similar mechanism for transport node.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - I'm still missing something.. I see elem is initialized as an empty map, and then populated with GET values. I'm missing the part where passwords are coming from the state..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - the code actually works since I don't assign these elem structs into "username_password_login" etc schema as I should.
I'll revise this farther.

@ksamoray ksamoray force-pushed the compute_mgr_creds branch 5 times, most recently from a447291 to df08b5f Compare October 12, 2023 10:44
NSX does not return sensitive data (e.g passwords) on GET operation, and
therefore TF detects a diff.
But as the schema.Resource object contains the credentials from the
state, we can keep these intact and then the value will remain as is and
stored again in the state.

Signed-off-by: Kobi Samoray <[email protected]>
Values used for type identifier parsing were incorrect

Signed-off-by: Kobi Samoray <[email protected]>
@ksamoray ksamoray merged commit 9094cc9 into vmware:master Oct 13, 2023
2 checks passed
@ksamoray ksamoray deleted the compute_mgr_creds branch October 13, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants