-
Notifications
You must be signed in to change notification settings - Fork 2
Adds remote tdx provider for development #40
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
Conversation
certFile := cCtx.String("tls-certificate") | ||
keyFile := cCtx.String("tls-private-key") | ||
certFile := cCtx.String("tls-certificate-path") | ||
keyFile := cCtx.String("tls-private-key-path") |
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.
oh interesting, that's a separate bugfixe!
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.
could be it's own PR, seems important as main would be broken 🤔
cmd/proxy-client/main.go
Outdated
var issuer atls.Issuer | ||
if clientAttestationTypeFlag == "dummy" && devDummyDcapURL == "" { | ||
return errors.New("dummy client attestation type but remote not specified") | ||
} else if clientAttestationTypeFlag != "dummy" && devDummyDcapURL != "" { |
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.
upgrade string "dummy" to a const
cmd/proxy-client/main.go
Outdated
return err | ||
} | ||
var issuer atls.Issuer | ||
if clientAttestationTypeFlag == "dummy" && devDummyDcapURL == "" { |
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 if/else block feels error prone, and might be a nice one to structure in a more understandable and readable manner
one idea could be to create helper variables, like
isDummyAttestationType := clientAttestationTypeFlag == "dummy"
hasDummyDcapURL := devDummyDcapURL != ""
using that might feel awkward, but it'd make the code more legible
Looks great, directionally. Would be good to document the expected dummy dcap server response, and how to run one locally. |
should we merge this? I was expecting this in main already. Not that I need it at this point, my understanding was just that this is already live. |
Allows configuring a remote quotes provider.