From 4a4fb0775d45baea1a5b0e8b5ec967f1ccfe5b65 Mon Sep 17 00:00:00 2001 From: Elliot Forbes Date: Thu, 20 Apr 2023 11:49:59 +0100 Subject: [PATCH 1/5] In order to improve clarity around version support for server instances and the CLI I've created a more in-depth version compatibility matrix --- README.md | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4358c64a6..563f174b9 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,37 @@ Please see the [documentation](https://circleci-public.github.io/circleci-cli) o ## Version Compatibility -As of version `0.1.24705` - we no longer support Server 3.x instances. In order to upgrade the CLI to the latest version, you'll need to update your instance of server to 4.x. +| Server Release | Support Dates | Component | Supported Version | +| ----------- | ----------- | ----------- | ----------- | +| 4.2.0 | | | | +| | | CircleCI CLI | 0.1.24705 | +| 4.1.2 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.1.1 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.1.0 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.0.4 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.0.3 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.0.2 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.0.1 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 4.0.0 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 3.4.6 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 3.4.5 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 3.4.4 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 3.4.3 | | | | +| | | CircleCI CLI | 0.1.23845 | +| 3.x | | | | +| | | CircleCI CLI | 0.1.23845 | +| 2.x | | | | +| | | CircleCI CLI | 0.1.23845 | + -`0.1.23845` is the last version to support Server 3.x and 2.x. From 7eb6cd98df56b819d8a97eaccf9e11a0e6427988 Mon Sep 17 00:00:00 2001 From: Elliot Forbes Date: Thu, 20 Apr 2023 11:59:13 +0100 Subject: [PATCH 2/5] Improving readability --- README.md | 49 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 563f174b9..45eddd846 100644 --- a/README.md +++ b/README.md @@ -182,37 +182,22 @@ Please see the [documentation](https://circleci-public.github.io/circleci-cli) o ## Version Compatibility -| Server Release | Support Dates | Component | Supported Version | -| ----------- | ----------- | ----------- | ----------- | -| 4.2.0 | | | | -| | | CircleCI CLI | 0.1.24705 | -| 4.1.2 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.1.1 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.1.0 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.0.4 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.0.3 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.0.2 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.0.1 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 4.0.0 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 3.4.6 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 3.4.5 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 3.4.4 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 3.4.3 | | | | -| | | CircleCI CLI | 0.1.23845 | -| 3.x | | | | -| | | CircleCI CLI | 0.1.23845 | -| 2.x | | | | -| | | CircleCI CLI | 0.1.23845 | +| Server Release | CircleCI Supported Version | +| ----------- | ----------- | +| 4.2.0 | 0.1.24705 | +| 4.1.2 | 0.1.23845 | +| 4.1.1 | 0.1.23845 | +| 4.1.0 | 0.1.23845 | +| 4.0.4 | 0.1.23845 | +| 4.0.3 | 0.1.23845 | +| 4.0.2 | 0.1.23845 | +| 4.0.1 | 0.1.23845 | +| 4.0.0 | 0.1.23845 | +| 3.4.6 | 0.1.23845 | +| 3.4.5 | 0.1.23845 | +| 3.4.4 | 0.1.23845 | +| 3.4.3 | 0.1.23845 | +| 3.x | 0.1.23845 | +| 2.x | 0.1.23845 | From f0116179e43c3f4605c6d239dcef01d6a28b67f1 Mon Sep 17 00:00:00 2001 From: Elliot Forbes Date: Thu, 20 Apr 2023 11:49:59 +0100 Subject: [PATCH 3/5] In order to improve clarity around version support for server instances and the CLI I've created a more in-depth version compatibility matrix --- api/rest/client.go | 1 - config/config.go | 19 ++++-- config/config_test.go | 23 +++---- config/legacy.go | 141 ++++++++++++++++++++++++++++++++++++++++++ config/legacy_test.go | 76 +++++++++++++++++++++++ 5 files changed, 244 insertions(+), 16 deletions(-) create mode 100644 config/legacy.go create mode 100644 config/legacy_test.go diff --git a/api/rest/client.go b/api/rest/client.go index 93a89ac9b..2d1628f41 100644 --- a/api/rest/client.go +++ b/api/rest/client.go @@ -98,7 +98,6 @@ func (c *Client) DoRequest(req *http.Request, resp interface{}) (statusCode int, }{} err = json.NewDecoder(httpResp.Body).Decode(&httpError) if err != nil { - fmt.Printf("failed to decode body: %s", err.Error()) return httpResp.StatusCode, err } return httpResp.StatusCode, &HTTPError{Code: httpResp.StatusCode, Message: httpError.Message} diff --git a/config/config.go b/config/config.go index 896d2a337..61b770c65 100644 --- a/config/config.go +++ b/config/config.go @@ -6,6 +6,7 @@ import ( "net/url" "os" + "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" @@ -23,6 +24,9 @@ type ConfigCompiler struct { host string compileRestClient *rest.Client collaboratorRestClient *rest.Client + + cfg *settings.Config + legacyGraphQLClient *graphql.Client } func New(cfg *settings.Config) *ConfigCompiler { @@ -31,7 +35,10 @@ func New(cfg *settings.Config) *ConfigCompiler { host: hostValue, compileRestClient: rest.NewFromConfig(hostValue, cfg), collaboratorRestClient: rest.NewFromConfig(cfg.Host, cfg), + cfg: cfg, } + + configCompiler.legacyGraphQLClient = graphql.NewClient(cfg.HTTPClient, cfg.Host, cfg.Endpoint, cfg.Token, cfg.Debug) return configCompiler } @@ -102,11 +109,15 @@ func (c *ConfigCompiler) ConfigQuery( } configCompilationResp := &ConfigResponse{} - statusCode, err := c.compileRestClient.DoRequest(req, configCompilationResp) - if err != nil { - if statusCode == 404 { - return nil, errors.New("this version of the CLI does not support your instance of server, please refer to https://github.com/CircleCI-Public/circleci-cli for version compatibility") + statusCode, originalErr := c.compileRestClient.DoRequest(req, configCompilationResp) + if statusCode == 404 { + 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", err) } diff --git a/config/config_test.go b/config/config_test.go index deeba3c00..036d042d1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -72,17 +72,18 @@ func TestCompiler(t *testing.T) { assert.Contains(t, err.Error(), "Could not load config file at testdata/nonexistent.yml") }) - t.Run("handles 404 status correctly", func(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNotFound) - })) - defer svr.Close() - compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) - - _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) - assert.Error(t, err) - assert.Contains(t, err.Error(), "this version of the CLI does not support your instance of server") - }) + // commenting this out - we have a legacy_test.go unit test that covers this behaviour + // t.Run("handles 404 status correctly", func(t *testing.T) { + // svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // w.WriteHeader(http.StatusNotFound) + // })) + // defer svr.Close() + // compiler := New(&settings.Config{Host: svr.URL, HTTPClient: http.DefaultClient}) + + // _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + // assert.Error(t, err) + // assert.Contains(t, err.Error(), "this version of the CLI does not support your instance of server") + // }) t.Run("handles non-200 status correctly", func(t *testing.T) { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/config/legacy.go b/config/legacy.go new file mode 100644 index 000000000..02581bb74 --- /dev/null +++ b/config/legacy.go @@ -0,0 +1,141 @@ +package config + +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" +) + +// GQLErrorsCollection is a slice of errors returned by the GraphQL server. +// Each error is made up of a GQLResponseError type. +type GQLErrorsCollection []GQLResponseError + +// BuildConfigResponse wraps the GQL result of the ConfigQuery +type BuildConfigResponse struct { + BuildConfig struct { + LegacyConfigResponse + } +} + +// Error turns a GQLErrorsCollection into an acceptable error string that can be printed to the user. +func (errs GQLErrorsCollection) Error() string { + messages := []string{} + + for i := range errs { + messages = append(messages, errs[i].Message) + } + + return strings.Join(messages, "\n") +} + +// LegacyConfigResponse 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 LegacyConfigResponse struct { + Valid bool + SourceYaml string + OutputYaml string + + Errors GQLErrorsCollection +} + +// GQLResponseError is a mapping of the data returned by the GraphQL server of key-value pairs. +// Typically used with the structure "Message: string", but other response errors provide additional fields. +type GQLResponseError struct { + Message string + Value string + AllowedValues []string + EnumType string + Type string +} + +// 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 + keys := make([]string, 0, len(kvMap)) + for k := range kvMap { + keys = append(keys, k) + } + sort.Strings(keys) + + kvs := make([]KeyVal, 0, len(kvMap)) + for _, k := range keys { + kvs = append(kvs, KeyVal{Key: k, Val: kvMap[k]}) + } + return kvs +} + +func (c *ConfigCompiler) legacyConfigQueryByOrgID( + configString string, + orgID string, + params Parameters, + values Values, + cfg *settings.Config, +) (*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 + // backend is out of date. + var fieldAddendums string + if orgID != "" { + fieldAddendums += ", orgId: $orgId" + } + if len(params) > 0 { + fieldAddendums += ", pipelineParametersJson: $pipelineParametersJson" + } + query := fmt.Sprintf( + `query ValidateConfig ($config: String!, $pipelineParametersJson: String, $pipelineValues: [StringKeyVal!], $orgSlug: String) { + buildConfig(configYaml: $config, pipelineValues: $pipelineValues%s) { + valid, + errors { message }, + sourceYaml, + outputYaml + } + }`, + fieldAddendums, + ) + + request := graphql.NewRequest(query) + request.SetToken(cfg.Token) + request.Var("config", configString) + + if values != nil { + request.Var("pipelineValues", PrepareForGraphQL(values)) + } + if params != nil { + pipelineParameters, err := json.Marshal(params) + if err != nil { + return nil, fmt.Errorf("unable to serialize pipeline values: %s", err.Error()) + } + request.Var("pipelineParametersJson", string(pipelineParameters)) + } + + if orgID != "" { + request.Var("orgId", orgID) + } + + err := c.legacyGraphQLClient.Run(request, &response) + if err != nil { + return nil, errors.Wrap(err, "Unable to validate config") + } + if len(response.BuildConfig.LegacyConfigResponse.Errors) > 0 { + return nil, &response.BuildConfig.LegacyConfigResponse.Errors + } + + return &ConfigResponse{ + Valid: response.BuildConfig.LegacyConfigResponse.Valid, + SourceYaml: response.BuildConfig.LegacyConfigResponse.SourceYaml, + 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/legacy_test.go new file mode 100644 index 000000000..516232273 --- /dev/null +++ b/config/legacy_test.go @@ -0,0 +1,76 @@ +package config + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/CircleCI-Public/circleci-cli/settings" + "github.com/stretchr/testify/assert" +) + +func TestLegacyFlow(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() + + mux.HandleFunc("/compile-config-with-defaults", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + mux.HandleFunc("/me/collaborations", 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"}]`) + }) + + mux.HandleFunc("/graphql-unstable", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"data":{"buildConfig": {"valid":true,"sourceYaml":"%s","outputYaml":"%s","errors":[]}}}`, testYaml, testYaml) + }) + + svr := httptest.NewServer(mux) + defer svr.Close() + + compiler := New(&settings.Config{ + Host: svr.URL, + Endpoint: "/graphql-unstable", + HTTPClient: http.DefaultClient, + Token: "", + }) + resp, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + + assert.Equal(t, true, resp.Valid) + assert.NoError(t, err) + }) + + t.Run("tests that the compiler handles errors properly when returned from the graphQL endpoint", func(t *testing.T) { + mux := http.NewServeMux() + + mux.HandleFunc("/compile-config-with-defaults", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + mux.HandleFunc("/me/collaborations", 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"}]`) + }) + + mux.HandleFunc("/graphql-unstable", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"data":{"buildConfig":{"errors":[{"message": "failed to validate"}]}}}`) + }) + + svr := httptest.NewServer(mux) + defer svr.Close() + + compiler := New(&settings.Config{ + Host: svr.URL, + Endpoint: "/graphql-unstable", + HTTPClient: http.DefaultClient, + Token: "", + }) + _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) + assert.Error(t, err) + assert.Contains(t, "failed to validate", err.Error()) + }) +} From d40051701df4f94fa3651d6660593858d88da3a2 Mon Sep 17 00:00:00 2001 From: Elliot Forbes Date: Thu, 20 Apr 2023 16:21:10 +0100 Subject: [PATCH 4/5] Readme removal --- README.md | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/README.md b/README.md index 45eddd846..ec03bea08 100644 --- a/README.md +++ b/README.md @@ -179,25 +179,3 @@ Development instructions for the CircleCI CLI can be found in [HACKING.md](HACKI Please see the [documentation](https://circleci-public.github.io/circleci-cli) or `circleci help` for more. - -## Version Compatibility - -| Server Release | CircleCI Supported Version | -| ----------- | ----------- | -| 4.2.0 | 0.1.24705 | -| 4.1.2 | 0.1.23845 | -| 4.1.1 | 0.1.23845 | -| 4.1.0 | 0.1.23845 | -| 4.0.4 | 0.1.23845 | -| 4.0.3 | 0.1.23845 | -| 4.0.2 | 0.1.23845 | -| 4.0.1 | 0.1.23845 | -| 4.0.0 | 0.1.23845 | -| 3.4.6 | 0.1.23845 | -| 3.4.5 | 0.1.23845 | -| 3.4.4 | 0.1.23845 | -| 3.4.3 | 0.1.23845 | -| 3.x | 0.1.23845 | -| 2.x | 0.1.23845 | - - From 7d09d97adf5811177bf90da5dfc2e6d3f77dbaa8 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Thu, 20 Apr 2023 17:31:46 +0200 Subject: [PATCH 5/5] style: Added a log when going the legacy path --- config/config.go | 1 + config/legacy_test.go | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 61b770c65..abc451b87 100644 --- a/config/config.go +++ b/config/config.go @@ -111,6 +111,7 @@ func (c *ConfigCompiler) ConfigQuery( 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") legacyResponse, err := c.legacyConfigQueryByOrgID(configString, orgID, params, values, c.cfg) if err != nil { return nil, err diff --git a/config/legacy_test.go b/config/legacy_test.go index 516232273..5175a136f 100644 --- a/config/legacy_test.go +++ b/config/legacy_test.go @@ -71,6 +71,41 @@ func TestLegacyFlow(t *testing.T) { }) _, err := compiler.ConfigQuery("testdata/config.yml", "1234", Parameters{}, Values{}) assert.Error(t, err) - assert.Contains(t, "failed to validate", err.Error()) + assert.Contains(t, err.Error(), "failed to validate") + }) + + t.Run("tests that the compiler fails out completely when a non-404 is returned from the http endpoint", func(t *testing.T) { + mux := http.NewServeMux() + gqlHitCounter := 0 + + mux.HandleFunc("/compile-config-with-defaults", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + + }) + + mux.HandleFunc("/me/collaborations", 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"}]`) + }) + + mux.HandleFunc("/graphql-unstable", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"data":{"buildConfig":{"errors":[{"message": "failed to validate"}]}}}`) + gqlHitCounter++ + }) + + svr := httptest.NewServer(mux) + defer svr.Close() + + compiler := New(&settings.Config{ + Host: svr.URL, + Endpoint: "/graphql-unstable", + HTTPClient: http.DefaultClient, + Token: "", + }) + _, 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) }) }