Skip to content

Commit

Permalink
Merge pull request #977 from CircleCI-Public/task/DEVEX-1028/cli-abst…
Browse files Browse the repository at this point in the history
…ract-api-calls-to-allow-better-testing-and-backward-compatibility-CONFIG-ABSTRACTION

refactor: Abstracted the different config validation routes
  • Loading branch information
JulesFaucherre authored Aug 8, 2023
2 parents 4afe3de + 188282b commit 5b87507
Show file tree
Hide file tree
Showing 14 changed files with 355 additions and 138 deletions.
10 changes: 7 additions & 3 deletions api/rest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,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())
Expand All @@ -96,9 +96,13 @@ 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 := io.ReadAll(httpResp.Body)
if err != nil {
return httpResp.StatusCode, err
return 0, 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}
}
Expand Down
12 changes: 10 additions & 2 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -86,7 +90,11 @@ func newConfigCommand(globalConfig *settings.Config) *cobra.Command {
Use: "process <path>",
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")
Expand Down
10 changes: 8 additions & 2 deletions cmd/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 16 additions & 4 deletions cmd/policy/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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}

Expand Down
62 changes: 62 additions & 0 deletions config/api_client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package config

import (
"fmt"
"net/http"
"net/url"

"github.com/CircleCI-Public/circleci-cli/api/graphql"
"github.com/CircleCI-Public/circleci-cli/api/rest"
"github.com/CircleCI-Public/circleci-cli/settings"
)

const compilePath = "compile-config-with-defaults"

type apiClientVersion string

type APIClient interface {
CompileConfig(configContent string, orgID string, params Parameters, values Values) (*ConfigResponse, error)
}

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")
}
}

// 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
}

_, err = restClient.DoRequest(req, nil)
httpErr, ok := err.(*rest.HTTPError)
if !ok {
return "", err
}
if httpErr.Code == http.StatusNotFound {
return v1_string, nil
}
return v2_string, nil
}
44 changes: 44 additions & 0 deletions config/api_client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
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 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)
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("on other cases return v2", func(t *testing.T) {
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(500)
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)
})
})
}
40 changes: 31 additions & 9 deletions config/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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",
})
Expand Down Expand Up @@ -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",
})
Expand Down Expand Up @@ -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",
})
Expand Down
Loading

0 comments on commit 5b87507

Please sign in to comment.