-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add PKCS8 marshaling to PKI #3518
Conversation
@@ -22,6 +22,17 @@ key and issuing cert will be appended to the | |||
certificate pem. Defaults to "pem".`, | |||
} | |||
|
|||
fields["private_key_format"] = &framework.FieldSchema{ |
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.
Add this param to the docs page as well.
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.
Done.
@@ -106,6 +106,13 @@ func (b *backend) pathGenerateIntermediate( | |||
} | |||
} | |||
|
|||
if data.Get("private_key_format").(string) == "pkcs8" { |
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.
This is fine as-is, but noticed that the other params were being retrieved with b.getGenerationParams(data)
, so this could also be moved there as well
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.
I went down that path originally but there lay madness. The problem is that it's not called by all functions that can generate certs and what is then done with the generation params depends on the path.
After going down that path and then one other, I realized the easiest way by far was to just transform the output once it's already generated.
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.
One minor comment. Otherwise, LGTM!
|
||
privKeyTypeRaw, ok := resp.Data["private_key_type"] | ||
if !ok { | ||
return fmt.Errorf("error converting response to pkcs8: %q not found in response", "private_key_type") |
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.
Can we not use %q
here since its not taking any variable as input? Or was there a reason for doing it this way?
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.
I did it this way instead of \"
. Just felt nicer than escapes.
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.
Ah, I see. 👍
* oss/master: (30 commits) Handle 'not supplied' case for field type TypeNameString (#3546) Fix deprecated cassandra backend tests (#3543) changelog++ auth/aws: Make disallow_reauthentication and allow_instance_migration mutually exclusive (#3291) changelog++ More Mount Conflict Detection (#2919) Fix swallowed errors in TestRollbackManager_Join() (#3327) changelog++ added AWS enpoint handling (#3416) Seal wrap all root tokens and their leases (#3540) Return group memberships of entity during read (#3526) Add note on support for using rec keys on /sys/rekey (#3517) Add third party tools list to website (#3488) Minor client refactoring (#3539) changelog++ Add PKCS8 marshaling to PKI (#3518) Update SSH list roles docs (#3536) Update gocql dep changelog++ Return role info for each role on pathRoleList (#3532) ...
No description provided.