diff --git a/azure/converters/futures.go b/azure/converters/futures.go index 4d4950b428f..2de5d714ed1 100644 --- a/azure/converters/futures.go +++ b/azure/converters/futures.go @@ -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() diff --git a/azure/converters/futures_test.go b/azure/converters/futures_test.go index fd660d14fba..345366e5551 100644 --- a/azure/converters/futures_test.go +++ b/azure/converters/futures_test.go @@ -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", @@ -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 diff --git a/azure/defaults.go b/azure/defaults.go index e28acd11332..d281d244dbb 100644 --- a/azure/defaults.go +++ b/azure/defaults.go @@ -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" ) @@ -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) - }) -} diff --git a/azure/defaults_test.go b/azure/defaults_test.go index 3ca65db3c9a..994a5bd43df 100644 --- a/azure/defaults_test.go +++ b/azure/defaults_test.go @@ -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" @@ -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 diff --git a/azure/errors.go b/azure/errors.go index 24d53b6ea58..1a86e12cc4c 100644 --- a/azure/errors.go +++ b/azure/errors.go @@ -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 diff --git a/azure/errors_test.go b/azure/errors_test.go index 5c4c868bc4d..391e917154c 100644 --- a/azure/errors_test.go +++ b/azure/errors_test.go @@ -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) - } - }) - } -}