added a flag to make common name optional if desired #3940
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are other places in the logic that assume that |
jefferai
reviewed
Feb 8, 2018
builtin/logical/pki/cert_util.go
Outdated
| @@ -258,6 +258,10 @@ func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial | |||
| // If one does not pass, it is returned in the string argument. | |||
| func validateNames(req *logical.Request, names []string, role *roleEntry) string { | |||
| for _, name := range names { | |||
| if name == "" { | |||
There was a problem hiding this comment.
I don't think this should happen... Don't see a reason why we would want empty names in that list.
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3836
Fixes #3918
This PR retains the current behavior as the default, which is to expect the
common_namesupplied while generating the cert. But, a flag can be flipped on the role to makecommon_namefield optional.I did not see anything that is making the
alt_namesmandatory. Unless I am missing something, there is nothing to be done w.r.t makingalt_namesoptional. Also, the issue was not assertively mentioning the need to make changes toalt_names. Please provide feedback on thealt_namesbehavior and I'm happy to make any changes regarding that.