Support for ldap secret engine, static and dynamic roles #333
Conversation
also fixes a bug with an incorrect JsonConverter attribute on StaticCredentials.RotationPeriod
|
|
||
| namespace VaultSharp.V1.SecretsEngines.OpenLDAP | ||
| { | ||
| public class Role |
There was a problem hiding this comment.
Recommendation to rename this class to DynamicRole, and to also add the documentation similar to StaticRole.
|
|
||
| namespace VaultSharp.V1.SecretsEngines.OpenLDAP | ||
| { | ||
| public class LdapCredentials : UsernamePasswordCredentials |
There was a problem hiding this comment.
Recommendation to rename the class to LDAPCredentials instead of LdapCredentials ensuring that abbreviations are consistent.
| /// more details. This field may optionally be provided as a base64 encoded string. | ||
| /// </remarks> | ||
| [JsonPropertyName("creation_ldif")] | ||
| public string CreationLdif { get; set; } |
There was a problem hiding this comment.
Recommendation to capitalise all instances of Ldif with LDIF since it is an abbreviation.
capitalize acronyms
|
I agree with all suggestions, though I was following the conventions in the database secret engine so it might be worth revisiting as well. I also noticed that the LDAP secret engine is referenced as |
Hey, I guess we must not, since it would affect backward compatibility. We need to check with @rajanadar on this. The process is to mark it as a breaking change, but I'm not sure whether it will go in a patch or a major release. |
|
Any additional feedback for this PR? |
Fixes a type mismatch bug with the existing LDAP secret engine implementation. Adds full support for static and dynamic roles in the LDAP secret engine.
Vault API Docs: https://developer.hashicorp.com/vault/api-docs/v1.13.x/secret/ldap