fix(ci): update nanotdf create function signature to use options#374
fix(ci): update nanotdf create function signature to use options#374
Conversation
Doom4535
left a comment
Doom4535
left a comment
There was a problem hiding this comment.
I like the code cleanup; I believe the direct inclusion of the OpenTDF sdk can be avoided here by using the one that is already wrapped inside of the handler (although I have not tested these changes myself).
| // TODO: validate values are FQNs or return an error [https://github.com/opentdf/platform/issues/515] | ||
| _, err = h.sdk.CreateNanoTDF(enc, bytes.NewReader(b), *nanoTDFConfig) | ||
| _, err := h.sdk.CreateNanoTDF(enc, bytes.NewReader(b), options...) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return enc, nil |
There was a problem hiding this comment.
Slight code shortening:
if _, err := h.sdk.CreateNanoTDF(enc, bytes.NewReader(b), options...); err != nil {
return nil, err
}
return enc, nil| _, err := h.sdk.ReadNanoTDF(io.Writer(&outBuf), bytes.NewReader(toDecrypt)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &outBuf, nil |
There was a problem hiding this comment.
While we're here, we could shorten this spot a bit as well:
if _, err := h.sdk.ReadNanoTDF(io.Writer(&outBuf), bytes.NewReader(toDecrypt)); err != nil {
return nil, err
}
return &outBuf, nil| options := []sdk.NanoTDFOption{ | ||
| sdk.WithKasURL(h.platformEndpoint + kasUrlPath), | ||
| sdk.WithNanoDataAttributes(values), | ||
| } | ||
| if ecdsaBinding { | ||
| nanoTDFConfig.EnableECDSAPolicyBinding() | ||
| options = append(options, sdk.WithECDSAPolicyBinding()) | ||
| } |
There was a problem hiding this comment.
Is it possible to remove the need for the extra module inclusion of github.com/opentdf/platform/sdk by using the handler wrapped sdk? Something like:
options := []h.sdk.NanoTDFOption{
h.sdk.WithKasURL(h.platformEndpoint + kasUrlPath),
h.sdk.WithNanoDataAttributes(values),
}
if ecdsaBinding {
options = append(options, h.sdk.WithECDSAPolicyBinding())
}
|
@sujankota I believe the commit name also needs to be updated, as this fix is not for the CI/CD environment; also don't forget to signoff on the commit to satisfy the DCO (part of the new committing process: https://github.com/opentdf/otdfctl/blob/main/CONTRIBUTING.md). |
2 participants
No description provided.