From fde1bce50a3e19e9049a05de45f52a48d27c42e2 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Wed, 19 Jul 2023 11:23:27 +0200 Subject: [PATCH 01/11] refactor: Abstracted the different config validation routes --- api/rest/client.go | 9 ++- cmd/config.go | 12 ++- config/api_client.go | 71 ++++++++++++++++++ config/api_client_test.go | 61 +++++++++++++++ config/commands_test.go | 40 +++++++--- config/config.go | 74 ++++--------------- config/config_test.go | 56 +++++++------- config/{legacy.go => v1_api_client.go} | 40 +++++----- .../{legacy_test.go => v1_api_client_test.go} | 43 ++++++++--- config/v2_api_client.go | 62 ++++++++++++++++ config/v2_api_client_test.go | 17 +++++ local/local.go | 5 +- 12 files changed, 359 insertions(+), 131 deletions(-) create mode 100644 config/api_client.go create mode 100644 config/api_client_test.go rename config/{legacy.go => v1_api_client.go} (88%) rename config/{legacy_test.go => v1_api_client_test.go} (74%) create mode 100644 config/v2_api_client.go create mode 100644 config/v2_api_client_test.go diff --git a/api/rest/client.go b/api/rest/client.go index 2d1628f41..8204237fe 100644 --- a/api/rest/client.go +++ b/api/rest/client.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "net/http" "net/url" "strings" @@ -84,7 +85,7 @@ func (c *Client) enrichRequestHeaders(req *http.Request, payload interface{}) { } } -func (c *Client) DoRequest(req *http.Request, resp interface{}) (statusCode int, err error) { +func (c *Client) DoRequest(req *http.Request, resp interface{}) (int, error) { httpResp, err := c.client.Do(req) if err != nil { fmt.Printf("failed to make http request: %s\n", err.Error()) @@ -96,10 +97,14 @@ func (c *Client) DoRequest(req *http.Request, resp interface{}) (statusCode int, httpError := struct { Message string `json:"message"` }{} - err = json.NewDecoder(httpResp.Body).Decode(&httpError) + body, err := ioutil.ReadAll(httpResp.Body) if err != nil { return httpResp.StatusCode, err } + err = json.Unmarshal(body, &httpError) + if err != nil { + return httpResp.StatusCode, &HTTPError{Code: httpResp.StatusCode, Message: string(body)} + } return httpResp.StatusCode, &HTTPError{Code: httpResp.StatusCode, Message: httpError.Message} } diff --git a/cmd/config.go b/cmd/config.go index 6c85de763..690fc78d1 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -50,7 +50,11 @@ func newConfigCommand(globalConfig *settings.Config) *cobra.Command { Aliases: []string{"check"}, Short: "Check that the config file is well formed.", RunE: func(cmd *cobra.Command, args []string) error { - compiler := config.New(globalConfig) + compiler, err := config.NewWithConfig(globalConfig) + if err != nil { + return err + } + orgID, _ := cmd.Flags().GetString("org-id") orgSlug, _ := cmd.Flags().GetString("org-slug") path := config.DefaultConfigPath @@ -86,7 +90,11 @@ func newConfigCommand(globalConfig *settings.Config) *cobra.Command { Use: "process ", Short: "Validate config and display expanded configuration.", RunE: func(cmd *cobra.Command, args []string) error { - compiler := config.New(globalConfig) + compiler, err := config.NewWithConfig(globalConfig) + if err != nil { + return err + } + pipelineParamsFilePath, _ := cmd.Flags().GetString("pipeline-parameters") orgID, _ := cmd.Flags().GetString("org-id") orgSlug, _ := cmd.Flags().GetString("org-slug") diff --git a/config/api_client.go b/config/api_client.go new file mode 100644 index 000000000..1d3493d05 --- /dev/null +++ b/config/api_client.go @@ -0,0 +1,71 @@ +package config + +import ( + "fmt" + "net/url" + "sync" + + "github.com/CircleCI-Public/circleci-cli/api/graphql" + "github.com/CircleCI-Public/circleci-cli/api/rest" + "github.com/CircleCI-Public/circleci-cli/settings" +) + +var ( + compiler APIClient + compilerError error + once sync.Once + + compilePath = "compile-config-with-defaults" +) + +type apiClientVersion string + +type APIClient interface { + CompileConfig(configContent string, orgID string, params Parameters, values Values) (*ConfigResponse, error) +} + +func GetAPIClient(config *settings.Config) (APIClient, error) { + if compiler == nil { + once.Do(func() { + compiler, compilerError = newAPIClient(config) + }) + } + return compiler, compilerError +} + +func newAPIClient(config *settings.Config) (APIClient, error) { + hostValue := getCompileHost(config.Host) + restClient := rest.NewFromConfig(hostValue, config) + + version, err := detectAPIClientVersion(restClient) + if err != nil { + return nil, err + } + + switch version { + case v1_string: + return &v1APIClient{graphql.NewClient(config.HTTPClient, config.Host, config.Endpoint, config.Token, config.Debug)}, nil + case v2_string: + return &v2APIClient{restClient}, nil + default: + return nil, fmt.Errorf("Unable to recognise your Server's config file API") + } +} + +func detectAPIClientVersion(restClient *rest.Client) (apiClientVersion, error) { + req, err := restClient.NewRequest("POST", &url.URL{Path: compilePath}, nil) + if err != nil { + return "", err + } + + statusCode, err := restClient.DoRequest(req, nil) + if err != nil { + if _, ok := err.(*rest.HTTPError); ok { + return "", err + } + } + if statusCode == 404 { + return v1_string, nil + } + return v2_string, nil +} diff --git a/config/api_client_test.go b/config/api_client_test.go new file mode 100644 index 000000000..7011afafc --- /dev/null +++ b/config/api_client_test.go @@ -0,0 +1,61 @@ +package config + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/CircleCI-Public/circleci-cli/api/rest" + "gotest.tools/v3/assert" +) + +func TestAPIClient(t *testing.T) { + t.Run("detectCompilerVersion", func(t *testing.T) { + t.Run("when the route returns a 400 tells that the version is v2", func(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(400) + w.Header().Set("Content-Type", "application/octet-stream") + fmt.Fprintf(w, "Invalid input") + })) + url, err := url.Parse(svr.URL) + assert.NilError(t, err) + + restClient := rest.New(url, "token", http.DefaultClient) + version, err := detectAPIClientVersion(restClient) + assert.NilError(t, err) + assert.Equal(t, version, v2_string) + }) + + t.Run("when the route returns a 404 tells that the version is v1", func(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + w.Header().Set("Content-Type", "application/octet-stream") + fmt.Fprintf(w, "Invalid input") + })) + url, err := url.Parse(svr.URL) + assert.NilError(t, err) + + restClient := rest.New(url, "token", http.DefaultClient) + version, err := detectAPIClientVersion(restClient) + assert.NilError(t, err) + assert.Equal(t, version, v1_string) + }) + + t.Run("in any other case, return an error", func(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + w.Header().Set("Content-Type", "application/octet-stream") + fmt.Fprintf(w, "Invalid input") + })) + url, err := url.Parse(svr.URL) + assert.NilError(t, err) + + restClient := rest.New(url, "token", http.DefaultClient) + version, err := detectAPIClientVersion(restClient) + assert.Equal(t, version, apiClientVersion("")) + assert.Error(t, err, "Invalid input") + }) + }) +} diff --git a/config/commands_test.go b/config/commands_test.go index dc6e7a16a..786eafa18 100644 --- a/config/commands_test.go +++ b/config/commands_test.go @@ -8,6 +8,8 @@ import ( "net/http/httptest" "testing" + "github.com/CircleCI-Public/circleci-cli/api/collaborators" + "github.com/CircleCI-Public/circleci-cli/api/rest" "github.com/CircleCI-Public/circleci-cli/settings" "github.com/stretchr/testify/assert" ) @@ -18,7 +20,11 @@ func TestGetOrgID(t *testing.T) { fmt.Fprintf(w, `[{"vcs_type":"circleci","slug":"gh/test","id":"2345"}]`) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) t.Run("returns the original org-id passed if it is set", func(t *testing.T) { expected := "1234" @@ -66,9 +72,13 @@ func TestValidateConfig(t *testing.T) { fmt.Fprintf(w, `{"valid":true,"source-yaml":"%s","output-yaml":"%s","errors":[]}`, testYaml, testYaml) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - err := compiler.ValidateConfig(ValidateConfigOpts{ + err = compiler.ValidateConfig(ValidateConfigOpts{ ConfigPath: "testdata/config.yml", }) assert.NoError(t, err) @@ -87,9 +97,13 @@ func TestValidateConfig(t *testing.T) { fmt.Fprintf(w, `{"valid":true,"source-yaml":"%s","output-yaml":"%s","errors":[]}`, testYaml, testYaml) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - err := compiler.ValidateConfig(ValidateConfigOpts{ + err = compiler.ValidateConfig(ValidateConfigOpts{ ConfigPath: "testdata/config.yml", OrgID: "1234", }) @@ -119,9 +133,13 @@ func TestValidateConfig(t *testing.T) { svr := httptest.NewServer(mux) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - err := compiler.ValidateConfig(ValidateConfigOpts{ + err = compiler.ValidateConfig(ValidateConfigOpts{ ConfigPath: "testdata/config.yml", OrgSlug: "gh/test", }) @@ -151,9 +169,13 @@ func TestValidateConfig(t *testing.T) { svr := httptest.NewServer(mux) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - err := compiler.ValidateConfig(ValidateConfigOpts{ + err = compiler.ValidateConfig(ValidateConfigOpts{ ConfigPath: "testdata/config.yml", OrgSlug: "gh/nonexistent", }) diff --git a/config/config.go b/config/config.go index ba55404de..741384220 100644 --- a/config/config.go +++ b/config/config.go @@ -3,12 +3,9 @@ package config import ( "fmt" "io" - "net/url" "os" "github.com/CircleCI-Public/circleci-cli/api/collaborators" - "github.com/CircleCI-Public/circleci-cli/api/graphql" - "github.com/CircleCI-Public/circleci-cli/api/rest" "github.com/CircleCI-Public/circleci-cli/settings" "github.com/pkg/errors" ) @@ -22,29 +19,28 @@ var ( ) type ConfigCompiler struct { - host string - compileRestClient *rest.Client - collaborators collaborators.CollaboratorsClient - - cfg *settings.Config - legacyGraphQLClient *graphql.Client + apiClient APIClient + collaborators collaborators.CollaboratorsClient } -func New(cfg *settings.Config) *ConfigCompiler { - hostValue := GetCompileHost(cfg.Host) +func NewWithConfig(cfg *settings.Config) (*ConfigCompiler, error) { + apiClient, err := GetAPIClient(cfg) + if err != nil { + return nil, err + } collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) if err != nil { - panic(err) + return nil, err } + return New(apiClient, collaboratorsClient), nil +} +func New(apiClient APIClient, collaboratorsClient collaborators.CollaboratorsClient) *ConfigCompiler { configCompiler := &ConfigCompiler{ - host: hostValue, - compileRestClient: rest.NewFromConfig(hostValue, cfg), - collaborators: collaboratorsClient, - cfg: cfg, + apiClient: apiClient, + collaborators: collaboratorsClient, } - configCompiler.legacyGraphQLClient = graphql.NewClient(cfg.HTTPClient, cfg.Host, cfg.Endpoint, cfg.Token, cfg.Debug) return configCompiler } @@ -94,49 +90,7 @@ func (c *ConfigCompiler) ConfigQuery( return nil, fmt.Errorf("failed to load yaml config from config path provider: %w", err) } - compileRequest := CompileConfigRequest{ - ConfigYaml: configString, - Options: Options{ - OwnerID: orgID, - PipelineValues: values, - PipelineParameters: params, - }, - } - - req, err := c.compileRestClient.NewRequest( - "POST", - &url.URL{ - Path: "compile-config-with-defaults", - }, - compileRequest, - ) - if err != nil { - return nil, fmt.Errorf("an error occurred creating the request: %w", err) - } - - configCompilationResp := &ConfigResponse{} - statusCode, originalErr := c.compileRestClient.DoRequest(req, configCompilationResp) - if statusCode == 404 { - fmt.Fprintf(os.Stderr, "You are using a old version of CircleCI Server, please consider updating\n") - legacyResponse, err := c.legacyConfigQueryByOrgID(configString, orgID, params, values, c.cfg) - if err != nil { - return nil, err - } - return legacyResponse, nil - } - if originalErr != nil { - return nil, fmt.Errorf("config compilation request returned an error: %w", originalErr) - } - - if statusCode != 200 { - return nil, errors.New("unable to validate or compile config") - } - - if len(configCompilationResp.Errors) > 0 { - return nil, fmt.Errorf("config compilation contains errors: %s", configCompilationResp.Errors) - } - - return configCompilationResp, nil + return c.apiClient.CompileConfig(configString, orgID, params, values) } func loadYaml(path string) (string, error) { diff --git a/config/config_test.go b/config/config_test.go index 84af6c2cc..21a2b30f9 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -8,37 +8,25 @@ import ( "net/http/httptest" "testing" + "github.com/CircleCI-Public/circleci-cli/api/collaborators" + "github.com/CircleCI-Public/circleci-cli/api/rest" "github.com/CircleCI-Public/circleci-cli/settings" "github.com/stretchr/testify/assert" ) -func TestCompiler(t *testing.T) { - t.Run("test compiler setup", func(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprintf(w, `[{"vcs_type":"circleci","slug":"gh/test","id":"2345"}]`) - })) - defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) - - t.Run("assert compiler has correct host", func(t *testing.T) { - assert.Equal(t, "http://"+compiler.compileRestClient.BaseURL.Host, svr.URL) - }) - - t.Run("assert compiler has default api host", func(t *testing.T) { - newCompiler := New(&settings.Config{Host: defaultHost, HTTPClient: http.DefaultClient}) - assert.Equal(t, "https://"+newCompiler.compileRestClient.BaseURL.Host, defaultAPIHost) - }) - - t.Run("tests that we correctly get the config api host when the host is not the default one", func(t *testing.T) { +func TestConfig(t *testing.T) { + t.Run("test getCompileHost", func(t *testing.T) { + t.Run("for Server instances", func(t *testing.T) { // if the host isn't equal to `https://circleci.com` then this is likely a server instance and // wont have the api.X.com subdomain so we should instead just respect the host for config commands host := GetCompileHost("test") assert.Equal(t, host, "test") + }) + t.Run("for CircleCI servers", func(t *testing.T) { // If the host passed in is the same as the defaultHost 'https://circleci.com' - then we know this is cloud // and as such should use the `api.circleci.com` subdomain - host = GetCompileHost("https://circleci.com") + host := GetCompileHost("https://circleci.com") assert.Equal(t, host, "https://api.circleci.com") }) }) @@ -50,7 +38,11 @@ func TestCompiler(t *testing.T) { fmt.Fprintf(w, `{"valid":true,"source-yaml":"source","output-yaml":"output","errors":[]}`) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) result, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.NoError(t, err) @@ -65,9 +57,13 @@ func TestCompiler(t *testing.T) { fmt.Fprintf(w, `{"valid":true,"source-yaml":"source","output-yaml":"output","errors":[]}`) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - _, err := compiler.ConfigQuery("testdata/nonexistent.yml", "1234", Parameters{}, Values{}) + _, err = compiler.ConfigQuery("testdata/nonexistent.yml", "1234", Parameters{}, Values{}) assert.Error(t, err) assert.Contains(t, err.Error(), "Could not load config file at testdata/nonexistent.yml") }) @@ -90,9 +86,13 @@ func TestCompiler(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) - _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + _, err = compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.Error(t, err) assert.Contains(t, err.Error(), "config compilation request returned an error") }) @@ -111,7 +111,11 @@ func TestCompiler(t *testing.T) { fmt.Fprintf(w, `{"valid":true,"source-yaml":"source","output-yaml":"output","errors":[]}`) })) defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + cfg := &settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient} + apiClient := &v2APIClient{rest.NewFromConfig(cfg.Host, cfg)} + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) resp, err := compiler.ConfigQuery("testdata/test.yml", "1234", Parameters{}, Values{}) assert.NoError(t, err) diff --git a/config/legacy.go b/config/v1_api_client.go similarity index 88% rename from config/legacy.go rename to config/v1_api_client.go index 02581bb74..a65e9a134 100644 --- a/config/legacy.go +++ b/config/v1_api_client.go @@ -4,13 +4,17 @@ import ( "encoding/json" "fmt" "sort" - "strings" "github.com/CircleCI-Public/circleci-cli/api/graphql" - "github.com/CircleCI-Public/circleci-cli/settings" "github.com/pkg/errors" ) +const v1_string apiClientVersion = "v1" + +type v1APIClient struct { + gql *graphql.Client +} + // GQLErrorsCollection is a slice of errors returned by the GraphQL server. // Each error is made up of a GQLResponseError type. type GQLErrorsCollection []GQLResponseError @@ -24,13 +28,13 @@ type BuildConfigResponse struct { // Error turns a GQLErrorsCollection into an acceptable error string that can be printed to the user. func (errs GQLErrorsCollection) Error() string { - messages := []string{} + message := "config compilation contains errors:" for i := range errs { - messages = append(messages, errs[i].Message) + message += fmt.Sprintf("\n\t- %s", errs[i].Message) } - return strings.Join(messages, "\n") + return message } // LegacyConfigResponse is a structure that matches the result of the GQL @@ -54,6 +58,12 @@ type GQLResponseError struct { Type string } +// KeyVal is a data structure specifically for passing pipeline data to GraphQL which doesn't support free-form maps. +type KeyVal struct { + Key string `json:"key"` + Val interface{} `json:"val"` +} + // PrepareForGraphQL takes a golang homogenous map, and transforms it into a list of keyval pairs, since GraphQL does not support homogenous maps. func PrepareForGraphQL(kvMap Values) []KeyVal { // we need to create the slice of KeyVals in a deterministic order for testing purposes @@ -70,13 +80,7 @@ func PrepareForGraphQL(kvMap Values) []KeyVal { return kvs } -func (c *ConfigCompiler) legacyConfigQueryByOrgID( - configString string, - orgID string, - params Parameters, - values Values, - cfg *settings.Config, -) (*ConfigResponse, error) { +func (client *v1APIClient) CompileConfig(configContent string, orgID string, params Parameters, values Values) (*ConfigResponse, error) { var response BuildConfigResponse // GraphQL isn't forwards-compatible, so we are unusually selective here about // passing only non-empty fields on to the API, to minimize user impact if the @@ -101,8 +105,8 @@ func (c *ConfigCompiler) legacyConfigQueryByOrgID( ) request := graphql.NewRequest(query) - request.SetToken(cfg.Token) - request.Var("config", configString) + request.SetToken(client.gql.Token) + request.Var("config", configContent) if values != nil { request.Var("pipelineValues", PrepareForGraphQL(values)) @@ -119,7 +123,7 @@ func (c *ConfigCompiler) legacyConfigQueryByOrgID( request.Var("orgId", orgID) } - err := c.legacyGraphQLClient.Run(request, &response) + err := client.gql.Run(request, &response) if err != nil { return nil, errors.Wrap(err, "Unable to validate config") } @@ -133,9 +137,3 @@ func (c *ConfigCompiler) legacyConfigQueryByOrgID( OutputYaml: response.BuildConfig.LegacyConfigResponse.OutputYaml, }, nil } - -// KeyVal is a data structure specifically for passing pipeline data to GraphQL which doesn't support free-form maps. -type KeyVal struct { - Key string `json:"key"` - Val interface{} `json:"val"` -} diff --git a/config/legacy_test.go b/config/v1_api_client_test.go similarity index 74% rename from config/legacy_test.go rename to config/v1_api_client_test.go index 5175a136f..2eb0ce1da 100644 --- a/config/legacy_test.go +++ b/config/v1_api_client_test.go @@ -2,15 +2,17 @@ package config import ( "fmt" + "io/ioutil" "net/http" "net/http/httptest" "testing" + "github.com/CircleCI-Public/circleci-cli/api/collaborators" "github.com/CircleCI-Public/circleci-cli/settings" "github.com/stretchr/testify/assert" ) -func TestLegacyFlow(t *testing.T) { +func TestAPIV1Flow(t *testing.T) { t.Run("tests that the compiler defaults to the graphQL resolver should the original API request fail with 404", func(t *testing.T) { mux := http.NewServeMux() @@ -31,12 +33,17 @@ func TestLegacyFlow(t *testing.T) { svr := httptest.NewServer(mux) defer svr.Close() - compiler := New(&settings.Config{ + cfg := &settings.Config{ Host: svr.URL, Endpoint: "/graphql-unstable", HTTPClient: http.DefaultClient, Token: "", - }) + } + apiClient, err := newAPIClient(cfg) + assert.NoError(t, err) + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) resp, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.Equal(t, true, resp.Valid) @@ -63,13 +70,18 @@ func TestLegacyFlow(t *testing.T) { svr := httptest.NewServer(mux) defer svr.Close() - compiler := New(&settings.Config{ + cfg := &settings.Config{ Host: svr.URL, Endpoint: "/graphql-unstable", HTTPClient: http.DefaultClient, Token: "", - }) - _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + } + apiClient, err := newAPIClient(cfg) + assert.NoError(t, err) + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) + _, err = compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.Error(t, err) assert.Contains(t, err.Error(), "failed to validate") }) @@ -79,8 +91,14 @@ func TestLegacyFlow(t *testing.T) { gqlHitCounter := 0 mux.HandleFunc("/compile-config-with-defaults", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusInternalServerError) + body, err := ioutil.ReadAll(r.Body) + assert.NoError(t, err) + if len(body) == 0 { + w.WriteHeader(http.StatusBadRequest) + return + } + w.WriteHeader(http.StatusInternalServerError) }) mux.HandleFunc("/me/collaborations", func(w http.ResponseWriter, r *http.Request) { @@ -97,13 +115,18 @@ func TestLegacyFlow(t *testing.T) { svr := httptest.NewServer(mux) defer svr.Close() - compiler := New(&settings.Config{ + cfg := &settings.Config{ Host: svr.URL, Endpoint: "/graphql-unstable", HTTPClient: http.DefaultClient, Token: "", - }) - _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + } + apiClient, err := newAPIClient(cfg) + assert.NoError(t, err) + collaboratorsClient, err := collaborators.NewCollaboratorsRestClient(*cfg) + assert.NoError(t, err) + compiler := New(apiClient, collaboratorsClient) + _, err = compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.Error(t, err) assert.Contains(t, err.Error(), "config compilation request returned an error:") assert.Equal(t, 0, gqlHitCounter) diff --git a/config/v2_api_client.go b/config/v2_api_client.go new file mode 100644 index 000000000..6d9c6bbe1 --- /dev/null +++ b/config/v2_api_client.go @@ -0,0 +1,62 @@ +package config + +import ( + "fmt" + "net/url" + + "github.com/CircleCI-Public/circleci-cli/api/rest" + "github.com/pkg/errors" +) + +const v2_string apiClientVersion = "v2" + +type v2APIClient struct { + restClient *rest.Client +} + +func configErrorsAsError(configErrors []ConfigError) error { + message := "config compilation contains errors:" + for _, err := range configErrors { + message += fmt.Sprintf("\n\t- %s", err.Message) + } + return errors.New(message) +} + +func (client *v2APIClient) CompileConfig(configContent string, orgID string, params Parameters, values Values) (*ConfigResponse, error) { + compileRequest := CompileConfigRequest{ + ConfigYaml: configContent, + Options: Options{ + OwnerID: orgID, + PipelineValues: values, + PipelineParameters: params, + }, + } + + req, err := client.restClient.NewRequest( + "POST", + &url.URL{ + Path: "compile-config-with-defaults", + }, + compileRequest, + ) + if err != nil { + return nil, fmt.Errorf("an error occurred creating the request: %w", err) + } + + configCompilationResp := &ConfigResponse{} + statusCode, originalErr := client.restClient.DoRequest(req, configCompilationResp) + + if originalErr != nil { + return nil, fmt.Errorf("config compilation request returned an error: %w", originalErr) + } + + if statusCode != 200 { + return nil, errors.New("unable to validate or compile config") + } + + if len(configCompilationResp.Errors) > 0 { + return nil, fmt.Errorf("config compilation contains errors: %s", configErrorsAsError(configCompilationResp.Errors)) + } + + return configCompilationResp, nil +} diff --git a/config/v2_api_client_test.go b/config/v2_api_client_test.go new file mode 100644 index 000000000..d5ebec6f2 --- /dev/null +++ b/config/v2_api_client_test.go @@ -0,0 +1,17 @@ +package config + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +func TestConfigErrorsAsError(t *testing.T) { + err := configErrorsAsError([]ConfigError{ + {Message: "error on line 1"}, + {Message: "error on line 42"}, + }) + assert.Error(t, err, `config compilation contains errors: + - error on line 1 + - error on line 42`) +} diff --git a/local/local.go b/local/local.go index 39272c91a..4aa60ae72 100644 --- a/local/local.go +++ b/local/local.go @@ -26,7 +26,10 @@ func Execute(flags *pflag.FlagSet, cfg *settings.Config, args []string) error { var configResponse *config.ConfigResponse processedArgs, configPath := buildAgentArguments(flags) - compiler := config.New(cfg) + compiler, err := config.NewWithConfig(cfg) + if err != nil { + return err + } //if no orgId provided use org slug orgID, _ := flags.GetString("org-id") From fd1087e2145dc784709bd7498a3cfcf2d529893a Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Tue, 1 Aug 2023 16:50:16 +0200 Subject: [PATCH 02/11] style: Fix linter issue --- api/rest/client.go | 3 +-- config/api_client.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/rest/client.go b/api/rest/client.go index 8204237fe..a64d14778 100644 --- a/api/rest/client.go +++ b/api/rest/client.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http" "net/url" "strings" @@ -97,7 +96,7 @@ func (c *Client) DoRequest(req *http.Request, resp interface{}) (int, error) { httpError := struct { Message string `json:"message"` }{} - body, err := ioutil.ReadAll(httpResp.Body) + body, err := io.ReadAll(httpResp.Body) if err != nil { return httpResp.StatusCode, err } diff --git a/config/api_client.go b/config/api_client.go index 1d3493d05..f90d16d75 100644 --- a/config/api_client.go +++ b/config/api_client.go @@ -34,7 +34,7 @@ func GetAPIClient(config *settings.Config) (APIClient, error) { } func newAPIClient(config *settings.Config) (APIClient, error) { - hostValue := getCompileHost(config.Host) + hostValue := GetCompileHost(config.Host) restClient := rest.NewFromConfig(hostValue, config) version, err := detectAPIClientVersion(restClient) From 42ca438920441b5ade0856975e994cb113f5804d Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Wed, 2 Aug 2023 10:01:57 +0200 Subject: [PATCH 03/11] style: Linter error addressed --- cmd/policy/policy.go | 10 ++++++++-- config/api_client.go | 6 ++---- config/api_client_test.go | 23 +++-------------------- config/v1_api_client_test.go | 4 ++-- 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/cmd/policy/policy.go b/cmd/policy/policy.go index dfecd3371..6ccab181c 100644 --- a/cmd/policy/policy.go +++ b/cmd/policy/policy.go @@ -295,7 +295,10 @@ This group of commands allows the management of polices to be verified against b } if !noCompile && context == "config" { - compiler := config.New(globalConfig) + compiler, err := config.NewWithConfig(globalConfig) + if err != nil { + return err + } input, err = mergeCompiledConfig(compiler, config.ProcessConfigOpts{ ConfigPath: inputPath, OrgID: ownerID, @@ -376,7 +379,10 @@ This group of commands allows the management of polices to be verified against b } if !noCompile && context == "config" { - compiler := config.New(globalConfig) + compiler, err := config.NewWithConfig(globalConfig) + if err != nil { + return err + } input, err = mergeCompiledConfig(compiler, config.ProcessConfigOpts{ ConfigPath: inputPath, OrgID: ownerID, diff --git a/config/api_client.go b/config/api_client.go index f90d16d75..cd87a6fd6 100644 --- a/config/api_client.go +++ b/config/api_client.go @@ -59,10 +59,8 @@ func detectAPIClientVersion(restClient *rest.Client) (apiClientVersion, error) { } statusCode, err := restClient.DoRequest(req, nil) - if err != nil { - if _, ok := err.(*rest.HTTPError); ok { - return "", err - } + if _, ok := err.(*rest.HTTPError); !ok { + return "", err } if statusCode == 404 { return v1_string, nil diff --git a/config/api_client_test.go b/config/api_client_test.go index 7011afafc..cc61be18d 100644 --- a/config/api_client_test.go +++ b/config/api_client_test.go @@ -13,25 +13,9 @@ import ( func TestAPIClient(t *testing.T) { t.Run("detectCompilerVersion", func(t *testing.T) { - t.Run("when the route returns a 400 tells that the version is v2", func(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(400) - w.Header().Set("Content-Type", "application/octet-stream") - fmt.Fprintf(w, "Invalid input") - })) - url, err := url.Parse(svr.URL) - assert.NilError(t, err) - - restClient := rest.New(url, "token", http.DefaultClient) - version, err := detectAPIClientVersion(restClient) - assert.NilError(t, err) - assert.Equal(t, version, v2_string) - }) - t.Run("when the route returns a 404 tells that the version is v1", func(t *testing.T) { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(404) - w.Header().Set("Content-Type", "application/octet-stream") fmt.Fprintf(w, "Invalid input") })) url, err := url.Parse(svr.URL) @@ -43,10 +27,9 @@ func TestAPIClient(t *testing.T) { assert.Equal(t, version, v1_string) }) - t.Run("in any other case, return an error", func(t *testing.T) { + t.Run("on other cases return v2", func(t *testing.T) { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(500) - w.Header().Set("Content-Type", "application/octet-stream") fmt.Fprintf(w, "Invalid input") })) url, err := url.Parse(svr.URL) @@ -54,8 +37,8 @@ func TestAPIClient(t *testing.T) { restClient := rest.New(url, "token", http.DefaultClient) version, err := detectAPIClientVersion(restClient) - assert.Equal(t, version, apiClientVersion("")) - assert.Error(t, err, "Invalid input") + assert.NilError(t, err) + assert.Equal(t, version, v2_string) }) }) } diff --git a/config/v1_api_client_test.go b/config/v1_api_client_test.go index 2eb0ce1da..64a78c8fe 100644 --- a/config/v1_api_client_test.go +++ b/config/v1_api_client_test.go @@ -2,7 +2,7 @@ package config import ( "fmt" - "io/ioutil" + "io" "net/http" "net/http/httptest" "testing" @@ -91,7 +91,7 @@ func TestAPIV1Flow(t *testing.T) { gqlHitCounter := 0 mux.HandleFunc("/compile-config-with-defaults", func(w http.ResponseWriter, r *http.Request) { - body, err := ioutil.ReadAll(r.Body) + body, err := io.ReadAll(r.Body) assert.NoError(t, err) if len(body) == 0 { w.WriteHeader(http.StatusBadRequest) From 416053e9a6d51e9b4441c68a8e504ffae5795f9d Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Wed, 2 Aug 2023 15:09:33 +0200 Subject: [PATCH 04/11] Changed the way we create config API client Fixed the tests that needed to be adapted to this change --- cmd/policy/policy_test.go | 20 ++++++++++++++++---- config/api_client.go | 14 -------------- config/config.go | 2 +- config/v2_api_client.go | 4 +--- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/cmd/policy/policy_test.go b/cmd/policy/policy_test.go index 8fbe1a72c..227a6a66d 100644 --- a/cmd/policy/policy_test.go +++ b/cmd/policy/policy_test.go @@ -635,7 +635,10 @@ test: config CompilerServerHandler: func(w http.ResponseWriter, r *http.Request) { var req config.CompileConfigRequest err := json.NewDecoder(r.Body).Decode(&req) - require.NoError(t, err) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } // dummy compilation here (remove the _compiled_ key in compiled config, as compiled config can't have that at top-level key). var yamlResp map[string]any @@ -678,7 +681,10 @@ test: config CompilerServerHandler: func(w http.ResponseWriter, r *http.Request) { var req config.CompileConfigRequest err := json.NewDecoder(r.Body).Decode(&req) - require.NoError(t, err) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } response := config.ConfigResponse{Valid: true, SourceYaml: req.ConfigYaml, OutputYaml: req.ConfigYaml} @@ -837,7 +843,10 @@ test: config CompilerServerHandler: func(w http.ResponseWriter, r *http.Request) { var req config.CompileConfigRequest err := json.NewDecoder(r.Body).Decode(&req) - require.NoError(t, err) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } response := config.ConfigResponse{Valid: true, SourceYaml: req.ConfigYaml, OutputYaml: req.ConfigYaml} @@ -1082,7 +1091,10 @@ test: config CompilerServerHandler: func(w http.ResponseWriter, r *http.Request) { var req config.CompileConfigRequest err := json.NewDecoder(r.Body).Decode(&req) - require.NoError(t, err) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } response := config.ConfigResponse{Valid: true, SourceYaml: req.ConfigYaml, OutputYaml: req.ConfigYaml} diff --git a/config/api_client.go b/config/api_client.go index cd87a6fd6..6c62bef0b 100644 --- a/config/api_client.go +++ b/config/api_client.go @@ -3,7 +3,6 @@ package config import ( "fmt" "net/url" - "sync" "github.com/CircleCI-Public/circleci-cli/api/graphql" "github.com/CircleCI-Public/circleci-cli/api/rest" @@ -11,10 +10,6 @@ import ( ) var ( - compiler APIClient - compilerError error - once sync.Once - compilePath = "compile-config-with-defaults" ) @@ -24,15 +19,6 @@ type APIClient interface { CompileConfig(configContent string, orgID string, params Parameters, values Values) (*ConfigResponse, error) } -func GetAPIClient(config *settings.Config) (APIClient, error) { - if compiler == nil { - once.Do(func() { - compiler, compilerError = newAPIClient(config) - }) - } - return compiler, compilerError -} - func newAPIClient(config *settings.Config) (APIClient, error) { hostValue := GetCompileHost(config.Host) restClient := rest.NewFromConfig(hostValue, config) diff --git a/config/config.go b/config/config.go index 741384220..9d52be9a1 100644 --- a/config/config.go +++ b/config/config.go @@ -24,7 +24,7 @@ type ConfigCompiler struct { } func NewWithConfig(cfg *settings.Config) (*ConfigCompiler, error) { - apiClient, err := GetAPIClient(cfg) + apiClient, err := newAPIClient(cfg) if err != nil { return nil, err } diff --git a/config/v2_api_client.go b/config/v2_api_client.go index 6d9c6bbe1..2ae9a3152 100644 --- a/config/v2_api_client.go +++ b/config/v2_api_client.go @@ -34,9 +34,7 @@ func (client *v2APIClient) CompileConfig(configContent string, orgID string, par req, err := client.restClient.NewRequest( "POST", - &url.URL{ - Path: "compile-config-with-defaults", - }, + &url.URL{Path: compilePath}, compileRequest, ) if err != nil { From 4fe5391bfa9b45dda188fe80c161ff7580376b50 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Mon, 7 Aug 2023 17:16:35 +0200 Subject: [PATCH 05/11] fix snapcraft deploy --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 453bda49a..edd87a19d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -253,7 +253,7 @@ jobs: - attach_workspace: at: . - run: | - TAG=$(./dist/circleci-cli_linux_amd64/circleci version | cut -d' ' -f 1) && export TAG + TAG=$(./dist/circleci-cli_linux_amd64_v1/circleci version | cut -d' ' -f 1) && export TAG sed -i -- "s/%CLI_VERSION_PLACEHOLDER%/$TAG/g" snap/snapcraft.yaml - run: snapcraft - run: From d391614ba5727cb82c9eddedf7efd09236333969 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Tue, 8 Aug 2023 09:23:03 +0200 Subject: [PATCH 06/11] trigger ci From 188282bbd2fcf7681ee468a111b428db93ab6e39 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Tue, 8 Aug 2023 09:48:50 +0200 Subject: [PATCH 07/11] style: Addressed PR comments --- api/rest/client.go | 2 +- config/api_client.go | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/api/rest/client.go b/api/rest/client.go index a64d14778..1e8e9e6bb 100644 --- a/api/rest/client.go +++ b/api/rest/client.go @@ -98,7 +98,7 @@ func (c *Client) DoRequest(req *http.Request, resp interface{}) (int, error) { }{} body, err := io.ReadAll(httpResp.Body) if err != nil { - return httpResp.StatusCode, err + return 0, err } err = json.Unmarshal(body, &httpError) if err != nil { diff --git a/config/api_client.go b/config/api_client.go index 6c62bef0b..22c791fcc 100644 --- a/config/api_client.go +++ b/config/api_client.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "net/http" "net/url" "github.com/CircleCI-Public/circleci-cli/api/graphql" @@ -9,9 +10,7 @@ import ( "github.com/CircleCI-Public/circleci-cli/settings" ) -var ( - compilePath = "compile-config-with-defaults" -) +const compilePath = "compile-config-with-defaults" type apiClientVersion string @@ -38,17 +37,25 @@ func newAPIClient(config *settings.Config) (APIClient, error) { } } +// detectAPIClientVersion returns the highest available version of the config API. +// +// To do that it tries to request the `compilePath` API route. +// If the route returns a 404, this means the route does not exist on the requested host and the function returns +// `v1_string` indicating that the deprecated GraphQL endpoint should be used instead. +// Else if the route returns any other status, this means it is available for request and the function returns +// `v2_string` indicating that the route can be used func detectAPIClientVersion(restClient *rest.Client) (apiClientVersion, error) { req, err := restClient.NewRequest("POST", &url.URL{Path: compilePath}, nil) if err != nil { return "", err } - statusCode, err := restClient.DoRequest(req, nil) - if _, ok := err.(*rest.HTTPError); !ok { + _, err = restClient.DoRequest(req, nil) + httpErr, ok := err.(*rest.HTTPError) + if !ok { return "", err } - if statusCode == 404 { + if httpErr.Code == http.StatusNotFound { return v1_string, nil } return v2_string, nil From 3dd9730c5df245d4d6ca0a2d3dd34ad840271950 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Wed, 19 Jul 2023 09:20:24 +0200 Subject: [PATCH 08/11] refactor: Moved the orb validation in a orb API module --- api/api.go | 127 ------------------------------------------ api/orb/client.go | 105 ++++++++++++++++++++++++++++++++++ api/orb/deprecated.go | 50 +++++++++++++++++ api/orb/latest.go | 48 ++++++++++++++++ api/orb/yaml.go | 24 ++++++++ cmd/orb.go | 13 ++++- cmd/orb_test.go | 106 ++++++++++++++++++++++++++++++++++- 7 files changed, 341 insertions(+), 132 deletions(-) create mode 100644 api/orb/client.go create mode 100644 api/orb/deprecated.go create mode 100644 api/orb/latest.go create mode 100644 api/orb/yaml.go diff --git a/api/api.go b/api/api.go index 0c51b486d..2dbaca0bd 100644 --- a/api/api.go +++ b/api/api.go @@ -512,133 +512,6 @@ func WhoamiQuery(cl *graphql.Client) (*WhoamiResponse, error) { return &response, nil } -// OrbQuery validated and processes an orb. -func OrbQuery(cl *graphql.Client, configPath string, ownerId string) (*ConfigResponse, error) { - var response OrbConfigResponse - - config, err := loadYaml(configPath) - if err != nil { - return nil, err - } - - request, err := makeOrbRequest(cl, config, ownerId) - if err != nil { - return nil, err - } - - err = cl.Run(request, &response) - if err != nil { - return nil, errors.Wrap(err, "Unable to validate config") - } - - if len(response.OrbConfig.ConfigResponse.Errors) > 0 { - return nil, response.OrbConfig.ConfigResponse.Errors - } - - return &response.OrbConfig.ConfigResponse, nil -} - -func makeOrbRequest(cl *graphql.Client, configContent string, ownerId string) (*graphql.Request, error) { - handlesOwner := orbQueryHandleOwnerId(cl) - - if handlesOwner { - query := ` - query ValidateOrb ($config: String!, $owner: UUID) { - orbConfig(orbYaml: $config, ownerId: $owner) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }` - - request := graphql.NewRequest(query) - request.Var("config", configContent) - - if ownerId != "" { - request.Var("owner", ownerId) - } - - request.SetToken(cl.Token) - return request, nil - } - - if ownerId != "" { - return nil, errors.Errorf("Your version of Server does not support validating orbs that refer to other private orbs. Please see the README for more information on server compatibility: https://github.com/CircleCI-Public/circleci-cli#server-compatibility") - } - query := ` - query ValidateOrb ($config: String!) { - orbConfig(orbYaml: $config) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }` - - request := graphql.NewRequest(query) - request.Var("config", configContent) - - request.SetToken(cl.Token) - return request, nil -} - -type OrbIntrospectionResponse struct { - Schema struct { - Query struct { - Fields []struct { - Name string `json:"name"` - Args []struct { - Name string `json:"name"` - } `json:"args"` - } `json:"fields"` - } `json:"queryType"` - } `json:"__schema"` -} - -func orbQueryHandleOwnerId(cl *graphql.Client) bool { - query := ` -query ValidateOrb { - __schema { - queryType { - fields(includeDeprecated: true) { - name - args { - name - __typename - type { - name - } - } - } - } - } -}` - request := graphql.NewRequest(query) - response := OrbIntrospectionResponse{} - err := cl.Run(request, &response) - if err != nil { - return false - } - - request.SetToken(cl.Token) - - // Find the orbConfig query method, look at its arguments, if it has the "ownerId" argument, return true - for _, field := range response.Schema.Query.Fields { - if field.Name == "orbConfig" { - for _, arg := range field.Args { - if arg.Name == "ownerId" { - return true - } - } - } - } - - // else return false, ownerId is not supported - - return false -} - // OrbImportVersion publishes a new version of an orb using the provided source and id. func OrbImportVersion(cl *graphql.Client, orbSrc string, orbID string, orbVersion string) (*Orb, error) { var response OrbImportVersionResponse diff --git a/api/orb/client.go b/api/orb/client.go new file mode 100644 index 000000000..bc71ac2c0 --- /dev/null +++ b/api/orb/client.go @@ -0,0 +1,105 @@ +package orb + +import ( + "fmt" + "sync" + + "github.com/CircleCI-Public/circleci-cli/api" + "github.com/CircleCI-Public/circleci-cli/api/graphql" + "github.com/CircleCI-Public/circleci-cli/settings" +) + +var ( + once sync.Once + client Client +) + +// ConfigResponse is a structure that matches the result of the GQL +// query, so that we can use mapstructure to convert from +// nested maps to a strongly typed struct. +type QueryResponse struct { + OrbConfig struct { + api.ConfigResponse + } +} + +type Client interface { + OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) +} + +func GetClient(config *settings.Config) Client { + once.Do(func() { + createClient(config) + }) + return client +} + +func createClient(config *settings.Config) { + gql := graphql.NewClient(config.HTTPClient, config.Host, config.Endpoint, config.Token, config.Debug) + + ok, err := orbQueryHandleOwnerId(gql) + if err != nil { + fmt.Printf("While requesting orb server: %s", err) + return + } else if ok { + client = &latestClient{gql} + } else { + client = &deprecatedClient{gql} + } +} + +type OrbIntrospectionResponse struct { + Schema struct { + Query struct { + Fields []struct { + Name string `json:"name"` + Args []struct { + Name string `json:"name"` + } `json:"args"` + } `json:"fields"` + } `json:"queryType"` + } `json:"__schema"` +} + +func orbQueryHandleOwnerId(gql *graphql.Client) (bool, error) { + query := ` +query ValidateOrb { + __schema { + queryType { + fields(includeDeprecated: true) { + name + args { + name + __typename + type { + name + } + } + } + } + } +}` + request := graphql.NewRequest(query) + response := OrbIntrospectionResponse{} + err := gql.Run(request, &response) + if err != nil { + return false, err + } + + request.SetToken(gql.Token) + + // Find the orbConfig query method, look at its arguments, if it has the "ownerId" argument, return true + for _, field := range response.Schema.Query.Fields { + if field.Name == "orbConfig" { + for _, arg := range field.Args { + if arg.Name == "ownerId" { + return true, nil + } + } + } + } + + // else return false, ownerId is not supported + + return false, nil +} diff --git a/api/orb/deprecated.go b/api/orb/deprecated.go new file mode 100644 index 000000000..ca73f8c01 --- /dev/null +++ b/api/orb/deprecated.go @@ -0,0 +1,50 @@ +package orb + +import ( + "github.com/CircleCI-Public/circleci-cli/api" + "github.com/CircleCI-Public/circleci-cli/api/graphql" + "github.com/pkg/errors" +) + +type deprecatedClient struct { + gql *graphql.Client +} + +func (deprecated *deprecatedClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { + if ownerId != "" { + return nil, errors.New("Your version of Server does not support validating orbs that refer to other private orbs. Please see the README for more information on server compatibility: https://github.com/CircleCI-Public/circleci-cli#server-compatibility") + } + + var response QueryResponse + + configContent, err := loadYaml(configPath) + if err != nil { + return nil, err + } + + query := ` + query ValidateOrb ($config: String!) { + orbConfig(orbYaml: $config) { + valid, + errors { message }, + sourceYaml, + outputYaml + } + }` + + request := graphql.NewRequest(query) + request.Var("config", configContent) + + request.SetToken(deprecated.gql.Token) + + err = deprecated.gql.Run(request, &response) + if err != nil { + return nil, errors.Wrap(err, "Unable to validate config") + } + + if len(response.OrbConfig.ConfigResponse.Errors) > 0 { + return nil, response.OrbConfig.ConfigResponse.Errors + } + + return &response.OrbConfig.ConfigResponse, nil +} diff --git a/api/orb/latest.go b/api/orb/latest.go new file mode 100644 index 000000000..91918688d --- /dev/null +++ b/api/orb/latest.go @@ -0,0 +1,48 @@ +package orb + +import ( + "github.com/CircleCI-Public/circleci-cli/api" + "github.com/CircleCI-Public/circleci-cli/api/graphql" + "github.com/pkg/errors" +) + +type latestClient struct { + gql *graphql.Client +} + +func (latest *latestClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { + var response QueryResponse + + configContent, err := loadYaml(configPath) + if err != nil { + return nil, err + } + + query := ` + query ValidateOrb ($config: String!, $owner: UUID) { + orbConfig(orbYaml: $config, ownerId: $owner) { + valid, + errors { message }, + sourceYaml, + outputYaml + } + }` + request := graphql.NewRequest(query) + request.Var("config", configContent) + + if ownerId != "" { + request.Var("owner", ownerId) + } + request.SetToken(latest.gql.Token) + + err = latest.gql.Run(request, &response) + if err != nil { + return nil, errors.Wrap(err, "Unable to validate config") + } + + if len(response.OrbConfig.ConfigResponse.Errors) > 0 { + return nil, response.OrbConfig.ConfigResponse.Errors + } + + return &response.OrbConfig.ConfigResponse, nil +} diff --git a/api/orb/yaml.go b/api/orb/yaml.go new file mode 100644 index 000000000..22cae2e91 --- /dev/null +++ b/api/orb/yaml.go @@ -0,0 +1,24 @@ +package orb + +import ( + "io" + "os" + + "github.com/pkg/errors" +) + +func loadYaml(path string) (string, error) { + var err error + var config []byte + if path == "-" { + config, err = io.ReadAll(os.Stdin) + } else { + config, err = os.ReadFile(path) + } + + if err != nil { + return "", errors.Wrapf(err, "Could not load config file at %s", path) + } + + return string(config), nil +} diff --git a/cmd/orb.go b/cmd/orb.go index a7037c7d8..87e454c49 100644 --- a/cmd/orb.go +++ b/cmd/orb.go @@ -20,6 +20,7 @@ import ( "github.com/CircleCI-Public/circleci-cli/api" "github.com/CircleCI-Public/circleci-cli/api/collaborators" "github.com/CircleCI-Public/circleci-cli/api/graphql" + "github.com/CircleCI-Public/circleci-cli/api/orb" "github.com/CircleCI-Public/circleci-cli/filetree" "github.com/CircleCI-Public/circleci-cli/process" "github.com/CircleCI-Public/circleci-cli/prompt" @@ -732,7 +733,11 @@ func validateOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - _, err = api.OrbQuery(opts.cl, opts.args[0], orgId) + client := orb.GetClient(opts.cfg) + if client == nil { + return fmt.Errorf("Unable to validate orb") + } + _, err = client.OrbQuery(opts.args[0], orgId) if err != nil { return err @@ -754,7 +759,11 @@ func processOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - response, err := api.OrbQuery(opts.cl, opts.args[0], orgId) + client := orb.GetClient(opts.cfg) + if client == nil { + return fmt.Errorf("Unable to validate orb") + } + response, err := client.OrbQuery(opts.args[0], orgId) if err != nil { return err diff --git a/cmd/orb_test.go b/cmd/orb_test.go index 7b4e917ba..a2f98ca13 100644 --- a/cmd/orb_test.go +++ b/cmd/orb_test.go @@ -210,9 +210,12 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }) }) - Describe("with old server version", func() { + Describe("org-id parameter", func() { BeforeEach(func() { token = "testtoken" + }) + + It("should use the old GraphQL resolver when the parameter is not present", func() { command = exec.Command(pathCLI, "orb", "validate", "--skip-update-check", @@ -229,9 +232,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs panic(err) } }() - }) - It("should use the old GraphQL resolver", func() { By("setting up a mock server") mockOrbIntrospection(false, "", tempSettings) @@ -279,6 +280,105 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Eventually(session.Out).Should(gbytes.Say("Orb input is valid.")) Eventually(session).Should(gexec.Exit(0)) }) + + It("indicate a deprecation error when using the parameter with outdated Server", func() { + command = exec.Command(pathCLI, + "orb", "validate", + "--skip-update-check", + "--token", token, + "--host", tempSettings.TestServer.URL(), + "--org-id", "org-id", + "-", + ) + stdin, err := command.StdinPipe() + Expect(err).ToNot(HaveOccurred()) + go func() { + defer stdin.Close() + _, err := io.WriteString(stdin, "{}") + if err != nil { + panic(err) + } + }() + + By("setting up a mock server") + + mockOrbIntrospection(false, "", tempSettings) + + session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) + + Expect(err).ShouldNot(HaveOccurred()) + Eventually(session.Err).Should(gbytes.Say("Your version of Server does not support validating orbs that refer to other private orbs. Please see the README for more information on server compatibility: https://github.com/CircleCI-Public/circleci-cli#server-compatibility")) + Eventually(session).Should(gexec.Exit(-1)) + }) + + It("should work properly with modern Server", func() { + command = exec.Command(pathCLI, + "orb", "validate", + "--skip-update-check", + "--token", token, + "--host", tempSettings.TestServer.URL(), + "--org-id", "org-id", + "-", + ) + stdin, err := command.StdinPipe() + Expect(err).ToNot(HaveOccurred()) + go func() { + defer stdin.Close() + _, err := io.WriteString(stdin, "{}") + if err != nil { + panic(err) + } + }() + + By("setting up a mock server") + + mockOrbIntrospection(true, "", tempSettings) + gqlResponse := `{ + "orbConfig": { + "sourceYaml": "{}", + "valid": true, + "errors": [] + } + }` + + response := struct { + Query string `json:"query"` + Variables struct { + Config string `json:"config"` + Owner string `json:"owner"` + } `json:"variables"` + }{ + Query: ` + query ValidateOrb ($config: String!, $owner: UUID) { + orbConfig(orbYaml: $config, ownerId: $owner) { + valid, + errors { message }, + sourceYaml, + outputYaml + } + }`, + Variables: struct { + Config string `json:"config"` + Owner string `json:"owner"` + }{ + Config: "{}", + Owner: "org-id", + }, + } + expected, err := json.Marshal(response) + Expect(err).ShouldNot(HaveOccurred()) + + tempSettings.AppendPostHandler(token, clitest.MockRequestResponse{ + Status: http.StatusOK, + Request: string(expected), + Response: gqlResponse}) + + session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) + + Expect(err).ShouldNot(HaveOccurred()) + Eventually(session.Out).Should(gbytes.Say("Orb input is valid.")) + Eventually(session).Should(gexec.Exit(0)) + }) }) Context("with 'some orb'", func() { From 7408eb6280a5084ddda035fd22aacfebace697d5 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Mon, 24 Jul 2023 08:33:02 +0200 Subject: [PATCH 09/11] Adressed pr comments --- api/orb/client.go | 95 +++++++++++++++------- api/orb/{deprecated.go => v1_client.go} | 31 ++++---- api/orb/{latest.go => v2_client.go} | 32 ++++---- api/orb/yaml.go | 24 ------ cmd/orb.go | 12 +-- cmd/orb_test.go | 100 ++++++++++++------------ 6 files changed, 155 insertions(+), 139 deletions(-) rename api/orb/{deprecated.go => v1_client.go} (59%) rename api/orb/{latest.go => v2_client.go} (50%) delete mode 100644 api/orb/yaml.go diff --git a/api/orb/client.go b/api/orb/client.go index bc71ac2c0..607398f8d 100644 --- a/api/orb/client.go +++ b/api/orb/client.go @@ -2,18 +2,24 @@ package orb import ( "fmt" + "io" + "os" "sync" "github.com/CircleCI-Public/circleci-cli/api" "github.com/CircleCI-Public/circleci-cli/api/graphql" "github.com/CircleCI-Public/circleci-cli/settings" + "github.com/pkg/errors" ) var ( - once sync.Once - client Client + once sync.Once + client Client + clientError error ) +type clientVersion string + // ConfigResponse is a structure that matches the result of the GQL // query, so that we can use mapstructure to convert from // nested maps to a strongly typed struct. @@ -27,27 +33,43 @@ type Client interface { OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) } -func GetClient(config *settings.Config) Client { +func GetClient(config *settings.Config) (Client, error) { once.Do(func() { - createClient(config) + client, clientError = newClient(config) }) - return client + + return client, clientError } -func createClient(config *settings.Config) { +func newClient(config *settings.Config) (Client, error) { gql := graphql.NewClient(config.HTTPClient, config.Host, config.Endpoint, config.Token, config.Debug) - ok, err := orbQueryHandleOwnerId(gql) + clientVersion, err := detectClientVersion(gql) if err != nil { - fmt.Printf("While requesting orb server: %s", err) - return - } else if ok { - client = &latestClient{gql} - } else { - client = &deprecatedClient{gql} + return nil, err + } + + switch clientVersion { + case v1_string: + return &v1Client{gql}, nil + case v2_string: + return &v2Client{gql}, nil + default: + return nil, fmt.Errorf("Unable to recognise your server orb API") } } +func detectClientVersion(gql *graphql.Client) (clientVersion, error) { + handlesOwnerId, err := orbQueryHandleOwnerId(gql) + if err != nil { + return "", err + } + if !handlesOwnerId { + return v1_string, nil + } + return v2_string, nil +} + type OrbIntrospectionResponse struct { Schema struct { Query struct { @@ -62,22 +84,21 @@ type OrbIntrospectionResponse struct { } func orbQueryHandleOwnerId(gql *graphql.Client) (bool, error) { - query := ` -query ValidateOrb { - __schema { - queryType { - fields(includeDeprecated: true) { - name - args { - name - __typename - type { - name - } - } - } - } - } + query := `query IntrospectionQuery { + _schema { + queryType { + fields(includeDeprecated: true) { + name + args { + name + __typename + type { + name + } + } + } + } + } }` request := graphql.NewRequest(query) response := OrbIntrospectionResponse{} @@ -103,3 +124,19 @@ query ValidateOrb { return false, nil } + +func loadYaml(path string) (string, error) { + var err error + var config []byte + if path == "-" { + config, err = io.ReadAll(os.Stdin) + } else { + config, err = os.ReadFile(path) + } + + if err != nil { + return "", errors.Wrapf(err, "Could not load config file at %s", path) + } + + return string(config), nil +} diff --git a/api/orb/deprecated.go b/api/orb/v1_client.go similarity index 59% rename from api/orb/deprecated.go rename to api/orb/v1_client.go index ca73f8c01..3fee53040 100644 --- a/api/orb/deprecated.go +++ b/api/orb/v1_client.go @@ -6,11 +6,15 @@ import ( "github.com/pkg/errors" ) -type deprecatedClient struct { +// This client makes request to servers that **DON'T** have the field `ownerId` in the GraphQL query method: `orbConfig` + +const v1_string clientVersion = "v1" + +type v1Client struct { gql *graphql.Client } -func (deprecated *deprecatedClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { +func (client *v1Client) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { if ownerId != "" { return nil, errors.New("Your version of Server does not support validating orbs that refer to other private orbs. Please see the README for more information on server compatibility: https://github.com/CircleCI-Public/circleci-cli#server-compatibility") } @@ -22,24 +26,23 @@ func (deprecated *deprecatedClient) OrbQuery(configPath string, ownerId string) return nil, err } - query := ` - query ValidateOrb ($config: String!) { - orbConfig(orbYaml: $config) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }` + query := `query ValidateOrb ($config: String!) { + orbConfig(orbYaml: $config) { + valid, + errors { message }, + sourceYaml, + outputYaml + } +}` request := graphql.NewRequest(query) request.Var("config", configContent) - request.SetToken(deprecated.gql.Token) + request.SetToken(client.gql.Token) - err = deprecated.gql.Run(request, &response) + err = client.gql.Run(request, &response) if err != nil { - return nil, errors.Wrap(err, "Unable to validate config") + return nil, errors.Wrap(err, "Validating config") } if len(response.OrbConfig.ConfigResponse.Errors) > 0 { diff --git a/api/orb/latest.go b/api/orb/v2_client.go similarity index 50% rename from api/orb/latest.go rename to api/orb/v2_client.go index 91918688d..a33496f93 100644 --- a/api/orb/latest.go +++ b/api/orb/v2_client.go @@ -6,11 +6,15 @@ import ( "github.com/pkg/errors" ) -type latestClient struct { +// This client makes request to servers that **DO** have the field `ownerId` in the GraphQL query method: `orbConfig` + +const v2_string clientVersion = "v2" + +type v2Client struct { gql *graphql.Client } -func (latest *latestClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { +func (client *v2Client) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) { var response QueryResponse configContent, err := loadYaml(configPath) @@ -18,26 +22,26 @@ func (latest *latestClient) OrbQuery(configPath string, ownerId string) (*api.Co return nil, err } - query := ` - query ValidateOrb ($config: String!, $owner: UUID) { - orbConfig(orbYaml: $config, ownerId: $owner) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }` + query := `query ValidateOrb ($config: String!, $owner: UUID) { + orbConfig(orbYaml: $config, ownerId: $owner) { + valid, + errors { message }, + sourceYaml, + outputYaml + } +}` + request := graphql.NewRequest(query) request.Var("config", configContent) if ownerId != "" { request.Var("owner", ownerId) } - request.SetToken(latest.gql.Token) + request.SetToken(client.gql.Token) - err = latest.gql.Run(request, &response) + err = client.gql.Run(request, &response) if err != nil { - return nil, errors.Wrap(err, "Unable to validate config") + return nil, errors.Wrap(err, "Validating config") } if len(response.OrbConfig.ConfigResponse.Errors) > 0 { diff --git a/api/orb/yaml.go b/api/orb/yaml.go deleted file mode 100644 index 22cae2e91..000000000 --- a/api/orb/yaml.go +++ /dev/null @@ -1,24 +0,0 @@ -package orb - -import ( - "io" - "os" - - "github.com/pkg/errors" -) - -func loadYaml(path string) (string, error) { - var err error - var config []byte - if path == "-" { - config, err = io.ReadAll(os.Stdin) - } else { - config, err = os.ReadFile(path) - } - - if err != nil { - return "", errors.Wrapf(err, "Could not load config file at %s", path) - } - - return string(config), nil -} diff --git a/cmd/orb.go b/cmd/orb.go index 87e454c49..cdacda85b 100644 --- a/cmd/orb.go +++ b/cmd/orb.go @@ -733,9 +733,9 @@ func validateOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - client := orb.GetClient(opts.cfg) - if client == nil { - return fmt.Errorf("Unable to validate orb") + client, err := orb.GetClient(opts.cfg) + if err != nil { + return errors.Wrap(err, "Getting orb client") } _, err = client.OrbQuery(opts.args[0], orgId) @@ -759,9 +759,9 @@ func processOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - client := orb.GetClient(opts.cfg) - if client == nil { - return fmt.Errorf("Unable to validate orb") + client, err := orb.GetClient(opts.cfg) + if err != nil { + return errors.Wrap(err, "Getting orb client") } response, err := client.OrbQuery(opts.args[0], orgId) diff --git a/cmd/orb_test.go b/cmd/orb_test.go index a2f98ca13..66187398b 100644 --- a/cmd/orb_test.go +++ b/cmd/orb_test.go @@ -125,15 +125,14 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Config string `json:"config"` } `json:"variables"` }{ - Query: ` - query ValidateOrb ($config: String!, $owner: UUID) { - orbConfig(orbYaml: $config, ownerId: $owner) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }`, + Query: `query ValidateOrb ($config: String!, $owner: UUID) { + orbConfig(orbYaml: $config, ownerId: $owner) { + valid, + errors { message }, + sourceYaml, + outputYaml + } +}`, Variables: struct { Config string `json:"config"` }{ @@ -189,7 +188,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }` expectedRequestJson := ` { - "query": "\n\t\tquery ValidateOrb ($config: String!, $owner: UUID) {\n\t\t\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\t\t\tvalid,\n\t\t\t\terrors { message },\n\t\t\t\tsourceYaml,\n\t\t\t\toutputYaml\n\t\t\t}\n\t\t}", + "query": "query ValidateOrb ($config: String!, $owner: UUID) {\n\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\tvalid,\n\t\terrors { message },\n\t\tsourceYaml,\n\t\toutputYaml\n\t}\n}", "variables": { "config": "{}" } @@ -210,12 +209,12 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }) }) - Describe("org-id parameter", func() { + Describe("when using org-id parameter", func() { BeforeEach(func() { token = "testtoken" }) - It("should use the old GraphQL resolver when the parameter is not present", func() { + It("should use the old GraphQL resolver when the parameter is not present on the server pointed by host", func() { command = exec.Command(pathCLI, "orb", "validate", "--skip-update-check", @@ -251,15 +250,14 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Config string `json:"config"` } `json:"variables"` }{ - Query: ` - query ValidateOrb ($config: String!) { - orbConfig(orbYaml: $config) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }`, + Query: `query ValidateOrb ($config: String!) { + orbConfig(orbYaml: $config) { + valid, + errors { message }, + sourceYaml, + outputYaml + } +}`, Variables: struct { Config string `json:"config"` }{ @@ -281,7 +279,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Eventually(session).Should(gexec.Exit(0)) }) - It("indicate a deprecation error when using the parameter with outdated Server", func() { + It("indicate a deprecation error when the parameter is not present on the server pointed by host", func() { command = exec.Command(pathCLI, "orb", "validate", "--skip-update-check", @@ -311,7 +309,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Eventually(session).Should(gexec.Exit(-1)) }) - It("should work properly with modern Server", func() { + It("should work properly when the parameter is present", func() { command = exec.Command(pathCLI, "orb", "validate", "--skip-update-check", @@ -348,15 +346,14 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs Owner string `json:"owner"` } `json:"variables"` }{ - Query: ` - query ValidateOrb ($config: String!, $owner: UUID) { - orbConfig(orbYaml: $config, ownerId: $owner) { - valid, - errors { message }, - sourceYaml, - outputYaml - } - }`, + Query: `query ValidateOrb ($config: String!, $owner: UUID) { + orbConfig(orbYaml: $config, ownerId: $owner) { + valid, + errors { message }, + sourceYaml, + outputYaml + } +}`, Variables: struct { Config string `json:"config"` Owner string `json:"owner"` @@ -410,7 +407,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }` expectedRequestJson := ` { - "query": "\n\t\tquery ValidateOrb ($config: String!, $owner: UUID) {\n\t\t\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\t\t\tvalid,\n\t\t\t\terrors { message },\n\t\t\t\tsourceYaml,\n\t\t\t\toutputYaml\n\t\t\t}\n\t\t}", + "query": "query ValidateOrb ($config: String!, $owner: UUID) {\n\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\tvalid,\n\t\terrors { message },\n\t\tsourceYaml,\n\t\toutputYaml\n\t}\n}", "variables": { "config": "some orb" } @@ -447,7 +444,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }` expectedRequestJson := ` { - "query": "\n\t\tquery ValidateOrb ($config: String!, $owner: UUID) {\n\t\t\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\t\t\tvalid,\n\t\t\t\terrors { message },\n\t\t\t\tsourceYaml,\n\t\t\t\toutputYaml\n\t\t\t}\n\t\t}", + "query": "query ValidateOrb ($config: String!, $owner: UUID) {\n\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\tvalid,\n\t\terrors { message },\n\t\tsourceYaml,\n\t\toutputYaml\n\t}\n}", "variables": { "config": "some orb" } @@ -492,7 +489,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }` expectedRequestJson := ` { - "query": "\n\t\tquery ValidateOrb ($config: String!, $owner: UUID) {\n\t\t\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\t\t\tvalid,\n\t\t\t\terrors { message },\n\t\t\t\tsourceYaml,\n\t\t\t\toutputYaml\n\t\t\t}\n\t\t}", + "query": "query ValidateOrb ($config: String!, $owner: UUID) {\n\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\tvalid,\n\t\terrors { message },\n\t\tsourceYaml,\n\t\toutputYaml\n\t}\n}", "variables": { "config": "some orb" } @@ -529,7 +526,7 @@ See a full explanation and documentation on orbs here: https://circleci.com/docs }` expectedRequestJson := ` { - "query": "\n\t\tquery ValidateOrb ($config: String!, $owner: UUID) {\n\t\t\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\t\t\tvalid,\n\t\t\t\terrors { message },\n\t\t\t\tsourceYaml,\n\t\t\t\toutputYaml\n\t\t\t}\n\t\t}", + "query": "query ValidateOrb ($config: String!, $owner: UUID) {\n\torbConfig(orbYaml: $config, ownerId: $owner) {\n\t\tvalid,\n\t\terrors { message },\n\t\tsourceYaml,\n\t\toutputYaml\n\t}\n}", "variables": { "config": "some orb" } @@ -3715,22 +3712,21 @@ func mockOrbIntrospection(isValid bool, token string, tempSettings *clitest.Temp Expect(err).ToNot(HaveOccurred()) requestStruct := map[string]interface{}{ - "query": ` -query ValidateOrb { - __schema { - queryType { - fields(includeDeprecated: true) { - name - args { - name - __typename - type { - name - } - } - } - } - } + "query": `query IntrospectionQuery { + _schema { + queryType { + fields(includeDeprecated: true) { + name + args { + name + __typename + type { + name + } + } + } + } + } }`, "variables": map[string]interface{}{}, } From 2f881908287555251dbb0cd96c0e20da4e26443b Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Tue, 8 Aug 2023 10:00:00 +0200 Subject: [PATCH 10/11] refactor: Removed the singleton pattern --- api/orb/client.go | 17 +---------------- cmd/orb.go | 4 ++-- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/api/orb/client.go b/api/orb/client.go index 607398f8d..ab190ddf5 100644 --- a/api/orb/client.go +++ b/api/orb/client.go @@ -4,7 +4,6 @@ import ( "fmt" "io" "os" - "sync" "github.com/CircleCI-Public/circleci-cli/api" "github.com/CircleCI-Public/circleci-cli/api/graphql" @@ -12,12 +11,6 @@ import ( "github.com/pkg/errors" ) -var ( - once sync.Once - client Client - clientError error -) - type clientVersion string // ConfigResponse is a structure that matches the result of the GQL @@ -33,15 +26,7 @@ type Client interface { OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) } -func GetClient(config *settings.Config) (Client, error) { - once.Do(func() { - client, clientError = newClient(config) - }) - - return client, clientError -} - -func newClient(config *settings.Config) (Client, error) { +func NewClient(config *settings.Config) (Client, error) { gql := graphql.NewClient(config.HTTPClient, config.Host, config.Endpoint, config.Token, config.Debug) clientVersion, err := detectClientVersion(gql) diff --git a/cmd/orb.go b/cmd/orb.go index cdacda85b..d7a011eb3 100644 --- a/cmd/orb.go +++ b/cmd/orb.go @@ -733,7 +733,7 @@ func validateOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - client, err := orb.GetClient(opts.cfg) + client, err := orb.NewClient(opts.cfg) if err != nil { return errors.Wrap(err, "Getting orb client") } @@ -759,7 +759,7 @@ func processOrb(opts orbOptions, org orbOrgOptions) error { return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error()) } - client, err := orb.GetClient(opts.cfg) + client, err := orb.NewClient(opts.cfg) if err != nil { return errors.Wrap(err, "Getting orb client") } From d3625e8255dbe688a207618be447757aa0cb607d Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Tue, 8 Aug 2023 11:30:55 +0200 Subject: [PATCH 11/11] docs: Added comment on --- api/orb/client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/orb/client.go b/api/orb/client.go index ab190ddf5..445479a08 100644 --- a/api/orb/client.go +++ b/api/orb/client.go @@ -44,6 +44,10 @@ func NewClient(config *settings.Config) (Client, error) { } } +// detectClientVersion returns the highest available version of the orb API +// +// To do that it checks that whether the GraphQL query has the parameter "ownerId" or not. +// If it does not have the parameter, the function returns `v1_string` else it returns `v2_string` func detectClientVersion(gql *graphql.Client) (clientVersion, error) { handlesOwnerId, err := orbQueryHandleOwnerId(gql) if err != nil {