Azure / kubernetes-keyvault-flexvol

Azure keyvault integration with Kubernetes via a Flex Volume
MIT License
253 stars 84 forks source link

Move to using azure sdk instead of using custom logic to contact NMI in aad-pod-idnetity #96

Closed kkmsft closed 5 years ago

kkmsft commented 5 years ago

Currently we use custom logic to get the token from NMI. We should be ideally using the azure sdk to grab the token below:

    endpoint := fmt.Sprintf("%s?resource=%s", nmiendpoint, resource)
    client := &http.Client{}
    req, err := http.NewRequest("GET", endpoint, nil)
    if err != nil {
        return nil, err
    }
    req.Header.Add(podnsheader, podns)
    req.Header.Add(podnameheader, podname)
    resp, err := client.Do(req)
    if err != nil {
        return nil, errors.Wrap(err, "failed to query NMI")
    }
    defer func() {
        if err := resp.Body.Close(); err != nil {
            fmt.Print("failed to close NMI response body")
        }
    }()

    if resp.StatusCode == http.StatusOK {
        var nmiResp = NMIResponse{}
        if err := json.NewDecoder(resp.Body).Decode(&nmiResp); err != nil {
            return nil, errors.Wrap(err, "failed to decode NMI response")
        }

        r, _ := regexp.Compile("^(\\S{4})(\\S|\\s)*(\\S{4})$")
        fmt.Printf("\n accesstoken: %s\n", r.ReplaceAllString(nmiResp.Token.AccessToken, "$1##### REDACTED #####$3"))
        fmt.Printf("\n clientid: %s\n", r.ReplaceAllString(nmiResp.ClientID, "$1##### REDACTED #####$3"))

        token := nmiResp.Token
        clientID := nmiResp.ClientID

        if &token == nil || clientID == "" {
            return nil, fmt.Errorf("nmi did not return expected values in response: token and clientid")
        }
aramase commented 5 years ago

As per discussion with @ritazh and @kkmsft, replacing calls to nmi with azure sdk is not currently an option because of limitations.

Retry for nmi calls as discussed in our conversation - https://github.com/Azure/kubernetes-keyvault-flexvol/pull/102 [Pending testing]

ritazh commented 5 years ago

Closed via #102