From 3dd9730c5df245d4d6ca0a2d3dd34ad840271950 Mon Sep 17 00:00:00 2001 From: JulesFaucherre Date: Wed, 19 Jul 2023 09:20:24 +0200 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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 {