Skip to content

Commit

Permalink
Merge pull request #4057 from mboersma/go-away-autorest
Browse files Browse the repository at this point in the history
Remove unused go-autorest code
  • Loading branch information
k8s-ci-robot authored Oct 4, 2023
2 parents 1f387aa + 2d1e96b commit 0f43bda
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 301 deletions.
30 changes: 0 additions & 30 deletions azure/converters/futures.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,10 @@ import (
"encoding/base64"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
"github.com/pkg/errors"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
)

// SDKToFuture converts an SDK future to an infrav1.Future.
func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceName, rgName string) (*infrav1.Future, error) {
jsonData, err := future.MarshalJSON()
if err != nil {
return nil, errors.Wrap(err, "failed to marshal async future")
}

return &infrav1.Future{
Type: futureType,
ResourceGroup: rgName,
ServiceName: service,
Name: resourceName,
Data: base64.URLEncoding.EncodeToString(jsonData),
}, nil
}

// FutureToSDK converts an infrav1.Future to an SDK future.
func FutureToSDK(future infrav1.Future) (azureautorest.FutureAPI, error) {
futureData, err := base64.URLEncoding.DecodeString(future.Data)
if err != nil {
return nil, errors.Wrap(err, "failed to base64 decode future data")
}
var genericFuture azureautorest.Future
if err := genericFuture.UnmarshalJSON(futureData); err != nil {
return nil, errors.Wrap(err, "failed to unmarshal future data")
}
return &genericFuture, nil
}

// PollerToFuture converts an SDK poller to an infrav1.Future.
func PollerToFuture[T any](poller *runtime.Poller[T], futureType, service, resourceName, rgName string) (*infrav1.Future, error) {
token, err := poller.ResumeToken()
Expand Down
98 changes: 0 additions & 98 deletions azure/converters/futures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,11 @@ import (
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
. "github.com/onsi/gomega"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
)

var (
sdkFuture, _ = azureautorest.NewFutureFromResponse(&http.Response{
Status: "200 OK",
StatusCode: http.StatusOK,
Request: &http.Request{
Method: http.MethodDelete,
},
})

validFuture = infrav1.Future{
Type: infrav1.DeleteFuture,
ServiceName: "test-service",
Expand Down Expand Up @@ -72,95 +63,6 @@ var (
}
)

func Test_SDKToFuture(t *testing.T) {
cases := []struct {
name string
future azureautorest.FutureAPI
futureType string
service string
resourceName string
rgName string
expect func(*GomegaWithT, *infrav1.Future, error)
}{
{
name: "valid future",
future: &sdkFuture,
futureType: infrav1.DeleteFuture,
service: "test-service",
resourceName: "test-resource",
rgName: "test-group",
expect: func(g *GomegaWithT, f *infrav1.Future, err error) {
g.Expect(err).Should(BeNil())
g.Expect(f).Should(BeEquivalentTo(&infrav1.Future{
Type: infrav1.DeleteFuture,
ServiceName: "test-service",
Name: "test-resource",
ResourceGroup: "test-group",
Data: "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiIiwicG9sbGluZ1VSSSI6IiIsImxyb1N0YXRlIjoiU3VjY2VlZGVkIiwicmVzdWx0VVJJIjoiIn0=",
}))
},
},
}

for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
g := NewGomegaWithT(t)
result, err := SDKToFuture(c.future, c.futureType, c.service, c.resourceName, c.rgName)
c.expect(g, result, err)
})
}
}

func Test_FutureToSDK(t *testing.T) {
cases := []struct {
name string
future infrav1.Future
expect func(*GomegaWithT, azureautorest.FutureAPI, error)
}{
{
name: "data is empty",
future: emptyDataFuture,
expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) {
g.Expect(err.Error()).Should(ContainSubstring("failed to unmarshal future data"))
},
},
{
name: "data is not base64 encoded",
future: decodedDataFuture,
expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) {
g.Expect(err.Error()).Should(ContainSubstring("failed to base64 decode future data"))
},
},
{
name: "base64 data is not a valid future",
future: invalidFuture,
expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) {
g.Expect(err.Error()).Should(ContainSubstring("failed to unmarshal future data"))
},
},
{
name: "valid future data",
future: validFuture,
expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) {
g.Expect(err).Should(BeNil())
g.Expect(f).Should(BeAssignableToTypeOf(&azureautorest.Future{}))
},
},
}

for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
g := NewGomegaWithT(t)
result, err := FutureToSDK(c.future)
c.expect(g, result, err)
})
}
}

func TestPollerToFuture(t *testing.T) {
cases := []struct {
name string
Expand Down
34 changes: 0 additions & 34 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
"github.com/Azure/go-autorest/autorest"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
"sigs.k8s.io/cluster-api-provider-azure/version"
)
Expand Down Expand Up @@ -403,36 +402,3 @@ func (p CustomPutPatchHeaderPolicy) Do(req *policy.Request) (*http.Response, err

return req.Next()
}

// SetAutoRestClientDefaults set authorizer and user agent for autorest client.
func SetAutoRestClientDefaults(c *autorest.Client, auth autorest.Authorizer) {
c.Authorizer = auth
// Wrap the original Sender on the autorest.Client c.
// The wrapped Sender should set the x-ms-correlation-request-id on the given
// request, then pass the new request to the underlying Sender.
c.Sender = autorest.DecorateSender(c.Sender, msCorrelationIDSendDecorator)
// The default number of retries is 3. This means the client will attempt to retry operation results like resource
// conflicts (HTTP 409). For a reconciling controller, this is undesirable behavior since if the controller runs
// into an error reconciling, the controller would be better off to end with an error and try again later.
//
// Unfortunately, the naming of this field is a bit misleading. This is not actually "retry attempts", it actually
// is attempts. Setting this to a value of 0 will cause a panic in Go AutoRest.
c.RetryAttempts = 1
AutoRestClientAppendUserAgent(c, UserAgent())
}

// AutoRestClientAppendUserAgent autorest client calls "AddToUserAgent" but ignores errors.
func AutoRestClientAppendUserAgent(c *autorest.Client, extension string) {
_ = c.AddToUserAgent(extension) // intentionally ignore error as it doesn't matter
}

func msCorrelationIDSendDecorator(snd autorest.Sender) autorest.Sender {
return autorest.SenderFunc(func(r *http.Request) (*http.Response, error) {
// if the correlation ID was found in the request context, set
// it in the header
if corrID, ok := tele.CorrIDFromCtx(r.Context()); ok {
r.Header.Set(string(tele.CorrIDKeyVal), string(corrID))
}
return snd.Do(r)
})
}
91 changes: 0 additions & 91 deletions azure/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"sync"
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
"github.com/Azure/go-autorest/autorest"
. "github.com/onsi/gomega"
"go.uber.org/mock/gomock"
"sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure"
Expand Down Expand Up @@ -213,95 +211,6 @@ func defaultTestPipeline(policies []policy.Policy) runtime.Pipeline {
)
}

func TestAutoRestClientAppendUserAgent(t *testing.T) {
g := NewWithT(t)
userAgent := "cluster-api-provider-azure/2.29.2"

type args struct {
c *autorest.Client
extension string
}
tests := []struct {
name string
args args
want string
}{
{
name: "should append extension to user agent if extension is not empty",
args: args{
c: &autorest.Client{UserAgent: autorest.UserAgent()},
extension: userAgent,
},
want: fmt.Sprintf("%s %s", autorest.UserAgent(), userAgent),
},
{
name: "should no changed if extension is empty",
args: args{
c: &autorest.Client{UserAgent: userAgent},
extension: "",
},
want: userAgent,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
AutoRestClientAppendUserAgent(tt.args.c, tt.args.extension)

g.Expect(tt.want).To(Equal(tt.args.c.UserAgent))
})
}
}

func TestMSCorrelationIDSendDecorator(t *testing.T) {
g := NewWithT(t)
const corrID tele.CorrID = "TestMSCorrelationIDSendDecoratorCorrID"
ctx := context.WithValue(context.Background(), tele.CorrIDKeyVal, corrID)

// create a fake server so that the sender can send to
// somewhere
var wg sync.WaitGroup
receivedReqs := []*http.Request{}
wg.Add(1)
originHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
receivedReqs = append(receivedReqs, r)
wg.Done()
})

testSrv := httptest.NewServer(originHandler)
defer testSrv.Close()

// create a sender that sends to the fake server, then
// decorate the sender with the msCorrelationIDSendDecorator
origSender := autorest.SenderFunc(func(r *http.Request) (*http.Response, error) {
// preserve the incoming headers to the fake server, so that
// we can test that the fake server received the right
// correlation ID header.
req, err := http.NewRequest(http.MethodGet, testSrv.URL, http.NoBody)
if err != nil {
return nil, err
}
req = req.WithContext(ctx)
req.Header = r.Header
return testSrv.Client().Do(req)
})
newSender := autorest.DecorateSender(origSender, msCorrelationIDSendDecorator)

// create a new HTTP request and send it via the new decorated sender
req, err := http.NewRequest(http.MethodGet, "/abc", http.NoBody)
g.Expect(err).NotTo(HaveOccurred())

req = req.WithContext(ctx)
rsp, err := newSender.Do(req)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(rsp.Body.Close()).To(Succeed())
wg.Wait()
g.Expect(len(receivedReqs)).To(Equal(1))
receivedReq := receivedReqs[0]
g.Expect(
receivedReq.Header.Get(string(tele.CorrIDKeyVal)),
).To(Equal(string(corrID)))
}

func TestGetBootstrappingVMExtension(t *testing.T) {
testCases := []struct {
name string
Expand Down
5 changes: 0 additions & 5 deletions azure/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ func ResourceNotFound(err error) bool {
return hasStatusCode(err, http.StatusNotFound)
}

// ResourceConflict parses an error to check if its status code is Conflict (409).
func ResourceConflict(err error) bool {
return hasStatusCode(err, http.StatusConflict)
}

// hasStatusCode returns true if an error is a DetailedError or ResponseError with a matching status code.
func hasStatusCode(err error, statusCode int) bool {
derr := autorest.DetailedError{} // azure-sdk-for-go v1
Expand Down
43 changes: 0 additions & 43 deletions azure/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,46 +97,3 @@ func TestResourceNotFound(t *testing.T) {
})
}
}

func TestResourceConflict(t *testing.T) {
tests := []struct {
name string
err error
success bool
}{
{
name: "Not Found detailed error",
err: autorest.DetailedError{StatusCode: http.StatusNotFound},
success: false,
},
{
name: "Conflict detailed error",
err: autorest.DetailedError{StatusCode: http.StatusConflict},
success: true,
},
{
name: "Not Found response error",
err: &azcore.ResponseError{StatusCode: http.StatusNotFound},
success: false,
},
{
name: "Conflict response error",
err: &azcore.ResponseError{StatusCode: http.StatusConflict},
success: true,
},
{
name: "Conflict generic error",
err: errors.New("409: Conflict"),
success: false,
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
if got := ResourceConflict(tc.err); got != tc.success {
t.Errorf("ResourceConflict() = %v, want %v", got, tc.success)
}
})
}
}

0 comments on commit 0f43bda

Please sign in to comment.