Skip to content

Add support to set the GC Stack the CLI should use by default #4940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
94 changes: 94 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 @@ -336,3 +338,95 @@ func (c *Client) ValidateToken() (*ValidateTokenResponse, error) {

return &vtr, nil
}

// Organization represents a Grafana Cloud k6 organization
type Organization struct {
ID int `json:"id"`
GrafanaStackName string `json:"grafana_stack_name"`
GrafanaStackID int `json:"grafana_stack_id"`
Comment on lines +345 to +346
Copy link
Contributor

@joanlopez joanlopez Jul 22, 2025

Choose a reason for hiding this comment

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

I have tested the endpoint, and looks like it could return a null grafana_stack_name and/or grafana_stack_id.

Is that only a particular case for my account? Or an edge case that we should handle?
Because, in such case, I think we should make this nil-able, and/or somehow explicitly document that it can be the case.

I'm wondering the wide variety of corner cases we can find once we release this to users 🤔

}

// AccountMeResponse represents the response of the /account/me endpoint.
type AccountMeResponse struct {
Organizations []Organization `json:"organizations"`
}

// AccountMe retrieves the current user's account information.
func (c *Client) AccountMe() (*AccountMeResponse, error) {
// TODO: remove this hardcoded URL
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the problem with this? Can we create a dedicated client?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree that as we usually rely on Client.baseURL, and in this case it seems to be a different host, would likely be better to split into a dedicated client, as otherwise we could easily mess things up in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the problem with this?

I guess that, at the very least, we should parameterize the baseURL (ideally into a different client, as mentioned above), and only keep the specific path/endpoint hard-coded.

req, err := c.NewRequest("GET", "https://api.k6.io/v3/account/me", nil)
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 := c.NewRequest("GET", "https://api.k6.io/v3/account/me", nil)
req, err := c.NewRequest(http.GetMethod, "https://api.k6.io/v3/account/me", nil)

Same for the others

if err != nil {
return nil, err
}

amr := AccountMeResponse{}
err = c.Do(req, &amr)
if err != nil {
return nil, err
}

return &amr, nil
}

// GetDefaultProject retrieves the default project for the given stack ID.
func (c *Client) GetDefaultProject(stackID int64) (int64, string, error) {
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 don't know what the long-term plans are here.

The new REST API does a few things differently:

  • StackID is mandatory
  • Auth header is different (using Bearer instead of Token)

Should we change the existing Client, or add a new one?

What do you think about using the auto-generated Go SDK?

Moving to the new API completely is impossible until we enforce StackID (maybe in v2?). So yeah, we will be living in this mixed APIs state for a while.

Copy link
Contributor

Choose a reason for hiding this comment

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

until we enforce StackID (maybe in v2?).

Is it really a strict requirement? It seems we can get the StackID from the token just by calling /account/me, or is only available in /v3?

What do you think about using the auto-generated Go SDK?

It sounds to me a great idea to explore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the existing Client, or add a new one?

As discussed above, I'd suggest adding a new client, so we keep things isolated, especially considering we're speaking about different hosts or different versions of the API.

What do you think about using the auto-generated Go SDK?

I think we should use it when possible, but I suspect most of the operations that are supported now (by k6) aren't supported by the latest version of the Public API. But yeah, if we start adding support into k6 for the operations available through the auto-generated client, then I'd suggest using it for those, for sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it really a strict requirement? It seems we can get the StackID from the token just by calling /account/me, or is only available in /v3?

A user can be in many Stacks! Which one should we pick? 👀

That's the current problem, IMO.

I'd suggest adding a new client

I'll try to do this. I'll also ask the backend about the /me endpoint and if they plan to include it in the new API.

That said, I'll do the smallest thing we can in this area to move this PR forward. We can keep adding things to that new client in future PRs (e.g., when I add k6 plan I'll need a few more methods)

// TODO: remove this hardcoded URL
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before

req, err := c.NewRequest("GET", "https://api.k6.io/cloud/v6/projects", nil)
if err != nil {
return 0, "", err
}

q := req.URL.Query()
q.Add("$orderby", "created")
req.URL.RawQuery = q.Encode()

req.Header.Set("X-Stack-Id", fmt.Sprintf("%d", stackID))
// TODO: by default the client uses Token instead of Bearer
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.token))
req.Header.Set("User-Agent", "Go-http-client")
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.Header.Set("User-Agent", "Go-http-client")

Is it required?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess Backend folks may want to know where the API is being consumed from, right @dgzlopes? 🤔

If we ever replace this custom implementation in favor of the auto-generated Go SDK, the User-Agent is yet another configuration attribute that you can customize.

Wrt the value, I think we might want to use something that explicitly let us know this request is coming from the k6 binary.


// TODO: Can't use c.Do bc it messes up the headers
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 this is yet another indicator that we should likely have a different client implementation for this.

resp, err := http.DefaultClient.Do(req)
if err != nil {
Comment on lines +388 to +390
Copy link
Contributor

Choose a reason for hiding this comment

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

Then we need a dedicated client overwriting this different logic, otherwise testing will be quite difficult

return 0, "", 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 resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return 0, "", fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body))
}

var parsed struct {
Count int64 `json:"@count"`
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
Count int64 `json:"@count"`

Do we need to parse it?

Value []struct {
ID int64 `json:"id"`
Name string `json:"name"`
IsDefault bool `json:"is_default"`
FolderUID string `json:"grafana_folder_uid"`
} `json:"value"`
}

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

if len(parsed.Value) == 0 {
return 0, "", fmt.Errorf("no projects found for stack ID %d", stackID)
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
return 0, "", fmt.Errorf("no projects found for stack ID %d", stackID)
return 0, "", fmt.Errorf("no projects found for stack ID %d", stackSlug)

We should be consistent, if we prefer the Slug on input then we should return a slug on errors. Otherwise, they will experience the same difficulties we're carefully avoiding on inputs.

}

for _, proj := range parsed.Value {
if proj.IsDefault {
return proj.ID, proj.Name, nil
}
}

return 0, "", fmt.Errorf("no default project found for stack ID %d", stackID)
}
17 changes: 16 additions & 1 deletion cloudapi/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const LegacyCloudConfigKey = "loadimpact"
//nolint:lll
type Config struct {
// TODO: refactor common stuff between cloud execution and output
StackID null.Int `json:"stackID,omitempty" envconfig:"K6_CLOUD_STACK_ID"`
StackSlug null.String `json:"stackSlug,omitempty" envconfig:"K6_CLOUD_STACK_SLUG"`
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"`
Expand Down Expand Up @@ -103,6 +105,12 @@ func NewConfig() Config {
//
//nolint:cyclop
func (c Config) Apply(cfg Config) Config {
if cfg.StackID.Valid {
c.StackID = cfg.StackID
}
if cfg.StackSlug.Valid {
c.StackSlug = cfg.StackSlug
}
if cfg.Token.Valid {
c.Token = cfg.Token
}
Expand Down Expand Up @@ -240,7 +248,8 @@ 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 take out the ProjectID, Name, Token and StackID from the options.cloud (or legacy loadimpact struct) map:
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
// Only take out the ProjectID, Name, Token and StackID from the options.cloud (or legacy loadimpact struct) map:
// Only take out the ProjectID, Name, Token, StackID, StackSlug from the options.cloud (or legacy loadimpact struct) map:

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

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

full := Config{
Token: null.NewString("Token", true),
StackID: null.NewInt(1, true),
StackSlug: null.NewString("StackSlug", true),
ProjectID: null.NewInt(1, true),
Name: null.NewString("Name", true),
Host: null.NewString("Host", true),
Expand Down
29 changes: 29 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 @@ -165,6 +166,34 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error {
modifyAndPrintBar(c.gs, progressBar, pb.WithConstProgress(0, "Validating script options"))
client := cloudapi.NewClient(
logger, cloudConfig.Token.String, cloudConfig.Host.String, build.Version, cloudConfig.Timeout.TimeDuration())

if cloudConfig.ProjectID.Int64 == 0 {
projectID, err := resolveDefaultProjectID(
c.gs,
client,
test.derivedConfig.Collectors["cloud"],
cloudConfig.Token.String,
cloudConfig.StackSlug,
&cloudConfig.StackID,
&cloudConfig.ProjectID.Int64,
)
if err != nil {
return err
}

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 err = client.ValidateOptions(arc.Options); err != nil {
return err
}
Expand Down
Loading
Loading