Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions cloudapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package cloudapi

import (
"bytes"
"encoding/json"
"fmt"
"io"
"mime/multipart"
"net/http"
"strconv"
Expand Down Expand Up @@ -70,6 +72,12 @@ type ValidateTokenResponse struct {
Token string `json:"token-info"`
}

// AuthenticationResponse represents the response from the /cloud/v6/auth endpoint.
type AuthenticationResponse struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type AuthenticationResponse struct {
type authenticationResponse struct {

do we really need to export it?

StackID int64 `json:"stack_id"`
DefaultProjectID int64 `json:"default_project_id"`
}

func (c *Client) handleLogEntriesFromCloud(ctrr CreateTestRunResponse) {
logger := c.logger.WithField("source", "grafana-k6-cloud")
for _, logEntry := range ctrr.Logs {
Expand Down Expand Up @@ -336,3 +344,40 @@ func (c *Client) ValidateToken() (*ValidateTokenResponse, error) {

return &vtr, nil
}

// ValidateAuth validates a token and stack URL, returning the stack ID and default project ID.
// The stackURL must be a normalized full URL (e.g., https://my-team.grafana.net).
func (c *Client) ValidateAuth(stackURL string) (*AuthenticationResponse, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same question actually. And more than that, should it be the goal for all v6 endpoints to be called via that library?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be great but would probably need a larger refactor, not directly related to what this PR does. I'm planning to use this library when working on #5009 and I can update this function then.

req, err := http.NewRequest(http.MethodGet, "https://api.k6.io/cloud/v6/auth", nil) //nolint:noctx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
req, err := http.NewRequest(http.MethodGet, "https://api.k6.io/cloud/v6/auth", nil) //nolint:noctx
req, err := http.NewRequestWithContext(http.MethodGet, c.authBase, nil) //nolint:noctx

In addition, we should use the context, so we can cancel the requests when the user aborts the script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not blocking nor critical 👇🏻

I understand why this cannot use c.baseURL here, as the baseURL contains the v1 path. However, I would prefer for us not to hardcode the API URL here if possible.

I would suggest refactoring the client if possible to store the API version it talks to, and select the base api path to use according:

  • "/v1" if talking to the v1 API
  • "/cloud/v6" if talking to the v6 API

Specifically thinking of this code here:

baseURL: fmt.Sprintf("%s/v1", host),

In an ideal world, I think this should call something along the lines of c.BaseURL(cloudAPIVersion6) and get a fully qualified url as a result 🙇🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the changes! Let me know what you think 👀

if err != nil {
return nil, err
}

req.Header.Set("X-Stack-Url", stackURL)
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.token))
req.Header.Set("User-Agent", "k6cloud/"+c.version)

resp, err := c.client.Do(req)
if err != nil {
return nil, fmt.Errorf("request failed: %w", err)
}
defer func() {
if resp != nil {
_, _ = io.Copy(io.Discard, resp.Body)
if cerr := resp.Body.Close(); cerr != nil && err == nil {
err = cerr
}
}
}()

if err := CheckResponse(resp); err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error schema in v6 AP is different from the old one. Should the error handling code be updated too?

return nil, err
}

authResp := AuthenticationResponse{}
if err := json.NewDecoder(resp.Body).Decode(&authResp); err != nil {
return nil, fmt.Errorf("failed to decode response: %w", err)
}

return &authResp, nil
}
25 changes: 21 additions & 4 deletions cloudapi/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ const LegacyCloudConfigKey = "loadimpact"
//nolint:lll
type Config struct {
// TODO: refactor common stuff between cloud execution and output
Token null.String `json:"token" envconfig:"K6_CLOUD_TOKEN"`
ProjectID null.Int `json:"projectID" envconfig:"K6_CLOUD_PROJECT_ID"`
Name null.String `json:"name" envconfig:"K6_CLOUD_NAME"`
StackID null.Int `json:"stackID,omitempty" envconfig:"K6_CLOUD_STACK_ID"`
StackURL null.String `json:"stackURL,omitempty" envconfig:"K6_CLOUD_STACK"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
StackURL null.String `json:"stackURL,omitempty" envconfig:"K6_CLOUD_STACK"`
StackURL null.String `json:"stackURL,omitempty" envconfig:"K6_CLOUD_STACK_URL"`

I added URL because 2 out of 3 have it, but if the intention is to not have it then remove it from the other two.

DefaultProjectID null.Int `json:"defaultProjectID,omitempty"`
Token null.String `json:"token" envconfig:"K6_CLOUD_TOKEN"`
ProjectID null.Int `json:"projectID" envconfig:"K6_CLOUD_PROJECT_ID"`
Name null.String `json:"name" envconfig:"K6_CLOUD_NAME"`

Host null.String `json:"host" envconfig:"K6_CLOUD_HOST"`
Timeout types.NullDuration `json:"timeout" envconfig:"K6_CLOUD_TIMEOUT"`
Expand Down Expand Up @@ -103,6 +106,15 @@ func NewConfig() Config {
//
//nolint:cyclop
func (c Config) Apply(cfg Config) Config {
if cfg.StackID.Valid {
c.StackID = cfg.StackID
}
if cfg.StackURL.Valid && !c.StackURL.Valid {
c.StackURL = cfg.StackURL
}
if cfg.DefaultProjectID.Valid {
c.DefaultProjectID = cfg.DefaultProjectID
}
if cfg.Token.Valid {
c.Token = cfg.Token
}
Expand Down Expand Up @@ -240,7 +252,9 @@ func mergeFromCloudOptionAndExternal(
if err := json.Unmarshal(source, &tmpConfig); err != nil {
return err
}
// Only take out the ProjectID, Name and Token from the options.cloud (or legacy loadimpact struct) map:

// Only merge ProjectID, Name, Token, and StackID from options.
// StackURL and DefaultProjectID can only be set via login.
Comment on lines +255 to +257
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get this check in a position near to the others, please?

if tmpConfig.ProjectID.Valid {
conf.ProjectID = tmpConfig.ProjectID
}
Expand All @@ -250,6 +264,9 @@ func mergeFromCloudOptionAndExternal(
if tmpConfig.Token.Valid {
conf.Token = tmpConfig.Token
}
if tmpConfig.StackID.Valid {
conf.StackID = tmpConfig.StackID
}

return nil
}
Expand Down
1 change: 1 addition & 0 deletions cloudapi/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func TestConfigApply(t *testing.T) {

full := Config{
Token: null.NewString("Token", true),
StackID: null.NewInt(1, true),
ProjectID: null.NewInt(1, true),
Name: null.NewString("Name", true),
Host: null.NewString("Host", true),
Expand Down
37 changes: 37 additions & 0 deletions internal/cmd/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"go.k6.io/k6/internal/build"
"go.k6.io/k6/internal/ui/pb"
"go.k6.io/k6/lib"
"gopkg.in/guregu/null.v3"

"github.com/fatih/color"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -174,6 +175,12 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error {
return err
}

if cloudConfig.ProjectID.Int64 == 0 {
if err := resolveAndSetProjectID(c.gs, &cloudConfig, tmpCloudConfig, arc); err != nil {
return err
}
}

modifyAndPrintBar(c.gs, progressBar, pb.WithConstProgress(0, "Uploading archive"))

var cloudTestRun *cloudapi.CreateTestRunResponse
Expand Down Expand Up @@ -407,6 +414,36 @@ service. Be sure to run the "k6 cloud login" command prior to authenticate with
return cloudCmd
}

func resolveAndSetProjectID(
gs *state.GlobalState,
cloudConfig *cloudapi.Config,
tmpCloudConfig map[string]interface{},
arc *lib.Archive,
) error {
projectID, err := resolveDefaultProjectID(gs, cloudConfig)
if err != nil {
return err
}
if projectID > 0 {
tmpCloudConfig["projectID"] = projectID

b, err := json.Marshal(tmpCloudConfig)
if err != nil {
return err
}

arc.Options.Cloud = b
arc.Options.External[cloudapi.LegacyCloudConfigKey] = b

cloudConfig.ProjectID = null.IntFrom(projectID)
}
if projectID == 0 && (!cloudConfig.StackID.Valid || cloudConfig.StackID.Int64 == 0) {
gs.Logger.Warn("Warning: no projectID or default stack specified. Falling back to the first available stack.")
gs.Logger.Warn("Consider setting a default stack via the `k6 cloud login` command.")
}
return nil
}

func exactCloudArgs() cobra.PositionalArgs {
return func(_ *cobra.Command, args []string) error {
const baseErrMsg = `the "k6 cloud" command expects either a subcommand such as "run" or "login", or ` +
Expand Down
Loading
Loading