diff --git a/cmd/githubrunnerscalesetlistener/main.go b/cmd/githubrunnerscalesetlistener/main.go index 2c9d71b2..06684597 100644 --- a/cmd/githubrunnerscalesetlistener/main.go +++ b/cmd/githubrunnerscalesetlistener/main.go @@ -84,7 +84,13 @@ func run(rc RunnerScaleSetListenerConfig, logger logr.Logger) error { } } - actionsServiceClient, err := actions.NewClient(ctx, rc.ConfigureUrl, creds, fmt.Sprintf("actions-runner-controller/%s", build.Version), logger) + actionsServiceClient, err := actions.NewClient( + ctx, + rc.ConfigureUrl, + creds, + actions.WithUserAgent(fmt.Sprintf("actions-runner-controller/%s", build.Version)), + actions.WithLogger(logger), + ) if err != nil { return fmt.Errorf("failed to create an Actions Service client: %w", err) } diff --git a/github/actions/actions_server_test.go b/github/actions/actions_server_test.go new file mode 100644 index 00000000..8b66e45f --- /dev/null +++ b/github/actions/actions_server_test.go @@ -0,0 +1,81 @@ +package actions_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/stretchr/testify/require" +) + +// newActionsServer returns a new httptest.Server that handles the +// authentication requests neeeded to create a new client. Any requests not +// made to the /actions/runners/registration-token or +// /actions/runner-registration endpoints will be handled by the provided +// handler. The returned server is started and will be automatically closed +// when the test ends. +func newActionsServer(t *testing.T, handler http.Handler) *actionsServer { + var u string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // handle getRunnerRegistrationToken + if strings.HasSuffix(r.URL.Path, "/runners/registration-token") { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"token":"token"}`)) + return + } + + // handle getActionsServiceAdminConnection + if strings.HasSuffix(r.URL.Path, "/actions/runner-registration") { + claims := &jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(-1 * time.Minute)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Minute)), + Issuer: "123", + } + + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + privateKey, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(samplePrivateKey)) + require.NoError(t, err) + tokenString, err := token.SignedString(privateKey) + require.NoError(t, err) + w.Write([]byte(`{"url":"` + u + `","token":"` + tokenString + `"}`)) + return + } + + handler.ServeHTTP(w, r) + })) + + u = server.URL + + t.Cleanup(func() { + server.Close() + }) + + return &actionsServer{server} +} + +type actionsServer struct { + *httptest.Server +} + +func (s *actionsServer) configURLForOrg(org string) string { + return s.URL + "/" + org +} + +const samplePrivateKey = `-----BEGIN RSA PRIVATE KEY----- +MIICWgIBAAKBgHXfRT9cv9UY9fAAD4+1RshpfSSZe277urfEmPfX3/Og9zJYRk// +CZrJVD1CaBZDiIyQsNEzjta7r4UsqWdFOggiNN2E7ZTFQjMSaFkVgrzHqWuiaCBf +/BjbKPn4SMDmTzHvIe7Nel76hBdCaVgu6mYCW5jmuSH5qz/yR1U1J/WJAgMBAAEC +gYARWGWsSU3BYgbu5lNj5l0gKMXNmPhdAJYdbMTF0/KUu18k/XB7XSBgsre+vALt +I8r4RGKApoGif8P4aPYUyE8dqA1bh0X3Fj1TCz28qoUL5//dA+pigCRS20H7HM3C +ojoqF7+F+4F2sXmzFNd1NgY5RxFPYosTT7OnUiFuu2IisQJBALnMLe09LBnjuHXR +xxR65DDNxWPQLBjW3dL+ubLcwr7922l6ZIQsVjdeE0ItEUVRjjJ9/B/Jq9VJ/Lw4 +g9LCkkMCQQCiaM2f7nYmGivPo9hlAbq5lcGJ5CCYFfeeYzTxMqum7Mbqe4kk5lgb +X6gWd0Izg2nGdAEe/97DClO6VpKcPbpDAkBTR/JOJN1fvXMxXJaf13XxakrQMr+R +Yr6LlSInykyAz8lJvlLP7A+5QbHgN9NF/wh+GXqpxPwA3ukqdSqhjhWBAkBn6mDv +HPgR5xrzL6XM8y9TgaOlJAdK6HtYp6d/UOmN0+Butf6JUq07TphRT5tXNJVgemch +O5x/9UKfbrc+KyzbAkAo97TfFC+mZhU1N5fFelaRu4ikPxlp642KRUSkOh8GEkNf +jQ97eJWiWtDcsMUhcZgoB5ydHcFlrBIn6oBcpge5 +-----END RSA PRIVATE KEY-----` diff --git a/github/actions/client.go b/github/actions/client.go index 8e029086..f52d2b95 100644 --- a/github/actions/client.go +++ b/github/actions/client.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "io" + "log" "net/http" "net/url" "path" @@ -62,8 +63,8 @@ type Client struct { ActionsServiceAdminTokenExpiresAt *time.Time ActionsServiceURL *string - RetryMax *int - RetryWaitMax *time.Duration + retryMax int + retryWaitMax time.Duration creds *ActionsAuth githubConfigURL string @@ -71,14 +72,57 @@ type Client struct { userAgent string } -func NewClient(ctx context.Context, githubConfigURL string, creds *ActionsAuth, userAgent string, logger logr.Logger) (ActionsService, error) { +type ClientOption func(*Client) + +func WithUserAgent(userAgent string) ClientOption { + return func(c *Client) { + c.userAgent = userAgent + } +} + +func WithLogger(logger logr.Logger) ClientOption { + return func(c *Client) { + c.logger = logger + } +} + +func WithRetryMax(retryMax int) ClientOption { + return func(c *Client) { + c.retryMax = retryMax + } +} + +func WithRetryWaitMax(retryWaitMax time.Duration) ClientOption { + return func(c *Client) { + c.retryWaitMax = retryWaitMax + } +} + +func NewClient(ctx context.Context, githubConfigURL string, creds *ActionsAuth, options ...ClientOption) (ActionsService, error) { ac := &Client{ creds: creds, githubConfigURL: githubConfigURL, - logger: logger, - userAgent: userAgent, + logger: logr.Discard(), + + // retryablehttp defaults + retryMax: 4, + retryWaitMax: 30 * time.Second, } + for _, option := range options { + option(ac) + } + + retryClient := retryablehttp.NewClient() + + // TODO: this silences retryclient default logger, do we want to provide one + // instead? by default retryablehttp logs all requests to stderr + retryClient.Logger = log.New(io.Discard, "", log.LstdFlags) + + retryClient.RetryMax = ac.retryMax + retryClient.RetryWaitMax = ac.retryWaitMax + ac.Client = retryClient.StandardClient() + rt, err := ac.getRunnerRegistrationToken(ctx, githubConfigURL, *creds) if err != nil { return nil, fmt.Errorf("failed to get runner registration token: %w", err) @@ -121,9 +165,7 @@ func (c *Client) GetRunnerScaleSet(ctx context.Context, runnerScaleSetName strin req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -165,9 +207,7 @@ func (c *Client) GetRunnerScaleSetById(ctx context.Context, runnerScaleSetId int req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -182,7 +222,6 @@ func (c *Client) GetRunnerScaleSetById(ctx context.Context, runnerScaleSetId int return nil, err } return runnerScaleSet, nil - } func (c *Client) GetRunnerGroupByName(ctx context.Context, runnerGroup string) (*RunnerGroup, error) { @@ -204,9 +243,7 @@ func (c *Client) GetRunnerGroupByName(ctx context.Context, runnerGroup string) ( req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -260,9 +297,7 @@ func (c *Client) CreateRunnerScaleSet(ctx context.Context, runnerScaleSet *Runne req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -278,49 +313,6 @@ func (c *Client) CreateRunnerScaleSet(ctx context.Context, runnerScaleSet *Runne return createdRunnerScaleSet, nil } -func (c *Client) UpdateRunnerScaleSet(ctx context.Context, runnerScaleSetId int, runnerScaleSet *RunnerScaleSet) (*RunnerScaleSet, error) { - u := fmt.Sprintf("%s/%s/%d?api-version=6.0-preview", *c.ActionsServiceURL, scaleSetEndpoint, runnerScaleSetId) - - if err := c.refreshTokenIfNeeded(ctx); err != nil { - return nil, fmt.Errorf("failed to refresh admin token if needed: %w", err) - } - - body, err := json.Marshal(runnerScaleSet) - if err != nil { - return nil, err - } - - req, err := http.NewRequestWithContext(ctx, http.MethodPut, u, bytes.NewBuffer(body)) - if err != nil { - return nil, err - } - - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", *c.ActionsServiceAdminToken)) - - if c.userAgent != "" { - req.Header.Set("User-Agent", c.userAgent) - } - - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) - if err != nil { - return nil, err - } - - if resp.StatusCode != http.StatusOK { - return nil, ParseActionsErrorFromResponse(resp) - } - - var createdRunnerScaleSet *RunnerScaleSet - err = unmarshalBody(resp, &createdRunnerScaleSet) - if err != nil { - return nil, err - } - return createdRunnerScaleSet, nil -} - func (c *Client) DeleteRunnerScaleSet(ctx context.Context, runnerScaleSetId int) error { u := fmt.Sprintf("%s/%s/%d?api-version=6.0-preview", *c.ActionsServiceURL, scaleSetEndpoint, runnerScaleSetId) @@ -340,9 +332,7 @@ func (c *Client) DeleteRunnerScaleSet(ctx context.Context, runnerScaleSetId int) req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return err } @@ -372,9 +362,7 @@ func (c *Client) GetMessage(ctx context.Context, messageQueueUrl, messageQueueAc req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -425,9 +413,7 @@ func (c *Client) DeleteMessage(ctx context.Context, messageQueueUrl, messageQueu req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return err } @@ -497,9 +483,7 @@ func (c *Client) doSessionRequest(ctx context.Context, method, url string, reque req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return err } @@ -542,9 +526,7 @@ func (c *Client) AcquireJobs(ctx context.Context, runnerScaleSetId int, messageQ req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -581,9 +563,7 @@ func (c *Client) GetAcquirableJobs(ctx context.Context, runnerScaleSetId int) (* req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -629,9 +609,7 @@ func (c *Client) GenerateJitRunnerConfig(ctx context.Context, jitRunnerSetting * req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -667,9 +645,7 @@ func (c *Client) GetRunner(ctx context.Context, runnerId int64) (*RunnerReferenc req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -705,9 +681,7 @@ func (c *Client) GetRunnerByName(ctx context.Context, runnerName string) (*Runne req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return nil, err } @@ -752,9 +726,7 @@ func (c *Client) RemoveRunner(ctx context.Context, runnerId int64) error { req.Header.Set("User-Agent", c.userAgent) } - httpClient := c.getHTTPClient() - - resp, err := httpClient.Do(req) + resp, err := c.Do(req) if err != nil { return err } @@ -1012,24 +984,6 @@ func createJWTForGitHubApp(appAuth *GitHubAppAuth) (string, error) { return token.SignedString(privateKey) } -func (c *Client) getHTTPClient() *http.Client { - if c.Client != nil { - return c.Client - } - - retryClient := retryablehttp.NewClient() - - if c.RetryMax != nil { - retryClient.RetryMax = *c.RetryMax - } - - if c.RetryWaitMax != nil { - retryClient.RetryWaitMax = *c.RetryWaitMax - } - - return retryClient.StandardClient() -} - func unmarshalBody(response *http.Response, v interface{}) (err error) { if response != nil && response.Body != nil { var err error diff --git a/github/actions/client_generate_jit_test.go b/github/actions/client_generate_jit_test.go index 1b1d7330..cf594151 100644 --- a/github/actions/client_generate_jit_test.go +++ b/github/actions/client_generate_jit_test.go @@ -3,73 +3,60 @@ package actions_test import ( "context" "net/http" - "net/http/httptest" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGenerateJitRunnerConfig(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Get JIT Config for Runner", func(t *testing.T) { - name := "Get JIT Config for Runner" want := &actions.RunnerScaleSetJitRunnerConfig{} response := []byte(`{"count":1,"value":[{"id":1,"name":"scale-set-name"}]}`) runnerSettings := &actions.RunnerScaleSetJitRunnerSetting{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(response) })) - defer s.Close() + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - got, err := actionsClient.GenerateJitRunnerConfig(context.Background(), runnerSettings, 1) - if err != nil { - t.Fatalf("GenerateJitRunnerConfig got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GenerateJitRunnerConfig(%v) mismatch (-want +got):\n%s", name, diff) - } + got, err := client.GenerateJitRunnerConfig(ctx, runnerSettings, 1) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Default retries on server error", func(t *testing.T) { runnerSettings := &actions.RunnerScaleSetJitRunnerSetting{} - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 - + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _, _ = actionsClient.GenerateJitRunnerConfig(context.Background(), runnerSettings, 1) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(1), + actions.WithRetryWaitMax(1*time.Millisecond), + ) + require.NoError(t, err) + _, err = client.GenerateJitRunnerConfig(ctx, runnerSettings, 1) + assert.NotNil(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } diff --git a/github/actions/client_job_acquisition_test.go b/github/actions/client_job_acquisition_test.go index b7df3abb..dfd0d58d 100644 --- a/github/actions/client_job_acquisition_test.go +++ b/github/actions/client_job_acquisition_test.go @@ -3,22 +3,21 @@ package actions_test import ( "context" "net/http" - "net/http/httptest" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAcquireJobs(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Acquire Job", func(t *testing.T) { - name := "Acquire Job" - want := []int64{1} response := []byte(`{"value": [1]}`) @@ -28,24 +27,16 @@ func TestAcquireJobs(t *testing.T) { } requestIDs := want - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.AcquireJobs(context.Background(), session.RunnerScaleSet.Id, session.MessageQueueAccessToken, requestIDs) - if err != nil { - t.Fatalf("CreateRunnerScaleSet got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerScaleSet(%v) mismatch (-want +got):\n%s", name, diff) - } + got, err := client.AcquireJobs(ctx, session.RunnerScaleSet.Id, session.MessageQueueAccessToken, requestIDs) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Default retries on server error", func(t *testing.T) { @@ -55,90 +46,78 @@ func TestAcquireJobs(t *testing.T) { } var requestIDs []int64 = []int64{1} - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 - + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - } - - _, _ = actionsClient.AcquireJobs(context.Background(), session.RunnerScaleSet.Id, session.MessageQueueAccessToken, requestIDs) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(1*time.Millisecond), + ) + require.NoError(t, err) + _, err = client.AcquireJobs(context.Background(), session.RunnerScaleSet.Id, session.MessageQueueAccessToken, requestIDs) + assert.NotNil(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } func TestGetAcquirableJobs(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Acquire Job", func(t *testing.T) { - name := "Acquire Job" - want := &actions.AcquirableJobList{} response := []byte(`{"count": 0}`) runnerScaleSet := &actions.RunnerScaleSet{Id: 1} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.GetAcquirableJobs(context.Background(), runnerScaleSet.Id) - if err != nil { - t.Fatalf("GetAcquirableJobs got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetAcquirableJobs(%v) mismatch (-want +got):\n%s", name, diff) - } + got, err := client.GetAcquirableJobs(context.Background(), runnerScaleSet.Id) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Default retries on server error", func(t *testing.T) { runnerScaleSet := &actions.RunnerScaleSet{Id: 1} - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _, _ = actionsClient.GetAcquirableJobs(context.Background(), runnerScaleSet.Id) + client, err := actions.NewClient( + context.Background(), + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(1*time.Millisecond), + ) + require.NoError(t, err) + _, err = client.GetAcquirableJobs(context.Background(), runnerScaleSet.Id) + require.Error(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } diff --git a/github/actions/client_runner_scale_set_message_test.go b/github/actions/client_runner_scale_set_message_test.go index 2252f5a7..55e80267 100644 --- a/github/actions/client_runner_scale_set_message_test.go +++ b/github/actions/client_runner_scale_set_message_test.go @@ -5,18 +5,20 @@ import ( "encoding/json" "errors" "net/http" - "net/http/httptest" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestGetMessage(t *testing.T) { + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" runnerScaleSetMessage := &actions.RunnerScaleSetMessage{ MessageId: 1, @@ -26,89 +28,54 @@ func TestGetMessage(t *testing.T) { t.Run("Get Runner Scale Set Message", func(t *testing.T) { want := runnerScaleSetMessage response := []byte(`{"messageId":1,"messageType":"rssType"}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + s := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, s.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.GetMessage(context.Background(), s.URL, token, 0) - if err != nil { - t.Fatalf("GetMessage got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetMessage mismatch (-want +got):\n%s", diff) - } + got, err := client.GetMessage(ctx, s.URL, token, 0) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Default retries on server error", func(t *testing.T) { - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Nanosecond - retryClient.RetryMax = 1 + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _, _ = actionsClient.GetMessage(context.Background(), s.URL, token, 0) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(1*time.Millisecond), + ) + require.NoError(t, err) + _, err = client.GetMessage(ctx, server.URL, token, 0) + assert.NotNil(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryMax := 1 - retryWaitMax := 1 * time.Nanosecond - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _, _ = actionsClient.GetMessage(context.Background(), s.URL, token, 0) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - t.Run("Message token expired", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusUnauthorized) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetMessage(context.Background(), s.URL, token, 0) - if err == nil { - t.Fatalf("GetMessage did not get exepected error, ") - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetMessage(ctx, server.URL, token, 0) + require.NotNil(t, err) + var expectedErr *actions.MessageQueueTokenExpiredError require.True(t, errors.As(err, &expectedErr)) }, @@ -119,45 +86,38 @@ func TestGetMessage(t *testing.T) { Message: "Request returned status: 404 Not Found", StatusCode: 404, } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetMessage(context.Background(), s.URL, token, 0) - if err == nil { - t.Fatalf("GetMessage did not get exepected error, ") - } - if diff := cmp.Diff(want.Error(), err.Error()); diff != "" { - t.Errorf("GetMessage mismatch (-want +got):\n%s", diff) - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetMessage(ctx, server.URL, token, 0) + require.NotNil(t, err) + assert.Equal(t, want.Error(), err.Error()) + }) t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "text/plain") })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetMessage(context.Background(), s.URL, token, 0) - if err == nil { - t.Fatalf("GetMessage did not get exepected error,") - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetMessage(ctx, server.URL, token, 0) + assert.NotNil(t, err) + }) } func TestDeleteMessage(t *testing.T) { + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" runnerScaleSetMessage := &actions.RunnerScaleSetMessage{ MessageId: 1, @@ -165,105 +125,83 @@ func TestDeleteMessage(t *testing.T) { } t.Run("Delete existing message", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteMessage(context.Background(), s.URL, token, runnerScaleSetMessage.MessageId) - if err != nil { - t.Fatalf("DeleteMessage got unexepected error, %v", err) - } - }, - ) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + err = client.DeleteMessage(ctx, server.URL, token, runnerScaleSetMessage.MessageId) + assert.Nil(t, err) + }) t.Run("Message token expired", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusUnauthorized) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteMessage(context.Background(), s.URL, token, 0) - if err == nil { - t.Fatalf("DeleteMessage did not get exepected error, ") - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + err = client.DeleteMessage(ctx, server.URL, token, 0) + require.NotNil(t, err) var expectedErr *actions.MessageQueueTokenExpiredError - require.True(t, errors.As(err, &expectedErr)) - }, - ) + assert.True(t, errors.As(err, &expectedErr)) + }) t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "text/plain") })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteMessage(context.Background(), s.URL, token, runnerScaleSetMessage.MessageId) - if err == nil { - t.Fatalf("DeleteMessage did not get exepected error") - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + err = client.DeleteMessage(ctx, server.URL, token, runnerScaleSetMessage.MessageId) + require.NotNil(t, err) var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) + assert.True(t, errors.As(err, &expectedErr)) }, ) t.Run("Default retries on server error", func(t *testing.T) { actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - retryClient := retryablehttp.NewClient() + retryMax := 1 - retryClient.RetryWaitMax = time.Nanosecond - retryClient.RetryMax = retryMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _ = actionsClient.DeleteMessage(context.Background(), s.URL, token, runnerScaleSetMessage.MessageId) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(1*time.Nanosecond), + ) + require.NoError(t, err) + err = client.DeleteMessage(ctx, server.URL, token, runnerScaleSetMessage.MessageId) + assert.NotNil(t, err) expectedRetry := retryMax + 1 assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) + }) t.Run("No message found", func(t *testing.T) { want := (*actions.RunnerScaleSetMessage)(nil) rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + require.NoError(t, err) + + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(rsl) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err = actionsClient.DeleteMessage(context.Background(), s.URL, token, runnerScaleSetMessage.MessageId+1) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + err = client.DeleteMessage(ctx, server.URL, token, runnerScaleSetMessage.MessageId+1) var expectedErr *actions.ActionsError require.True(t, errors.As(err, &expectedErr)) - }, - ) + }) } diff --git a/github/actions/client_runner_scale_set_session_test.go b/github/actions/client_runner_scale_set_session_test.go index e3fc3193..f5fbceb7 100644 --- a/github/actions/client_runner_scale_set_session_test.go +++ b/github/actions/client_runner_scale_set_session_test.go @@ -4,19 +4,22 @@ import ( "context" "errors" "net/http" - "net/http/httptest" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreateMessageSession(t *testing.T) { + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + t.Run("CreateMessageSession unmarshals correctly", func(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" owner := "foo" runnerScaleSet := actions.RunnerScaleSet{ Id: 1, @@ -35,7 +38,7 @@ func TestCreateMessageSession(t *testing.T) { MessageQueueAccessToken: "fake.jwt.here", } - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { resp := []byte(`{ "ownerName": "foo", "runnerScaleSet": { @@ -47,31 +50,16 @@ func TestCreateMessageSession(t *testing.T) { }`) w.Write(resp) })) - defer srv.Close() - retryMax := 1 - retryWaitMax := 1 * time.Microsecond + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - actionsClient := actions.Client{ - ActionsServiceURL: &srv.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - - got, err := actionsClient.CreateMessageSession(context.Background(), runnerScaleSet.Id, owner) - if err != nil { - t.Fatalf("CreateMessageSession got unexpected error: %v", err) - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Fatalf("CreateMessageSession got unexpected diff: -want +got: %v", diff) - } + got, err := client.CreateMessageSession(ctx, runnerScaleSet.Id, owner) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("CreateMessageSession unmarshals errors into ActionsError", func(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" owner := "foo" runnerScaleSet := actions.RunnerScaleSet{ Id: 1, @@ -86,44 +74,32 @@ func TestCreateMessageSession(t *testing.T) { StatusCode: http.StatusBadRequest, } - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) resp := []byte(`{"typeName": "CSharpExceptionNameHere","message": "could not do something"}`) w.Write(resp) })) - defer srv.Close() - retryMax := 1 - retryWaitMax := 1 * time.Microsecond + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - actionsClient := actions.Client{ - ActionsServiceURL: &srv.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - - got, err := actionsClient.CreateMessageSession(context.Background(), runnerScaleSet.Id, owner) - if err == nil { - t.Fatalf("CreateMessageSession did not get expected error: %v", got) - } + _, err = client.CreateMessageSession(ctx, runnerScaleSet.Id, owner) + require.NotNil(t, err) errorTypeForComparison := &actions.ActionsError{} - if isActionsError := errors.As(err, &errorTypeForComparison); !isActionsError { - t.Fatalf("CreateMessageSession expected to be able to parse the error into ActionsError type: %v", err) - } + assert.True( + t, + errors.As(err, &errorTypeForComparison), + "CreateMessageSession expected to be able to parse the error into ActionsError type: %v", + err, + ) gotErr := err.(*actions.ActionsError) - - if diff := cmp.Diff(want, gotErr); diff != "" { - t.Fatalf("CreateMessageSession got unexpected diff: -want +got: %v", diff) - } + assert.Equal(t, want, gotErr) }) t.Run("CreateMessageSession call is retried the correct amount of times", func(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" owner := "foo" runnerScaleSet := actions.RunnerScaleSet{ Id: 1, @@ -133,37 +109,38 @@ func TestCreateMessageSession(t *testing.T) { } gotRetries := 0 - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusInternalServerError) gotRetries++ })) - defer srv.Close() retryMax := 3 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } + retryWaitMax := 1 * time.Microsecond wantRetries := retryMax + 1 - actionsClient := actions.Client{ - ActionsServiceURL: &srv.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - - _, _ = actionsClient.CreateMessageSession(context.Background(), runnerScaleSet.Id, owner) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) + _, err = client.CreateMessageSession(ctx, runnerScaleSet.Id, owner) + assert.NotNil(t, err) assert.Equalf(t, gotRetries, wantRetries, "CreateMessageSession got unexpected retry count: got=%v, want=%v", gotRetries, wantRetries) }) } func TestDeleteMessageSession(t *testing.T) { + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + t.Run("DeleteMessageSession call is retried the correct amount of times", func(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" runnerScaleSet := actions.RunnerScaleSet{ Id: 1, Name: "ScaleSet", @@ -172,39 +149,40 @@ func TestDeleteMessageSession(t *testing.T) { } gotRetries := 0 - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusInternalServerError) gotRetries++ })) - defer srv.Close() retryMax := 3 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } + retryWaitMax := 1 * time.Microsecond wantRetries := retryMax + 1 - actionsClient := actions.Client{ - ActionsServiceURL: &srv.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) sessionId := uuid.New() - _ = actionsClient.DeleteMessageSession(context.Background(), runnerScaleSet.Id, &sessionId) - + err = client.DeleteMessageSession(ctx, runnerScaleSet.Id, &sessionId) + assert.NotNil(t, err) assert.Equalf(t, gotRetries, wantRetries, "CreateMessageSession got unexpected retry count: got=%v, want=%v", gotRetries, wantRetries) }) } func TestRefreshMessageSession(t *testing.T) { + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + t.Run("RefreshMessageSession call is retried the correct amount of times", func(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" runnerScaleSet := actions.RunnerScaleSet{ Id: 1, Name: "ScaleSet", @@ -213,32 +191,29 @@ func TestRefreshMessageSession(t *testing.T) { } gotRetries := 0 - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusInternalServerError) gotRetries++ })) - defer srv.Close() retryMax := 3 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } + retryWaitMax := 1 * time.Microsecond wantRetries := retryMax + 1 - actionsClient := actions.Client{ - ActionsServiceURL: &srv.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) sessionId := uuid.New() - _, _ = actionsClient.RefreshMessageSession(context.Background(), runnerScaleSet.Id, &sessionId) - + _, err = client.RefreshMessageSession(context.Background(), runnerScaleSet.Id, &sessionId) + assert.NotNil(t, err) assert.Equalf(t, gotRetries, wantRetries, "CreateMessageSession got unexpected retry count: got=%v, want=%v", gotRetries, wantRetries) }) } diff --git a/github/actions/client_runner_scale_set_test.go b/github/actions/client_runner_scale_set_test.go index fb4ba2c5..e9d86e2d 100644 --- a/github/actions/client_runner_scale_set_test.go +++ b/github/actions/client_runner_scale_set_test.go @@ -6,853 +6,333 @@ import ( "errors" "fmt" "net/http" - "net/http/httptest" "net/url" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestGetRunnerScaleSet(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + scaleSetName := "ScaleSet" runnerScaleSet := actions.RunnerScaleSet{Id: 1, Name: scaleSetName} t.Run("Get existing scale set", func(t *testing.T) { want := &runnerScaleSet runnerScaleSetsResp := []byte(`{"count":1,"value":[{"id":1,"name":"ScaleSet"}]}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(runnerScaleSetsResp) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, err := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) - if err != nil { - t.Fatalf("CreateRunnerScaleSet got unexepected error, %v", err) - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerScaleSet(%v) mismatch (-want +got):\n%s", scaleSetName, diff) - } - }, - ) + got, err := client.GetRunnerScaleSet(ctx, scaleSetName) + require.NoError(t, err) + assert.Equal(t, want, got) + }) t.Run("GetRunnerScaleSet calls correct url", func(t *testing.T) { runnerScaleSetsResp := []byte(`{"count":1,"value":[{"id":1,"name":"ScaleSet"}]}`) url := url.URL{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(runnerScaleSetsResp) url = *r.URL })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) - if err != nil { - t.Fatalf("CreateRunnerScaleSet got unexepected error, %v", err) - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + require.NoError(t, err) u := url.String() expectedUrl := fmt.Sprintf("/_apis/runtime/runnerscalesets?name=%s&api-version=6.0-preview", scaleSetName) assert.Equal(t, expectedUrl, u) - - }, - ) + }) t.Run("Status code not found", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) - if err == nil { - t.Fatalf("GetRunnerScaleSet did not get exepected error, ") - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + assert.NotNil(t, err) + }) t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "text/plain") })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) - if err == nil { - t.Fatalf("GetRunnerScaleSet did not get exepected error,") - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + assert.NotNil(t, err) + }) t.Run("Default retries on server error", func(t *testing.T) { actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - retryClient := retryablehttp.NewClient() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - retryClient.RetryWaitMax = retryWaitMax - retryClient.RetryMax = retryMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, _ = actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _, _ = actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) + retryWaitMax := 1 * time.Microsecond + + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + assert.NotNil(t, err) expectedRetry := retryMax + 1 assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) + }) t.Run("RunnerScaleSet count is zero", func(t *testing.T) { want := (*actions.RunnerScaleSet)(nil) runnerScaleSetsResp := []byte(`{"count":0,"value":[{"id":1,"name":"ScaleSet"}]}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(runnerScaleSetsResp) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, _ := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerScaleSet(%v) mismatch (-want +got):\n%s", scaleSetName, diff) - } - - }, - ) + got, err := client.GetRunnerScaleSet(ctx, scaleSetName) + require.NoError(t, err) + assert.Equal(t, want, got) + }) t.Run("Multiple runner scale sets found", func(t *testing.T) { wantErr := fmt.Errorf("multiple runner scale sets found with name %s", scaleSetName) runnerScaleSetsResp := []byte(`{"count":2,"value":[{"id":1,"name":"ScaleSet"}]}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(runnerScaleSetsResp) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSet(context.Background(), scaleSetName) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - if err == nil { - t.Fatalf("GetRunnerScaleSet did not get exepected error, %v", wantErr) - } - - if diff := cmp.Diff(wantErr.Error(), err.Error()); diff != "" { - t.Errorf("GetRunnerScaleSet(%v) mismatch (-want +got):\n%s", scaleSetName, diff) - } - - }, - ) + _, err = client.GetRunnerScaleSet(ctx, scaleSetName) + require.NotNil(t, err) + assert.Equal(t, wantErr.Error(), err.Error()) + }) } func TestGetRunnerScaleSetById(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + scaleSetCreationDateTime := time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC) runnerScaleSet := actions.RunnerScaleSet{Id: 1, Name: "ScaleSet", CreatedOn: scaleSetCreationDateTime, RunnerSetting: actions.RunnerSetting{}} t.Run("Get existing scale set by Id", func(t *testing.T) { want := &runnerScaleSet rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + require.NoError(t, err) + sservere := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(rsl) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, err := actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) - if err != nil { - t.Fatalf("GetRunnerScaleSetById got unexepected error, %v", err) - } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerScaleSetById(%d) mismatch (-want +got):\n%s", runnerScaleSet.Id, diff) - } - }, - ) + client, err := actions.NewClient(ctx, sservere.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + got, err := client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + require.NoError(t, err) + assert.Equal(t, want, got) + }) t.Run("GetRunnerScaleSetById calls correct url", func(t *testing.T) { rsl, err := json.Marshal(&runnerScaleSet) - if err != nil { - t.Fatalf("%v", err) - } + require.NoError(t, err) + url := url.URL{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(rsl) url = *r.URL })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err = actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) - if err != nil { - t.Fatalf("GetRunnerScaleSetById got unexepected error, %v", err) - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + require.NoError(t, err) u := url.String() expectedUrl := fmt.Sprintf("/_apis/runtime/runnerscalesets/%d?api-version=6.0-preview", runnerScaleSet.Id) assert.Equal(t, expectedUrl, u) - - }, - ) + }) t.Run("Status code not found", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) - if err == nil { - t.Fatalf("GetRunnerScaleSetById did not get exepected error, ") - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + assert.NotNil(t, err) + }) t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "text/plain") })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) - if err == nil { - t.Fatalf("GetRunnerScaleSetById did not get exepected error,") - } - }, - ) + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + assert.NotNil(t, err) + }) t.Run("Default retries on server error", func(t *testing.T) { actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - retryClient := retryablehttp.NewClient() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - retryClient.RetryWaitMax = retryWaitMax - retryClient.RetryMax = retryMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, _ = actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _, _ = actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) + retryWaitMax := 1 * time.Microsecond + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) + + _, err = client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + require.NotNil(t, err) expectedRetry := retryMax + 1 assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) + }) t.Run("No RunnerScaleSet found", func(t *testing.T) { want := (*actions.RunnerScaleSet)(nil) rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + require.NoError(t, err) + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(rsl) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, _ := actionsClient.GetRunnerScaleSetById(context.Background(), runnerScaleSet.Id) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerScaleSetById(%v) mismatch (-want +got):\n%s", runnerScaleSet.Id, diff) - } - - }, - ) + got, err := client.GetRunnerScaleSetById(ctx, runnerScaleSet.Id) + require.NoError(t, err) + assert.Equal(t, want, got) + }) } func TestCreateRunnerScaleSet(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } + scaleSetCreationDateTime := time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC) runnerScaleSet := actions.RunnerScaleSet{Id: 1, Name: "ScaleSet", CreatedOn: scaleSetCreationDateTime, RunnerSetting: actions.RunnerSetting{}} t.Run("Create runner scale set", func(t *testing.T) { want := &runnerScaleSet rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + require.NoError(t, err) + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(rsl) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, err := actionsClient.CreateRunnerScaleSet(context.Background(), &runnerScaleSet) - if err != nil { - t.Fatalf("CreateRunnerScaleSet got exepected error, %v", err) - } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("CreateRunnerScaleSet(%d) mismatch (-want +got):\n%s", runnerScaleSet.Id, diff) - } - }, - ) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + got, err := client.CreateRunnerScaleSet(ctx, &runnerScaleSet) + require.NoError(t, err) + assert.Equal(t, want, got) + }) t.Run("CreateRunnerScaleSet calls correct url", func(t *testing.T) { rsl, err := json.Marshal(&runnerScaleSet) - if err != nil { - t.Fatalf("%v", err) - } + require.NoError(t, err) url := url.URL{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(rsl) url = *r.URL })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err = actionsClient.CreateRunnerScaleSet(context.Background(), &runnerScaleSet) - if err != nil { - t.Fatalf("CreateRunnerScaleSet got unexepected error, %v", err) - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.CreateRunnerScaleSet(ctx, &runnerScaleSet) + require.NoError(t, err) u := url.String() expectedUrl := "/_apis/runtime/runnerscalesets?api-version=6.0-preview" assert.Equal(t, expectedUrl, u) - - }, - ) + }) t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "text/plain") })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.CreateRunnerScaleSet(context.Background(), &runnerScaleSet) - if err == nil { - t.Fatalf("CreateRunnerScaleSet did not get exepected error, %v", &actions.ActionsError{}) - } + + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) + + _, err = client.CreateRunnerScaleSet(ctx, &runnerScaleSet) + require.NotNil(t, err) var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) + assert.True(t, errors.As(err, &expectedErr)) + }) t.Run("Default retries on server error", func(t *testing.T) { actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - retryClient := retryablehttp.NewClient() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - retryClient.RetryMax = retryMax - retryClient.RetryWaitMax = retryWaitMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, _ = actionsClient.CreateRunnerScaleSet(context.Background(), &runnerScaleSet) + retryMax := 1 + retryWaitMax := 1 * time.Microsecond + + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) + + _, err = client.CreateRunnerScaleSet(ctx, &runnerScaleSet) + require.NotNil(t, err) expectedRetry := retryMax + 1 assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _, _ = actionsClient.CreateRunnerScaleSet(context.Background(), &runnerScaleSet) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) -} - -func TestUpdateRunnerScaleSet(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" - scaleSetCreationDateTime := time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC) - runnerScaleSet := actions.RunnerScaleSet{Id: 1, Name: "ScaleSet", CreatedOn: scaleSetCreationDateTime, RunnerSetting: actions.RunnerSetting{}} - - t.Run("Update existing scale set", func(t *testing.T) { - want := &runnerScaleSet - rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Write(rsl) - })) - defer s.Close() - - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, err := actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, want) - if err != nil { - t.Fatalf("UpdateRunnerScaleSet got exepected error, %v", err) - } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("UpdateRunnerScaleSet(%d) mismatch (-want +got):\n%s", runnerScaleSet.Id, diff) - } - }, - ) - - t.Run("UpdateRunnerScaleSet calls correct url", func(t *testing.T) { - rsl, err := json.Marshal(&runnerScaleSet) - if err != nil { - t.Fatalf("%v", err) - } - url := url.URL{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write(rsl) - url = *r.URL - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err = actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - if err != nil { - t.Fatalf("UpdateRunnerScaleSet got unexepected error, %v", err) - } - - u := url.String() - expectedUrl := fmt.Sprintf("/_apis/runtime/runnerscalesets/%d?api-version=6.0-preview", runnerScaleSet.Id) - assert.Equal(t, expectedUrl, u) - - }, - ) - - t.Run("Status code not found", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNotFound) - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - if err == nil { - t.Fatalf("UpdateRunnerScaleSet did not get exepected error,") - } - var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) - - t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusBadRequest) - w.Header().Set("Content-Type", "text/plain") - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, err := actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - if err == nil { - t.Fatalf("UpdateRunnerScaleSet did not get exepected error") - } - var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) - - t.Run("Default retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryClient := retryablehttp.NewClient() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - retryClient.RetryWaitMax = retryWaitMax - retryClient.RetryMax = retryMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _, _ = actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _, _ = actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - - t.Run("No RunnerScaleSet found", func(t *testing.T) { - want := (*actions.RunnerScaleSet)(nil) - rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Write(rsl) - })) - defer s.Close() - - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - got, err := actionsClient.UpdateRunnerScaleSet(context.Background(), runnerScaleSet.Id, &runnerScaleSet) - if err != nil { - t.Fatalf("UpdateRunnerScaleSet got unexepected error, %v", err) - } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("UpdateRunnerScaleSet(%v) mismatch (-want +got):\n%s", runnerScaleSet.Id, diff) - } - - }, - ) -} - -func TestDeleteRunnerScaleSet(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" - scaleSetCreationDateTime := time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC) - runnerScaleSet := actions.RunnerScaleSet{Id: 1, Name: "ScaleSet", CreatedOn: scaleSetCreationDateTime, RunnerSetting: actions.RunnerSetting{}} - - t.Run("Delete existing scale set", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNoContent) - })) - defer s.Close() - - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - if err != nil { - t.Fatalf("DeleteRunnerScaleSet got unexepected error, %v", err) - } - }, - ) - - t.Run("DeleteRunnerScaleSet calls correct url", func(t *testing.T) { - url := url.URL{} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNoContent) - url = *r.URL - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - if err != nil { - t.Fatalf("DeleteRunnerScaleSet got unexepected error, %v", err) - } - - u := url.String() - expectedUrl := fmt.Sprintf("/_apis/runtime/runnerscalesets/%d?api-version=6.0-preview", runnerScaleSet.Id) - assert.Equal(t, expectedUrl, u) - - }, - ) - - t.Run("Status code not found", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNotFound) - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - if err == nil { - t.Fatalf("DeleteRunnerScaleSet did not get exepected error, ") - } - var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) - - t.Run("Error when Content-Type is text/plain", func(t *testing.T) { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusBadRequest) - w.Header().Set("Content-Type", "text/plain") - })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err := actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - if err == nil { - t.Fatalf("DeleteRunnerScaleSet did not get exepected error") - } - var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) - - t.Run("Default retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryClient := retryablehttp.NewClient() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - retryClient.RetryWaitMax = retryWaitMax - retryClient.RetryMax = retryMax - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - _ = actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - - t.Run("Custom retries on server error", func(t *testing.T) { - actualRetry := 0 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - actualRetry++ - })) - defer s.Close() - retryMax := 1 - retryWaitMax, err := time.ParseDuration("1µs") - if err != nil { - t.Fatalf("%v", err) - } - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - RetryMax: &retryMax, - RetryWaitMax: &retryWaitMax, - } - _ = actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - expectedRetry := retryMax + 1 - assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) - }, - ) - - t.Run("No RunnerScaleSet found", func(t *testing.T) { - want := (*actions.RunnerScaleSet)(nil) - rsl, err := json.Marshal(want) - if err != nil { - t.Fatalf("%v", err) - } - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Write(rsl) - })) - defer s.Close() - - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - err = actionsClient.DeleteRunnerScaleSet(context.Background(), runnerScaleSet.Id) - var expectedErr *actions.ActionsError - require.True(t, errors.As(err, &expectedErr)) - }, - ) + }) } diff --git a/github/actions/client_runner_test.go b/github/actions/client_runner_test.go index a8184b57..9406425a 100644 --- a/github/actions/client_runner_test.go +++ b/github/actions/client_runner_test.go @@ -3,23 +3,21 @@ package actions_test import ( "context" "net/http" - "net/http/httptest" "testing" "time" "github.com/actions/actions-runner-controller/github/actions" - "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -var tokenExpireAt = time.Now().Add(10 * time.Minute) - func TestGetRunner(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Get Runner", func(t *testing.T) { - name := "Get Runner" var runnerID int64 = 1 want := &actions.RunnerReference{ Id: int(runnerID), @@ -27,59 +25,45 @@ func TestGetRunner(t *testing.T) { } response := []byte(`{"id": 1, "name": "self-hosted-ubuntu"}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.GetRunner(context.Background(), runnerID) - if err != nil { - t.Fatalf("GetRunner got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunner(%v) mismatch (-want +got):\n%s", name, diff) - } + got, err := client.GetRunner(ctx, runnerID) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Default retries on server error", func(t *testing.T) { var runnerID int64 = 1 - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 + retryWaitMax := 1 * time.Millisecond + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _, _ = actionsClient.GetRunner(context.Background(), runnerID) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth, actions.WithRetryMax(retryMax), actions.WithRetryWaitMax(retryWaitMax)) + require.NoError(t, err) + _, err = client.GetRunner(ctx, runnerID) + require.Error(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } func TestGetRunnerByName(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Get Runner by Name", func(t *testing.T) { var runnerID int64 = 1 @@ -90,130 +74,102 @@ func TestGetRunnerByName(t *testing.T) { } response := []byte(`{"count": 1, "value": [{"id": 1, "name": "self-hosted-ubuntu"}]}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.GetRunnerByName(context.Background(), runnerName) - if err != nil { - t.Fatalf("GetRunnerByName got unexepected error, %v", err) - } - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("GetRunnerByName(%v) mismatch (-want +got):\n%s", runnerName, diff) - } + got, err := client.GetRunnerByName(ctx, runnerName) + require.NoError(t, err) + assert.Equal(t, want, got) }) t.Run("Get Runner by name with not exist runner", func(t *testing.T) { var runnerName string = "self-hosted-ubuntu" response := []byte(`{"count": 0, "value": []}`) - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(response) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - got, err := actionsClient.GetRunnerByName(context.Background(), runnerName) - if err != nil { - t.Fatalf("GetRunnerByName got unexepected error, %v", err) - } - - if diff := cmp.Diff((*actions.RunnerReference)(nil), got); diff != "" { - t.Errorf("GetRunnerByName(%v) mismatch (-want +got):\n%s", runnerName, diff) - } + got, err := client.GetRunnerByName(ctx, runnerName) + require.NoError(t, err) + assert.Nil(t, got) }) t.Run("Default retries on server error", func(t *testing.T) { var runnerName string = "self-hosted-ubuntu" - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 + + retryWaitMax := 1 * time.Millisecond + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _, _ = actionsClient.GetRunnerByName(context.Background(), runnerName) + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth, actions.WithRetryMax(retryMax), actions.WithRetryWaitMax(retryWaitMax)) + require.NoError(t, err) + _, err = client.GetRunnerByName(ctx, runnerName) + require.Error(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } func TestDeleteRunner(t *testing.T) { - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjI1MTYyMzkwMjJ9.tlrHslTmDkoqnc4Kk9ISoKoUNDfHo-kjlH-ByISBqzE" + ctx := context.Background() + auth := &actions.ActionsAuth{ + Token: "token", + } t.Run("Delete Runner", func(t *testing.T) { var runnerID int64 = 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) })) - defer s.Close() - actionsClient := actions.Client{ - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } + client, err := actions.NewClient(ctx, server.configURLForOrg("my-org"), auth) + require.NoError(t, err) - if err := actionsClient.RemoveRunner(context.Background(), runnerID); err != nil { - t.Fatalf("RemoveRunner got unexepected error, %v", err) - } + err = client.RemoveRunner(ctx, runnerID) + assert.NoError(t, err) }) t.Run("Default retries on server error", func(t *testing.T) { var runnerID int64 = 1 - retryClient := retryablehttp.NewClient() - retryClient.RetryWaitMax = 1 * time.Millisecond - retryClient.RetryMax = 1 + retryWaitMax := 1 * time.Millisecond + retryMax := 1 actualRetry := 0 - expectedRetry := retryClient.RetryMax + 1 + expectedRetry := retryMax + 1 - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := newActionsServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) actualRetry++ })) - defer s.Close() - httpClient := retryClient.StandardClient() - actionsClient := actions.Client{ - Client: httpClient, - ActionsServiceURL: &s.URL, - ActionsServiceAdminToken: &token, - ActionsServiceAdminTokenExpiresAt: &tokenExpireAt, - } - - _ = actionsClient.RemoveRunner(context.Background(), runnerID) + client, err := actions.NewClient( + ctx, + server.configURLForOrg("my-org"), + auth, + actions.WithRetryMax(retryMax), + actions.WithRetryWaitMax(retryWaitMax), + ) + require.NoError(t, err) + err = client.RemoveRunner(ctx, runnerID) + require.Error(t, err) assert.Equalf(t, actualRetry, expectedRetry, "A retry was expected after the first request but got: %v", actualRetry) }) } diff --git a/github/actions/multi_client.go b/github/actions/multi_client.go index 1d1aad1d..c7a53b74 100644 --- a/github/actions/multi_client.go +++ b/github/actions/multi_client.go @@ -104,7 +104,13 @@ func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, m.logger.Info("creating new client", "githubConfigURL", githubConfigURL, "namespace", namespace) - client, err := NewClient(ctx, githubConfigURL, &creds, m.userAgent, m.logger) + client, err := NewClient( + ctx, + githubConfigURL, + &creds, + WithUserAgent(m.userAgent), + WithLogger(m.logger), + ) if err != nil { return nil, err } diff --git a/github/actions/multi_client_test.go b/github/actions/multi_client_test.go index 11aeb7fd..fb4a64db 100644 --- a/github/actions/multi_client_test.go +++ b/github/actions/multi_client_test.go @@ -6,20 +6,15 @@ import ( "fmt" "net/http" "net/http/httptest" - "os" "strings" "testing" "time" - "github.com/actions/actions-runner-controller/logging" + "github.com/go-logr/logr" ) func TestAddClient(t *testing.T) { - logger, err := logging.NewLogger(logging.LogLevelDebug, logging.LogFormatText) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: creating logger: %v\n", err) - os.Exit(1) - } + logger := logr.Discard() multiClient := NewMultiClient("test-user-agent", logger).(*multiClient) ctx := context.Background()