Bug fix: PKI ca cert field set initial value to 720hr#9465
Conversation
chelshaw
left a comment
chelshaw
left a comment
There was a problem hiding this comment.
I'm biased, but I think this looks good 😄
| @helperTextDisabled="Vault will use the default lease duration" | ||
| @helperTextEnabled="Lease will expire after" | ||
| @description={{attr.helpText}} | ||
| @initialValue={{or (get model valuePath) attr.options.setDefault}} |
There was a problem hiding this comment.
if the only thing that isCaCert changes is the initialValue, could initialValue be a computed property in the component? that'd keep this template a little cleaner. whaddya think?
There was a problem hiding this comment.
heh, this idea is actually irrelevant given my other comment. i'll leave it here though just in case. :)
| label: 'TTL', | ||
| editType: 'ttl', | ||
| defaultValue: '720h', | ||
| defaultValue: '30h', |
There was a problem hiding this comment.
another option for solving this would be to that would keep the defaultValues set in the model and alleviate the need for computed properties in the component or conditionals in the template would be to overwrite ttl value that is extended inside the pki-ca-certificate model. i haven't tested it out but it's worth a shot! i think it's good to define values in the same source as much as possible. what are your thoughts?
andaley
left a comment
andaley
left a comment
There was a problem hiding this comment.
i had one suggestion, and otherwise looks like the tests need to be updated/fixed. thanks for following up on this!
|
Closing for work that will change the workflow. |
…hicorp#9343) (hashicorp#9465) * Add O= restrictions in addition to OU= restrictions * Add changelog * Add goDoc to test * Don't let test certificate expire. Co-authored-by: Kit Haines <khaines@mit.edu>
3 participants
We had originally added the
intiailvalueon thepki-certificatefile in this PR. However, this set all initial values used in the PKI to 720hr, which was problematic when you created a certificate after you created your CA certificate; naturally, it outlived your CA certificate.To fix this we are now only set the initial value of ca certs to 720h, and everything else to 30hr.
Note: there is duplication between the
TtlPicker2components displayed whenisCaCertis true and whenisCaCertis false. We opted for the duplication instead of making a difficult to read inline conditional for theintialValueproperty, which is the only property that is different between the two.