feat: *: use Track 2 SDKs #410
Conversation
Signed-off-by: Steve Kuznetsov <[email protected]>
|
Obviates #195 |
| keyvaultRegionAnnotationKey: []byte(result.Header.Get(keyvaultRegionAnnotationValue)), | ||
| versionAnnotationKey: []byte(encryptionResponseVersion), | ||
| algorithmAnnotationKey: []byte(encryptionAlgorithm), | ||
| // dateAnnotationKey: []byte(result.Header.Get(dateAnnotationValue)), |
There was a problem hiding this comment.
@aramase we need to find a path forward if these are must-have
| } | ||
|
|
||
| func getVaultDNSSuffix(managedHSM bool, env *azure.Environment) string { | ||
| func getVaultDNSSuffix(managedHSM bool, cloud string) (string, error) { |
There was a problem hiding this comment.
@aramase I went further than your original PR and entirely removed autorest. PTAL
| proxyPort: 7788, | ||
| expectedVaultURL: "http://localhost:7788/testkv.vault.azure.net/", | ||
| }, | ||
| //{ |
There was a problem hiding this comment.
Tactically: we will remove HTTP proxy support, and the user of this will migrate, as proxying Entra traffic through plaintext is not what we want anyway.
| } | ||
| if kvClient == nil { | ||
| t.Fatalf("newKeyVaultClient() expected kv client to not be nil") | ||
| } | ||
| if !strings.Contains(kvClient.GetUserAgent(), "k8s-kms-keyvault") { |
There was a problem hiding this comment.
@aramase I could not figure out what the point of these getters were in the public interface and why this was being tested
| if !strings.Contains(kvClient.GetUserAgent(), "k8s-kms-keyvault") { | ||
| t.Fatalf("newKeyVaultClient() expected k8s-kms-keyvault user agent") | ||
| } | ||
| if kvClient.GetVaultURL() != test.expectedVaultURL { |
There was a problem hiding this comment.
This is likely best as a unit test for getVaultURL and getProxiedVaultURL, speaks to a need for some refactor there to better encapsulate the logic rather than punching a hole through the public interface of the return from the constructor to test some implementation detail. WDYT?
| func (t *transporter) Do(req *http.Request) (*http.Response, error) { | ||
| // adds the target header if proxy mode is enabled | ||
| req.Header.Set(consts.RequestHeaderTargetType, consts.TargetTypeAzureActiveDirectory) | ||
| return http.DefaultClient.Do(req) |
There was a problem hiding this comment.
Using the default client seems a bit sketch?
Reason for Change:
Migrate from autorest/adal to azidentity
Migrate from github.com/Azure/azure-sdk-for-go/services/keyvault to sdk/keyvault
Issue Fixed:
fixes #179
fixes #180
Unblocks #409